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

Bazel 7 no longer allow label_flag used as constraint values for platforms #20494

Closed
tgeng opened this issue Dec 12, 2023 · 7 comments
Closed
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug untriaged

Comments

@tgeng
Copy link
Contributor

tgeng commented Dec 12, 2023

Description of the bug:

With Bazel 7 it seems one can no longer use a label_flag target when a constraint value is expected. I keep getting the following error

ERROR: /Users/tgeng/tmp/bazel-label-flag/BUILD:1:9: in constraint_values attribute of platform rule //:target_base: '//:target_cpu' does not have mandatory providers: 'ConstraintValueInfo'

Which category does this issue belong to?

No response

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

The following works with Bazel 6.4.0

~/tmp/workspace $ cat BUILD
platform(
    name = "target_base",
    constraint_values = ["target_cpu"],
)

label_flag(
    name = "target_cpu",
    build_setting_default = "@platforms//cpu:arm",
)
~/tmp/workspace $ bazel build //:target_base
INFO: Analyzed target //:target_base (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:target_base up-to-date (nothing to build)
INFO: Elapsed time: 0.074s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

But with Bazel 7 I have the following error:

~/tmp/workspace $ bazel build //:target_base
Starting local Bazel server and connecting to it...
ERROR: /Users/tgeng/tmp/bazel-label-flag/BUILD:1:9: in constraint_values attribute of platform rule //:target_base: '//:target_cpu' does not have mandatory providers: 'ConstraintValueInfo'
ERROR: /Users/tgeng/tmp/bazel-label-flag/BUILD:1:9: Analysis of target '//:target_base' failed
ERROR: Analysis of target '//:target_base' failed; build aborted
INFO: Elapsed time: 2.021s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully

Which operating system are you running Bazel on?

macOS, Linux

What is the output of bazel info release?

release 7.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

I tried running bisect but it failed.

$ bazelisk --bisect=6.4.0..7.0.0 build //:target_base


--- Getting the list of commits between 6.4.0 and 7.0.0

The good Bazel commit is not an ancestor of the bad Bazel commit, overriding the good Bazel commit to the merge base commit d60ce2c7c86393638c77698c00c2168a7a936a53
Found 3738 commits between (d60ce2c7c86393638c77698c00c2168a7a936a53, 7.0.0]


--- Verifying if the given good Bazel commit (d60ce2c7c86393638c77698c00c2168a7a936a53) is actually good

bazel build //:target_base
Starting local Bazel server and connecting to it...
INFO: Analyzed target //:target_base (2 packages loaded, 4 targets configured).
INFO: Found 1 target...
Target //:target_base up-to-date (nothing to build)
INFO: Elapsed time: 2.371s, Critical Path: 0.03s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action


--- Start bisecting



--- Testing with Bazel built at 6b7f1ef79096ae19cd84b2f077dd1292370a4cfb, 3738 commits remaining...

2023/12/12 00:11:29 Using unreleased version at commit 6b7f1ef79096ae19cd84b2f077dd1292370a4cfb
2023/12/12 00:11:29 Downloading https://storage.googleapis.com/bazel-builds/artifacts/centos7/6b7f1ef79096ae19cd84b2f077dd1292370a4cfb/bazel...
Downloading: 53 MB out of 53 MB (100%)
bazel build //:target_base
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
INFO: Analyzed target //:target_base (2 packages loaded, 4 targets configured).
INFO: Found 1 target...
Target //:target_base up-to-date (nothing to build)
INFO: Elapsed time: 4.813s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action


--- Succeeded at 6b7f1ef79096ae19cd84b2f077dd1292370a4cfb



--- Testing with Bazel built at a3ba48e7bf87c72bddb29b42b7825e746ec00f94, 1868 commits remaining...

2023/12/12 00:11:34 Using unreleased version at commit a3ba48e7bf87c72bddb29b42b7825e746ec00f94
2023/12/12 00:11:34 Downloading https://storage.googleapis.com/bazel-builds/artifacts/centos7/a3ba48e7bf87c72bddb29b42b7825e746ec00f94/bazel...
Downloading: 54 MB out of 54 MB (100%)
bazel build //:target_base
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
INFO: Analyzed target //:target_base (2 packages loaded, 4 targets configured).
INFO: Found 1 target...
Target //:target_base up-to-date (nothing to build)
INFO: Elapsed time: 4.627s, Critical Path: 0.03s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action


--- Succeeded at a3ba48e7bf87c72bddb29b42b7825e746ec00f94



--- Testing with Bazel built at a8d561cfc57117d834a70c981c264c9414f5ff5e, 933 commits remaining...

2023/12/12 00:11:40 Using unreleased version at commit a8d561cfc57117d834a70c981c264c9414f5ff5e
2023/12/12 00:11:40 Downloading https://storage.googleapis.com/bazel-builds/artifacts/centos7/a8d561cfc57117d834a70c981c264c9414f5ff5e/bazel...
Downloading: 54 MB out of 54 MB (100%)
bazel build //:target_base
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
INFO: Analyzed target //:target_base (2 packages loaded, 5 targets configured).
INFO: Found 1 target...
Target //:target_base up-to-date (nothing to build)
INFO: Elapsed time: 4.730s, Critical Path: 0.03s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action


--- Succeeded at a8d561cfc57117d834a70c981c264c9414f5ff5e



--- Testing with Bazel built at 6ed80327e796299ecffcdc6fcc94896fd32eaafe, 466 commits remaining...

2023/12/12 00:11:46 Using unreleased version at commit 6ed80327e796299ecffcdc6fcc94896fd32eaafe
2023/12/12 00:11:46 Downloading https://storage.googleapis.com/bazel-builds/artifacts/centos7/6ed80327e796299ecffcdc6fcc94896fd32eaafe/bazel...
2023/12/12 00:11:46 could not run Bazel: could not download Bazel: failed to download bazel: failed to download bazel: HTTP GET https://storage.googleapis.com/bazel-builds/artifacts/centos7/6ed80327e796299ecffcdc6fcc94896fd32eaafe/bazel failed with error 404

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@iancha1992 iancha1992 added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Dec 12, 2023
@meteorcloudy
Copy link
Member

meteorcloudy commented Dec 12, 2023

I bisected with bazelisk --bisect=6.4.0..HEAD build //:target_base to avoid the problematic commit (that is also being fixed so --bisect=6.4.0..7.0.0 could also work in about ~30 mins).

The culprit is 87fb462

/cc @katre , is this a intended breaking change or a regression in Bazel 7?

@katre
Copy link
Member

katre commented Dec 12, 2023

This is an intended change (and there was a previous issue about the same thing for rules_docker: bazelbuild/rules_docker#2275).

I do not understand what the intention behind this code is: there is already a perfectly good way to change the target platform with a flag, it is --platforms.

You should have fully static, non-configurable platform definitions, and use --platforms to change them (on the command line or in a transition, as needed).

This change will not be reverted.

@katre
Copy link
Member

katre commented Dec 12, 2023

To back up a step: @tgeng, why are you using this construct, and what do you think are the benefits?

@tgeng
Copy link
Contributor Author

tgeng commented Dec 12, 2023

I am using platform to set some additional constraint values, for example java and scala version (we did some platform/constraint gymnastics to support multiple language and dependency versions for our particular use case). We also do cross compiling across x86 and arm and currently the target os/arch switching is done through label_flag. This way, we don't need to create os/arch-specific platforms because the platform that contains language/dependency version can simply be derived from a "base" target, whose os and arch are specified through label_flag.

Without this ability, we will need to duplicate all of the platforms for every os/arch we need to support. Also, the transition logic would be much more complicated because one now loses the ability to change "an aspect" of the configuration. Instead, one must always change all the constraints through --platform.

@katre
Copy link
Member

katre commented Dec 12, 2023

I'd recommend using parent platforms, then, to get the same effect:

platform(
    name = "base",
    constraint_values = [java version, scala version],
)

platform(
    name = "intel",
    parents = [":base"], # Despite name, doesn't allow multiple parents
    constraint_values = [intel, linux],
)

But, honestly, I recommend not using constraints for language versions, for the reasons you note: this leads very quickly to combinatorial explosion of platforms. Instead, I recommend using user-defined flags for such things, since they can be set conveniently from bazelrc files and represent data that isn't actually tied to the platform.

To go back into the reasons behind 87fb462, as Bazel uses platforms more and more heavily, it becomes difficult if they depend on the configuration: every configured target needs to reference at least one platform, so reducing the number in memory has a large impact on Bazel's memory usage and efficiency.

@tgeng
Copy link
Contributor Author

tgeng commented Dec 12, 2023

As far as I understand, using flags won't be sufficient for our use case. Language version also affects dependency versions and we are using configurable dependencies to abstract away that. For example, scala 2 doesn't have binary backward-compatibility, so a scala library needs to depend on different standard library targets, depending on the current language version being set. In our use case, "platform" is more than just the platform of the host, it also includes language version and dependency versions. (we can't realistically enforce single version for everything because we needed a lot of external dependencies).

So I guess the way forward is to just bite the bullet and live with the combinatorial explosion of platforms in our transition logic.

@tgeng tgeng changed the title Bazel 7 no longer allow label_flag used as constraint values Bazel 7 no longer allow label_flag used as constraint values for platforms Dec 12, 2023
@katre
Copy link
Member

katre commented Jan 31, 2024

I think that's right, although I'd encourage you to try using the flag-based scala version (and others) first: I don't see what you can select with platforms that you can't select with an actual flag.

Closing this since it's working as intended.

@katre katre closed this as completed Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug untriaged
Projects
None yet
Development

No branches or pull requests

6 participants
@katre @meteorcloudy @tgeng @iancha1992 @rakshitasingh05 and others