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

exec_compatible_with should be able to be passed platforms #25363

Closed
matts1 opened this issue Feb 24, 2025 · 2 comments
Closed

exec_compatible_with should be able to be passed platforms #25363

matts1 opened this issue Feb 24, 2025 · 2 comments
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request untriaged

Comments

@matts1
Copy link
Contributor

matts1 commented Feb 24, 2025

Description of the feature request:

exec_compatible_with takes in a list of ConstraintValueInfo. A platform is simply a list of constraint values, so there's no reason you shouldn't be able to pass a platform in instead.

Which category does this issue belong to?

Configurability

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

This would simplify toolchain definition, as you'd now be able to specify:

platform(
  name = "my_remote_execution_platform",
  constraints = [
    ":my_constraint1",
    ":my_constraint2",
  ]
)

toolchain(
  name = "my_toolchain",
  toolchain = ":toolchain_config",
  exec_compatible_with = [":my_remote_execution_platform"],
)

This would allow you to automatically keep the toolchain definitions in line with your execution platform definitions.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

8.1.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 HEAD ?


Have you found anything relevant by searching the web?

No response

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

No response

@katre
Copy link
Member

katre commented Feb 24, 2025

The reason to not accept a platform is because constraints exist as an indirection between specific platforms and generic capabilities. Targets and toolchains should clarify what capabilities they require (using exec_compatible_with and target_compatible_with), and platforms specify the capabilities they provide. Allowing the use of platforms in those (or in config_setting, another common request) makes them much more tightly coupled.

In addition, the semantics are potentially confusing. Given the following, which platform can be used?

platform(name = "foo", constraint_values = ["@platforms//cpu:x86_64"])
platform(name = "baz", constraint_values = ["@platforms//cpu:x86_64"])
platform(name = "quux", parents = [":foo"])

toolchain(
    ...
    exec_compatible_with = [":foo"],
)

There are plausible arguments both for and against the platforms :bar and :quux matching the toolchain, and it's entirely possible that different users will expect different behavior.

I could see an argument for a rule that wraps the ConstraintCollection, and that can be used anywhere that expects an array of ConstraintValueInfo providers, but it would require some deep thinking around inheritance and equality. If you are interested and this is useful, please start with a proposal so we can discuss the full details of an implementation.

@katre katre closed this as completed Feb 24, 2025
@fmeum fmeum reopened this Feb 24, 2025
@fmeum fmeum closed this as not planned Won't fix, can't repro, duplicate, stale Feb 24, 2025
@matts1
Copy link
Contributor Author

matts1 commented Feb 24, 2025

I think what you said makes sense. I think It'd make more sense to just put a provider wrapping depset[ConstraintValueInfo] or sequence[ConstraintValueInfo] in starlark though (similar to what we did for action sets in the rule based toolchain):

ConstraintValueSetInfo = provider(
  fields = {"constraints": "set of constraints to be provided"}
)

def _extract_platform_constraints_impl(ctx):
  return [ConstraintValueSetInfo(constraints = ctx.attr.platform[platform_common.PlatformInfo].constraints)]

extract_platform_constraints = rule(
  implementation = _extract_platform_constraints_impl,
  attrs = {
    "platform": attr.label(providers = [platform_common.PlatformInfo]),
  }
  provides = [ConstraintValueSetInfo]
)

This would resolve your above problems by making it explicitly depending on the constraints of the platform rather than the platform.

extract_constraints(name = "foo_constraints", platform = ":foo")
toolchain(exec_compatible_with = [":foo_constraints"])

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