Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stdlib is built 4 times #3535

Open
linzhp opened this issue Apr 16, 2023 · 4 comments
Open

Stdlib is built 4 times #3535

linzhp opened this issue Apr 16, 2023 · 4 comments

Comments

@linzhp
Copy link
Contributor

linzhp commented Apr 16, 2023

For almost all go_library targets, rules_go builds 4 copies for stdlib: fastbuild vs opt (for tools), and cgo on vs. off. This happens to go_library targets that don't depend on cgo, e.g., @bazel_gazelle//cmd/gazelle:gazelle_lib. Also, I don't see a reason why we need to build stdlib for tools with both cgo on and off.

What version of rules_go are you using?

0.39.0

What version of gazelle are you using?

master branch

What version of Bazel are you using?

6.1.1

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

macOS arm64

Any other potentially useful information about your toolchain?

What did you do?

Check out Gazelle repo and run bazel aquery 'mnemonic(GoStdlib, deps(//cmd/gazelle:gazelle_lib))'

What did you expect to see?

There are only 1-2 GoStdlib actions

What did you see instead?

4 GoStdlib actions, for fastbuild vs opt, and cgo on and off

@fmeum
Copy link
Member

fmeum commented Apr 17, 2023

We can force any of these settings. Since only a tiny fraction of the stdlib is C, maybe we always want to build it with opt rather than fastbuild?

@linzhp
Copy link
Contributor Author

linzhp commented Apr 17, 2023

yeah, we can force "opt" that except in dbg mode, where it may remove some symbols needed for debug.

What I wonder is why there are both cgo on and off configurations. Instead of forcing either cgo on or off, I think we should remove the additional transition.

@linzhp
Copy link
Contributor Author

linzhp commented Apr 18, 2023

I just apply the patch below to force opt, but it doesn't stop bazel from build stdlib for 4 times though. There are other difference between "for tool" and regular builds:

  • is exec configuration: true|false
  • platform_suffix: exec-2B5CBBC6|null
  • copt: [-g0] vs empty
  • cxxopt: [-g0] vs empty
  • strip: always|sometimes
diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl
index 2883b982..c27bf1a3 100644
--- a/go/private/rules/transition.bzl
+++ b/go/private/rules/transition.bzl
@@ -272,12 +272,13 @@ def _go_stdlib_transition_impl(settings, _attr):
             settings[label] = value
     settings["//go/config:tags"] = [t for t in settings["//go/config:tags"] if t in _TAG_AFFECTS_STDLIB]
     settings["//go/private:bootstrap_nogo"] = False
+    settings["//command_line_option:compilation_mode"] = "opt"
     return settings
 
 go_stdlib_transition = transition(
     implementation = _go_stdlib_transition_impl,
-    inputs = _reset_transition_keys,
-    outputs = _reset_transition_keys,
+    inputs = _reset_transition_keys + ["//command_line_option:compilation_mode"],
+    outputs = _reset_transition_keys + ["//command_line_option:compilation_mode"],
 )

@fmeum
Copy link
Member

fmeum commented Apr 19, 2023

Yes, for these differences getting rid of a separate exec build will be difficult without new Bazel features. My work on these features is progressing, but it will take more time before this can be used for rules_go.

I think that the best we can do for now is to get the count down to a single exec build of the stdlib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants