toolchains: remove default value from cgo and darwin constraints#2118
toolchains: remove default value from cgo and darwin constraints#2118jayconrod merged 1 commit intobazel-contrib:masterfrom
Conversation
Previously, //go/toolchains:cgo_constraint and darwin_constraint had default values cgo_on and is_darwin (on macOS only). This enabled cgo and the darwin config_setting (again, macOS only) when the default target platform (@bazel_tools//platforms:target_platform) is used, which is normally true. I believed these defaults would only apply to platforms that didn't specify a value otherwise. It appears that these defaults also add constraints to toolchains that don't specify a value otherwise (bazelbuild/bazel#8778). These means non-Go toolchains (e.g., docker_toolchain) are incompatible with non-default target platforms, since they don't specify values for these constraints. With this change, cgo_constraint no longer has a default value. darwin_constraint and its values are deleted entirely. Platforms defined in //go/toolchain specify cgo_on or cgo_off. Toolchains defined in @go_sdk (or any similar workspace) require cgo_on or no value (i.e., cgo is off by default), except for the toolchains for the host platform, which require cgo_off or no value (i.e., cgo is on by default). darwin_constraint and its values are deleted entirely. I can't think of an automatic way to make a single case in a select expression work. To slightly mitigate this, ios and ios_$goarch are added to //go/platform for use in separate select cases. The darwin config_setting will only be true on macOS, but the darwin build constraint will continue to work on both operating systems. Fixes bazel-contrib#2115 Updates bazel-contrib#2089
|
@chrislovecnm PTAL. I think this should fix the issue. @steeve Unfortunately, this means |
|
Will test drive 😁 |
|
@jayconrod I'm not worried about the new constraint, in fact I think it's nicer (like android vs linux). But I think gazelle might need updating too. |
|
@steeve I've opened bazel-contrib/bazel-gazelle#566 for this. |
|
great job jay thank you |
|
For the record I can't update Bazel nor rules_go because of issues related to this. I'll try this PR ! |
|
Would it make sense to leverage https://github.com/bazelbuild/platforms too ? |
Eventually yes, but not until that's canonical for the minimum supported version of Bazel. I think there's currently an incompatible flag that turns the targets in |
|
@jayconrod just tested external dns and I can run the container. If you want to double check my WORKSPACE file as I pulled in the PR directly from the github archive. Just want to make sure I tested it correctly. https://github.com/chrislovecnm/external-dns/blob/work-on-bazel/WORKSPACE |
|
@nlopezgi if you want to take a look. This seems to be working :) |
|
@jayconrod / @nlopezgi we talked about testing this, probably in rules_docker, how do you recommend proceeding?? |
|
@chrislovecnm Thanks for testing! I think this change and the underlying change is too complicated to backport to the release-0.18 branch. I'm planning to cut release-0.19 soon, once a few more cgo issues are resolved and dependencies are updated. |
`make bazel-crossbuild-kops` is currently failing [0] due to a bug in rules_go which was fixed [1]. Also updating gazelle while I'm at it. [0] https://prow.k8s.io/view/gcs/kubernetes-jenkins/logs/kops-postsubmit/1167506341137223682 [1] bazel-contrib/rules_go#2118
These changes fix a build error currently at HEAD and are also needed to prepare for building gke-exec-auth-plugin for Windows. Updates: - go toolchain to 1.13 - rules_go to v0.20.0 - gazelle to v0.19.0 - rules_docker to v0.11.0 io_bazel_rules_go needs updating for bazel-contrib/rules_go#2118 (bazel-contrib/rules_go#1240). bazel_gazelle and go_sdk need updating for compatibility with updated vendor/ targets. rules_docker needs updating to eliminate failures when running `bazel build ...` at HEAD. The corresponding update to the go toolchain is needed to completely eliminate the failures. For some more details see: https://gist.github.com/pjh/3e409e8d73dad63782b333c6535696de
These changes fix a build error currently at HEAD and are also needed to prepare for building gke-exec-auth-plugin for Windows. Updates: - go toolchain to 1.13 - rules_go to v0.20.0 - gazelle to v0.19.0 - rules_docker to v0.11.0 io_bazel_rules_go needs updating for bazel-contrib/rules_go#2118 (bazel-contrib/rules_go#1240). bazel_gazelle and go_sdk need updating for compatibility with updated vendor/ targets. rules_docker needs updating to eliminate failures when running `bazel build ...` at HEAD. The corresponding update to the go toolchain is needed to completely eliminate the failures. For some more details see: https://gist.github.com/pjh/3e409e8d73dad63782b333c6535696de
…when build sdist (bazel-contrib#2126) Building sdist results in `Could not find a version that satisfies the requirement setuptool` this regressed when a fix in parameter handling got introduced in bazel-contrib#2091. Before this change the building from sdist when using `experimental_index_url` would break because `--no-index` is passed to `pip`. This means that `pip` would fail to locate build time dependencies needed for the packages and would just not work. In `whl_library` we setup `PYTHONPATH` to have some build dependencies available (like `setuptools`) and we could use them during building from `sdist` and to do so we need to add `--no-build-isolation` flag. However, for some cases we need to also add other build-time dependencies (e.g. `flit_core`) so that the building of the wheel in the `repository_rule` context is successfuly. Removing `--no-index` allows `pip` to silently fetch the needed build dependencies from PyPI if they are missing and continue with the build. This is not a perfect solution, but it does unblock users to use the `sdist` distributions with the experimental feature enabled by using `experimental_index_url` (see bazel-contrib#260 for tracking of the completion). Fixes bazel-contrib#2118 Fixes bazel-contrib#2152 --------- Co-authored-by: aignas <240938+aignas@users.noreply.github.com>
Previously, //go/toolchains:cgo_constraint and darwin_constraint had
default values cgo_on and is_darwin (on macOS only). This enabled cgo
and the darwin config_setting (again, macOS only) when the default
target platform (@bazel_tools//platforms:target_platform) is used,
which is normally true.
I believed these defaults would only apply to platforms that didn't
specify a value otherwise. It appears that these defaults also add
constraints to toolchains that don't specify a value otherwise
(bazelbuild/bazel#8778). These means non-Go toolchains (e.g.,
docker_toolchain) are incompatible with non-default target platforms,
since they don't specify values for these constraints.
With this change, cgo_constraint no longer has a default
value. darwin_constraint and its values are deleted
entirely. Platforms defined in //go/toolchain specify cgo_on or
cgo_off. Toolchains defined in @go_sdk (or any similar workspace)
require cgo_on or no value (i.e., cgo is off by default), except for
the toolchains for the host platform, which require cgo_off or no
value (i.e., cgo is on by default).
darwin_constraint and its values are deleted entirely. I can't think
of an automatic way to make a single case in a select expression
work. To slightly mitigate this, ios and ios_$goarch are added to
//go/platform for use in separate select cases. The darwin
config_setting will only be true on macOS, but the darwin build
constraint will continue to work on both operating systems.
Fixes #2115
Updates #2089