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

Change platforms based on command line flags #19786

Closed
blorente opened this issue Oct 10, 2023 · 9 comments
Closed

Change platforms based on command line flags #19786

blorente opened this issue Oct 10, 2023 · 9 comments
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request untriaged

Comments

@blorente
Copy link

blorente commented Oct 10, 2023

Description of the feature request:

I am testing the latest prerelease, 7.0.0-pre.20230926.1, and I'm finding that commit 87fb46 broke one of our workflows.
We want to make a platform that you can transition to depending on a command line flag (in particular, whether you want to use a musl linker). In the past, we did it like:

constraint_value(name = "default", ...)
label_setting(
  name = "linker_setting",
  build_setting_default = ":default",
)
platform(
  name = "transition_use_musl" ,
  constraint_values = [":linker_setting"],
)

Understandably, the new commit means that label_setting can't derive its actual from the configuration. However, I'm not really sure how to proceed while the rest of Implement Platform-Based Flags #19409 is implemented. Is this something we should change in label_setting, or should we work around it some other way?

Which category does this issue belong to?

Configurability

What underlying problem are you trying to solve with this feature?

I would like to be able to configure a platform (as the target for a transition) based on a command line flag. Currently, in the rolling release, there seems to be no way to do so.

Which operating system are you running Bazel on?

macOS, but would like to configure the platform

What is the output of bazel info release?

release 7.0.0-pre.20230926.1

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

Have you found anything relevant by searching the web?

Asked on Slack: https://bazelbuild.slack.com/archives/CDCMRLS23/p1696954138130789

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

cc\ @katre , since they're the ones driving the work forward.

@katre
Copy link
Member

katre commented Oct 10, 2023

Several questions, really:

  1. Most basic: Why change the value of the --platforms flag based on another flag, instead of just directly setting --platforms?
  2. Why is "linker type" part of the platform definition at all? Since you can conceivably have multiple linkers installed on the same computer, this feels to me to be separate from platform definiton, and should probably be its own user-defined flag.

To clarify point two, you could have two cc_toolchain targets (one with and one without musl), and switch them like this:

# Define a custom flag:
bool_flag(name = "use_musl", default_value = "false") # bool_flag is defined in bazel_skylib
config_setting(
  name = "musl_enabled",
  flag_values = {
    ":use_musl": "true",
  },
)
config_setting(
  name = "musl_disabled",
  flag_values = {
    ":use_musl": "false",
  },
)

cc_toolchain(
  name = "cc_toolchain_with_musl",
  ....
)
cc_toolchain(
  name = "cc_toolchain_without_musl",
  ....
)

toolchain(
  name = "toolchain_with_musl",
  toolchain = ":cc_toolchain_with_musl",
  toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
  exec_compatible_with = ...
  target_compatible_with = ...,
  target_settings = [
    ":musl_enabled", # This toolchain is only available when the config_setting is true, which is to say, when the flag is set.
  ],
)
toolchain(
  name = "toolchain_without_musl",
  toolchain = ":cc_toolchain_without_musl",
  toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
  exec_compatible_with = ...
  target_compatible_with = ...,
  target_settings = [
    ":musl_disabled", # This toolchain is only available when the config_setting is true, which is to say, when the flag is not set.
  ],
)

This lets your flag to control "musl" be completely orthogonal to your platform definitions.

@sgowroji sgowroji added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Oct 10, 2023
@blorente
Copy link
Author

Unfortunately, we need to add the constraints inside a transition.
The broader context is that we've added musl support to rules_docker. So, to answer your questions:

  1. Most basic: Why change the value of the --platforms flag based on another flag, instead of just directly setting --platforms?

We're not using the platform specified in the CLI. We're using it to say "build a binary for {{platform}} to later be bundled into this platform" through a transition, which might be different from either the exec or host platforms.

  1. Why is "linker type" part of the platform definition at all? Since you can conceivably have multiple linkers installed on the same computer, this feels to me to be separate from platform definiton, and should probably be its own user-defined flag.

Probably "choice of linker" was not the best wording for our aim here. We want to tell rules_docker to build binaries for the platform we want (which will later be bundled into images). They accomplish that via a transition, so a platform is how we've coerced the rules to build for the platform we want.
I admit it's not ideal, and if the solution of the ticket is "it's already working as intended, find another way to do it" it's totally fine. I just want to make sure it's not a bug or something that's coming later.

@katre
Copy link
Member

katre commented Oct 10, 2023

See the design for standard platform transitions. This is currently being implemented, but the basic idea is that you (or your users) would tell rules_docker what specific platform to build for.

@blorente
Copy link
Author

Just read the proposal. It very helpful. It wouldn't let us toggle musl on or off from the command line (afaict platforms are still not configurable), but it will get us most of the way there.

Do you know if this will be implemented when Bazel 7 releases?

@katre
Copy link
Member

katre commented Oct 12, 2023

We're placing the new rules in the https://github.com/bazelbuild/platforms/ repository, so they're not bundled in Bazel at all. I believe the platform_data rule should be available in the next few weeks but I'm not working on it directly.

@blorente
Copy link
Author

Good to know, thanks. I'll close this issue, as I don't think there are any more action items.

Thanks again for your help!

@blorente
Copy link
Author

@katre We've continued testing on this, and saw that it breaks rules_docker outright, which likely means that Bazel 7 will be incompatible with rules_docker out of the box.
I understand that rules_docker is deprecated and shouldn't be used. We'll be working around that on our own internally (probably by switching to rules_oci). However, it's something the Bazel team might want to be aware of, as much of the community might be using rules_docker. I'm happy to follow up with any person who'd be relevant with more details.

@katre
Copy link
Member

katre commented Oct 31, 2023

It's unfortunate that rules_docker was using this construct, if they had discussed it with us I'd have been happy to explain the core problems.

If anyone who is using rules_docker can explain to me what this was supposed to be doing (it seems like they wanted a platform transition without using the --platforms flag, which doesn't make sense to me), I'm happy to discuss workarounds.

@fmeum
Copy link
Collaborator

fmeum commented Oct 31, 2023

Cc @alexeagle

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: feature request untriaged
Projects
None yet
Development

No branches or pull requests

6 participants