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

Use build_settings and transitions instead of aspect for cross-compilation #2219

Closed
jayconrod opened this issue Sep 13, 2019 · 2 comments · Fixed by #2414
Closed

Use build_settings and transitions instead of aspect for cross-compilation #2219

jayconrod opened this issue Sep 13, 2019 · 2 comments · Fixed by #2414

Comments

@jayconrod
Copy link
Contributor

rules_go provides two ways to cross-compile code:

  1. Users may set a target platform using --platforms on the command line. This selects a toolchain whose "default platform" is the target platform. We declare a separate go_toolchain and toolchain for each supported platform (even though the toolchains include the same files) to make this work. This mechanism generally works well.
  2. Users may set goos, goarch and other attributes on go_binary and go_test. Both rules use an aspect that creates a second graph of actions if goos, goarch and other attributes are different than the default. This mechanism the the source of a lot of problems. In particular, select expressions (used for platform-specific dependencies) do not work because select is applied before analysis.

Recent versions of Bazel support Starlark Build Configurations, which give users and Starlark rule authors much greater control over configuration transitions. We should use this functionality to remove the aspect. We may also be able to simplify toolchain declarations.

@siddharthab
Copy link
Contributor

#1600 can be marked as a duplicate of this issue.

@jmillikin-stripe
Copy link
Contributor

#2249 is related -- i want to use build settings to avoid the architecture-specific path component of go_binary outputs.

@jayconrod jayconrod added this to the v0.23 milestone Mar 20, 2020
jayconrod pushed a commit that referenced this issue Mar 26, 2020
This changes how the goos, goarch, race, msan, static, and pure
attributes of go_binary and go_test are implemented.

Previously, the go_binary and go_test rules used an aspect on the deps
and embed attributes. If any of the mode attributes listed above were
set, the aspect declared additional output files and actions to
generate them for the specified configuration. This approach had two
significant problems. First, the aspect caused the analysis to be
performed twice, wasting time and memory even in the case
where no cross-compilation was needed. Second, the aspect was not
compatible with conditional dependencies specified with select
expressions: aspects run on the configured target graph, but the mode
attributes could not affect Bazel's configuration.

This change deletes the aspect and implements these attributes using a
different mechanism: build settings and configuration transitions.

A new package is added //go/config with a build setting for each
attribute. Most settings are flags that may be set on the
command-line. This is an alternative to --feature flags, which will be
deprecated and eventually removed.

The target //:go_config gathers build settings relevant to Go and
provides a GoContextInfo (private).

The target //:go_context_data depends on //:go_config, //:stdlib,
//:nogo, and a few other standard dependencies. go_binary and go_test
rules no longer need to depend on these targets directly. Go rules
should now only need to depend on //:go_context_data and the Go
toolchain. Unfortunately, neither //:go_context_data nor //:go_config
can be part of the toolchain because the toolchain is always analyzed
for the execution configuration, not the target configuration.

Because of the simplified dependencies, the go_rule function is no
longer needed by most rules. This change makes it possible to
deprecate it.

Fixes #1351
Fixes #2219
Updates #2302
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Apr 3, 2020
This only applies to the master branch, not to current release
branches.

This is needed for configuration transition support. 1.2.0 (the
current minimum) and 2.1.0 (the version before 2.2.0) both have build
failures triggered by transitions.

For bazelbuild#2219
jayconrod pushed a commit that referenced this issue Apr 6, 2020
This only applies to the master branch, not to current release
branches.

This is needed for configuration transition support. 1.2.0 (the
current minimum) and 2.1.0 (the version before 2.2.0) both have build
failures triggered by transitions.

For #2219
jayconrod pushed a commit that referenced this issue Apr 17, 2020
This changes how the goos, goarch, race, msan, static, and pure
attributes of go_binary and go_test are implemented.

Previously, the go_binary and go_test rules used an aspect on the deps
and embed attributes. If any of the mode attributes listed above were
set, the aspect declared additional output files and actions to
generate them for the specified configuration. This approach had two
significant problems. First, the aspect caused the analysis to be
performed twice, wasting time and memory even in the case
where no cross-compilation was needed. Second, the aspect was not
compatible with conditional dependencies specified with select
expressions: aspects run on the configured target graph, but the mode
attributes could not affect Bazel's configuration.

This change deletes the aspect and implements these attributes using a
different mechanism: build settings and configuration transitions.

A new package is added //go/config with a build setting for each
attribute. Most settings are flags that may be set on the
command-line. This is an alternative to --feature flags, which will be
deprecated and eventually removed.

The target //:go_config gathers build settings relevant to Go and
provides a GoContextInfo (private).

The target //:go_context_data depends on //:go_config, //:stdlib,
//:nogo, and a few other standard dependencies. go_binary and go_test
rules no longer need to depend on these targets directly. Go rules
should now only need to depend on //:go_context_data and the Go
toolchain. Unfortunately, neither //:go_context_data nor //:go_config
can be part of the toolchain because the toolchain is always analyzed
for the execution configuration, not the target configuration.

Because of the simplified dependencies, the go_rule function is no
longer needed by most rules. This change makes it possible to
deprecate it.

Fixes #1351
Fixes #2219
Updates #2302
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants