-
-
Notifications
You must be signed in to change notification settings - Fork 678
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
Add support for --@io_bazel_rules_go//go/toolchain:sdk_version flag. #3260
Conversation
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. |
bb5854f
to
3f36886
Compare
There was a problem hiding this 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.
@JamesMBartlett If you rebase on |
4b7e1d9
to
735282c
Compare
go/toolchains.rst
Outdated
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``. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
735282c
to
f0e0f6b
Compare
- 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]>
f0e0f6b
to
d0c6770
Compare
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:sdk_version
. Eg. ago1.17.3
SDK would declare a config_setting forsdk_version="1"
,sdk_version="1.17"
,sdk_version="1.17.3"
, andsdk_version=""
.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]