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

Add support for --@io_bazel_rules_go//go/toolchain:sdk_version flag. #3260

Merged
merged 1 commit into from
Aug 16, 2022

Conversation

JamesMBartlett
Copy link
Contributor

What type of PR is this?
Feature

What does this PR do? Why is it needed?
This PR adds a build flag --@io_bazel_rules_go//go/toolchain:sdk_version, that allows changing which go SDK version to use to build targets. Implementation details:

  • Each go_sdk now has config_settings that specify different values of sdk_version. Eg. a go1.17.3 SDK would declare a config_setting for sdk_version="1", sdk_version="1.17", sdk_version="1.17.3", and sdk_version="".
  • Then each toolchain that is registered for an SDK will have a config_setting_group constraint that combines the above config_settings. In the example given above, the go1.17.3 SDK would have a toolchain that only gets selected if sdk_version is 1, 1.17, 1.17.3 or the empty string (default).
  • If sdk_version flag is not specified, then all SDKs can be selected because of the inclusion of the empty string config_setting in the config_setting_group.

Which issues(s) does this PR fix?
Partially #3202
Second diff coming to fully close that issue.

Other notes for review
See #3202 for more discussion about this feature.

Working with my employer on signing the CLA at the moment.

Signed-off-by: James Bartlett [email protected]

@google-cla
Copy link

google-cla bot commented Aug 2, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking pretty good already, thanks for the contribution!

I will look into the CI failure, doesn't seem to be this PR that causes it.

go/private/BUILD.sdk.bazel Outdated Show resolved Hide resolved
go/private/go_toolchain.bzl Outdated Show resolved Hide resolved
go/toolchains.rst Outdated Show resolved Hide resolved
go/toolchains.rst Show resolved Hide resolved
go/tools/gopackagesdriver/aspect.bzl Outdated Show resolved Hide resolved
@fmeum
Copy link
Collaborator

fmeum commented Aug 3, 2022

@JamesMBartlett If you rebase on master, you should be able to resolve the CI failure by updating the patch in tests/bcr/bcr.patch - if you have trouble doing that, let me know and I will take care of it.

@JamesMBartlett JamesMBartlett force-pushed the james/multiple_sdk_versions branch 3 times, most recently from 4b7e1d9 to 735282c Compare August 3, 2022 21:58
the flag ``--@io_bazel_rules_go//go/toolchain:sdk_version="version"`` where
``"version"`` is the SDK version you would like to build with, eg. ``"1.18.3"``.
The SDK version can omit the patch, or include a prerelease part, eg. ``"1"``,
``"1.18"``, ``"1.18.0"``, and ``"1.19beta1"`` are all valid values for ``sdk_version``.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we really match 1.19beta1 or would we have to specify 1.19.0beta1? Based on the settings above, I would have guessed the latter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I was on holiday.

Yeah you're right, we don't match 1.19beta1. I've updated the docs because I don't see a trivial way to support this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

- Each go_sdk now has config_settings that specify different values of
  sdk_version. Eg. a go1.17.3 SDK would declare a config_setting for
  sdk_version=1.17, sdk_version=1.17.3, and sdk_version="".
- Then each toolchain that is registered for an SDK will have a
  config_setting_group constraint that combines the above
  config_settings. In the example given above, the go1.17.3 SDK would
  have a toolchain that only gets selected if sdk_version is 1.17,
  1.17.3 or the empty string (default).
- If sdk_version flag is not specified, then all SDKs can be selected
  because of the inclusion of the empty string config_setting in the
  config_setting_group.

Signed-off-by: James Bartlett <[email protected]>
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

Successfully merging this pull request may close these issues.

2 participants