-
-
Notifications
You must be signed in to change notification settings - Fork 661
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 platform contraints for GOAMD64 #3251
Conversation
The new constraints `//go/constraints/amd64:v1` to `v4` can be used to mark a platform as supporting the corresponding level of `GOAMD64`. To use the new feature, users would supply their own platform: ```starlark platform( name = "amd64_with_avx", contraint_values = [ "@platforms//os:linux", "@platforms//cpu:x86_64", "@io_bazel_rules_go//go/constraints/amd64:v3", ], ) ``` and then specify the platform with `--platforms`.
Thanks for the quick reaction! Looks very promising (and almost definitely is what I need), I left a note on #3248. |
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.
I'd like to see @linzhp run this on the big repo, but LGTM
@achew22 Do you know a good place in the docs where I could explain this new feature? |
The changes look good to me. @rabbbit Did you test it in Uber? |
I patched it and can confirm the env contains the variable I need/want.
I have not tested whether the actual variable has an effect (yet), but this change looks good - thanks for the help @fmeum! :) |
It would be nice if instead of having to use |
We generally want to move away from that approach, but introduce a wrapper rule that transitions to a different user-provided platform. That gives you clear semantics with |
What type of PR is this?
Feature
What does this PR do? Why is it needed?
The new constraints
//go/constraints/amd64:v1
tov4
can be used tomark a platform as supporting the corresponding level of
GOAMD64
.To use the new feature, users would supply their own platform:
and then specify the platform with
--platforms
.Which issues(s) does this PR fix?
Fixes #3248
Other notes for review