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

Provide hooks for host platform autodetection #8766

Open
Tracked by #24
hlopko opened this issue Jul 2, 2019 · 27 comments
Open
Tracked by #24

Provide hooks for host platform autodetection #8766

hlopko opened this issue Jul 2, 2019 · 27 comments
Labels
help wanted Someone outside the Bazel team could own this P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request under investigation

Comments

@hlopko
Copy link
Member

hlopko commented Jul 2, 2019

Problem description

With #7081 we will get a nice repository rule that will autodetect the host platform target. This target will be used as a default value of the --platforms and --host_platform Bazel options, and also as the default execution platform.

Currently the autodetection detects the cpu and os and selects the right constraint from https://github.com/bazelbuild/platforms. This means that any Bazel toolchain that uses at most these 2 settings will be selectable by this platform.

But that is not enough for toolchains that define custom constraint settings. For those toolchains to be selected we need to put their constraints into the host platform. And those toolchains will very probably need to perform some kind of host system inspection to properly tell which constraint value to add.

Alternatives

Platform inheritance

One solution is to tell users to create their own platform target that inherits from the autodetected host platform. User will be responsible for making sure to collect all the constraint_settings from all the toolchains in their build and make sure they are represented in the inherited platform. Then they have to direct --platforms and --host_platform into their inherited platform target.

With this approach very few projects will end up using the default value of --platforms and --host_platform. This is not great and goes against the grain of 'flagless builds` effort of the configurability team.

Hooks into host platform autodetection

This approach makes advantage of the observation that all these toolchains are already being registered in the WORKSPACE file and they potentially already perform the autodetection for themselves. What we need is to pass a list of labels from these custom repository rules to the @local_config_platform rule.

Explicit approach

User will have to collect generated constraints from all their rules (e.g. rules will write a please_put_these_constraints_into_host.bzl file in their repo, the user will load that bzl file in the WORKSPACE, and use a constant from there), call local_config_platform manually, and pass constraints as an argument to the call.

  • Advantages:
    • no need to change default values of Bazel options
  • Disadvantages:
    • need to collect transitive constraints from rules repos
    • need to call local_config_platform manually
Implicit approach

We will figure out a way to allow rules to hook directly into local_config_platform. I have no idea how that would look like. Maybe the audience has ideas :)

  • Advantages:
    • no need to change default values of Bazel options
    • no need to call local_config_platform manually
    • no need to collect constraints
  • Disadvantages:
    • it's a kind of magic
@hlopko hlopko added under investigation team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. bazel 1.0 labels Jul 2, 2019
@hlopko hlopko self-assigned this Jul 2, 2019
@hlopko
Copy link
Member Author

hlopko commented Jul 2, 2019

CC @katre @dslomov @aehlig @laurentlb Wdyt? Do you have ideas for the Implicit approach for hooks?

@hlopko
Copy link
Member Author

hlopko commented Jul 2, 2019

@hlopko
Copy link
Member Author

hlopko commented Jul 3, 2019

@aehlig kindly prototyped an example using native.existing_rules and annotating repositories that want to contribute to the local_config_platform using a special tag.

Roughly:

def _impl(ctx):
    loads = []
    concats = []
    for repo in ctx.attr.relevant:
      loads.append("load('@" + repo + "//:foo.bzl', " + repo + "_foo='FOO')")
      concats.append(repo + "_foo")
    ctx.file("relevant.bzl", "\n".join(loads) + "\nFOO=" + " + ".join(concats))
    ctx.file(
        "BUILD",
        "load(':relevant.bzl', 'FOO')\n" +
          "platform(name = 'host', constraint_values = FOO)"

real_local_config_platform = repository_rule(
    implementation = _impl,
    attrs = {
        "relevant": attr.string_list(),
    },
)

def local_config_platform(name):
    existing = native.existing_rules()
    relevant = [
        k
        for k, v in existing.items()
        if "tags" in v and "local_config_platform_hook" in v["tags"]
    ]
    real_local_config_platform(name = name, relevant = relevant)

Therefore unless somebody objects I'll go ahead and implement the implicit approach for Hooks into host platform autodetection.

@agoulti
Copy link

agoulti commented Jul 3, 2019

I'd like to discuss how this might work with rbe_autoconfig, since we'll likely want something similar (and in general, for any other platform providers).

From what I understand, the Explicit approach should be easy to reuse in other rules and gives direct control to the users (I don't quite understand why please_put_these_constraints_into_host.bzl is needed, I assume a technical reason?)

I don't understand the code snippet for the Implicit approach. It seems like it's looking for a specific tag, but I don't know enough Starlark to understand it.

Where is the tag going to be set? Is this the per-target "tag" similar to "no-sandbox" or something different?

@hlopko
Copy link
Member Author

hlopko commented Jul 4, 2019

please_put_these_constraints_into_host.bzl is how we pass data from one repo to another. One repository writes a bzl file with a Starlark variable, following repository will load the bzl file and read the variable.

Implicit approach works by collecting all repositories with a specific tag, read a specific bzl file from each, and collect all the information in the final repository. E.g. cc_configure will be responsible for adding the tag to the repository it creates (cc_configure creates @local_config_cc repository - https://github.com/bazelbuild/bazel/blob/master/tools/cpp/cc_configure.bzl#L164. We'd just put , tags = ["tag_that_we_agreed_on"] and we're done.), and to write the bzl file that the @local_config_platform will load. @local_config_platform will iterate over all existing repositories, take those with the tag, load their bzl files, and process the data.

@hlopko
Copy link
Member Author

hlopko commented Jul 5, 2019

@aehlig @dslomov, your take? (I know @aehlig was slightly pro, and dslomov@ slightly against :) I'll also wait for @katre's opinion.

@aehlig
Copy link
Contributor

aehlig commented Jul 5, 2019

@aehlig @dslomov, your take?

As you already know, I think collecting "all repositories of a kind" via native.existing_rules() is fine. What I'm worried about is the use of random tags; it would be nice, if there was a simple rule for the user to know which tags have a special meaning to bazel and which are free for their own use.

@agoulti
Copy link

agoulti commented Jul 5, 2019

What would be a way to register toolchains whose constraints would not get automatically included in the host platform (for use in the remote platforms)?

Currently there is a way to reuse the auto-detection code by running Bazel inside a Docker container (see docker_toolchain_autoconfig).

If the produced toolchains indiscriminately auto-registered themselves on the host, this method would be broken.

@katre
Copy link
Member

katre commented Jul 8, 2019

This needs to be re-written as a design proposal and discussed in that fashion. A github issue thread isn't very discoverable for others who are interested in this topic.

This can be used as a tracking bug for the implementation after the design is agreed upon.

@lberki lberki removed the bazel 1.0 label Jul 25, 2019
@laurentlb laurentlb added P2 We'll consider working on this in future. (Assignee optional) type: feature request and removed untriaged labels Jul 29, 2019
@hlopko hlopko assigned katre and unassigned hlopko Dec 6, 2019
@katre
Copy link
Member

katre commented May 11, 2020

I am not actively working on this, so unassigning in case someone else wants to take a shot.

@katre katre removed their assignment May 11, 2020
@katre katre added the help wanted Someone outside the Bazel team could own this label May 11, 2020
@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
@burdiyan
Copy link

burdiyan commented Jun 3, 2022

It seems to me that unless there's an easy way to hook into the platform autodetection, the whole idea of platforms, constrains, and toolchains becomes very hard to use, and inevitably not flag-less.

@burdiyan
Copy link

burdiyan commented Jun 3, 2022

The explicit approach @hlopko describes in the first comment mentions this:

need to call local_config_platform manually

It seems like there's some function local_config_platform that could be called somewhere to provide additional constrains that might have been detection by a custom repository rule. But I couldn't find any references to this. The only thing I know about is the @local_config_platform external repository that is defines default constraints for the auto-detected platforms.

@katre
Copy link
Member

katre commented Jun 6, 2022

There is no function currently that allows this: I believe @hlopko was suggesting a possible way to implement autodetection by adding such a function.

I agree that this would be a useful feature, but unfortunately no one has had the time to write a design proposal and get agreement on what the best mechanism would be. We're keeping this open to track that it's something we want to come back to, but it isn't, unfortunately, something we can prioritize now.

I'm definitely open to accepting design proposals and implementations from the community, if anyone wants to take a look into the problem, but I am aware that the design is definitely the hard part here.

@fmeum
Copy link
Collaborator

fmeum commented Jul 26, 2022

@Wyverald If module extensions could depend on repositories defined in other module extensions, I think that this could be used to solve this issue in a very natural way:

  1. Somewhere in @bazel_tools, offer a module extension with a register_host_constraints tag class taking a label to a .bzl file. The .bzl file is expected to export an ADDITIONAL_HOST_CONSTRAINTS list with constraint value labels.
  2. Every ruleset generates such a .bzl file in a repository rule and passes the label to register_host_constraints in its MODULE.bazel file.
  3. A starlarkified local_config_platform module extension loops over all tags and generates a .bzl file that loads all constraints from the individual .bzl files and exports them as the combined HOST_CONSTRAINTS.

This is pretty much @hlopko's idea from #8766 (comment) expressed in Bzlmod terms. It looks like this could be done entirely in Starlark.

@fmeum
Copy link
Collaborator

fmeum commented Aug 7, 2022

This actually works today without any changes to Bazel (although it is affected by #15916 if the main module defines constraints).

As a demo, I created https://github.com/fmeum/local_config_platform, which contains a starlarkified local_config_platform Bazel module offering a host_platform module extension with a tag add_constraints that modules can use to specify a .bzl file with additional constraints.

https://github.com/fmeum/local_config_platform/tree/main/tests/bcr contains an example module that depends on a module foo which adds a synthetic constraint here.

@katre Does this look reasonable? I'm open to providing a PR and/or a design doc. This could even be maintained outside the Bazel core as an independent Bazel module - although that would require users to point --platforms to the platform defined by this module.

@katre
Copy link
Member

katre commented Aug 8, 2022

@fmeum This is very cool, and is basically what I was thinking of. I'd like to read a design doc (hopefully with some pointers to the bzlmod docs, because I am not full up to date on that), but this is definitely looking interesting.

@alexeagle
Copy link
Contributor

FYI we discussed in the SIG meeting that the group is fine funding Fabian's time to write that design doc.

@katre
Copy link
Member

katre commented Dec 20, 2022

This is very exciting, I'm looking forward to what comes out.

@cameron-martin
Copy link
Contributor

@fmeum How would this example work if you wanted the inspect the host to decide which platform constraints to add?

@fmeum
Copy link
Collaborator

fmeum commented Mar 21, 2023

@cameron-martin You can pass the label of a generated constraints.bzl file to the module extension. The local_config_platform module contains some helper functions for this.

@cameron-martin
Copy link
Contributor

I see this now. Looks great 👍

@brentleyjones
Copy link
Contributor

@fmeum What's the progress on the design document? I think being able to add constraints, such as target_vendor from apple_support, to the host platform would be very valuable.

@fmeum
Copy link
Collaborator

fmeum commented Mar 6, 2024

@brentleyjones @cgrindel and everyone else with a use case, could you briefly describe what you would use this feature for in a comment?

I can finally start working on the doc in April.

@brentleyjones
Copy link
Contributor

The main thing I want to solve is the non-deterministic toolchain resolution surrounding the apple_support cc_toolchain. We added the target_vendor constraint to the apple_support platforms, but we had to give it a default value to allow it to match the auto-generated host platform. Ideally we could have that host platform have a value set for that constraint if there is a dependency on apple_support and the OS is in fact macOS.

@cameron-martin
Copy link
Contributor

@fmeum Our company creates hardware and some tests require that hardware so have an execution constraint specifying so, mainly for the purpose of remote execution. However for local execution I would like to modify the host platform to add this constraint if the host has this hardware.

@cgrindel
Copy link

cgrindel commented Mar 6, 2024

We have a client that has targets which rely on a GPU. However, not all of the developer machines have the GPU. We are adding a custom constraint to thetarget_compatible_with for those targets. Ideally, we would be able to detect whether the GPU is present on the host and add a constraint to the platform defined in @local_config_platform.

@Wyverald
Copy link
Member

All interested parties: please see design doc https://docs.google.com/document/d/1g5JAAOfLsvQKBGqzSLFp1hIYFoQsgOslsjaIGV6P7Tk/edit

copybara-service bot pushed a commit that referenced this issue Mar 29, 2024
* Upgrade to `platforms` 0.0.9
* `--host_platform` now defaults to `@bazel_tools//tools:host_platform`, which is an alias of `@platforms//host`
* `local_config_platform` (the repo rule) now just outputs a thin wrapper; `@local_config_platform//:host` is an alias of `@platforms//host`, and `@local_config_platform//:constraints.bzl` re-exports `@platforms//host:constraints.bzl`
* Removed all test mocks of `local_config_platform`. A follow-up will guard prod usage behind a flag.

Work towards #8766 and #18285.

PiperOrigin-RevId: 620316973
Change-Id: I6593f62569f31faee69e88a520a0f7f42009e05d
fweikert pushed a commit to fweikert/bazel that referenced this issue Apr 8, 2024
* Upgrade to `platforms` 0.0.9
* `--host_platform` now defaults to `@bazel_tools//tools:host_platform`, which is an alias of `@platforms//host`
* `local_config_platform` (the repo rule) now just outputs a thin wrapper; `@local_config_platform//:host` is an alias of `@platforms//host`, and `@local_config_platform//:constraints.bzl` re-exports `@platforms//host:constraints.bzl`
* Removed all test mocks of `local_config_platform`. A follow-up will guard prod usage behind a flag.

Work towards bazelbuild#8766 and bazelbuild#18285.

PiperOrigin-RevId: 620316973
Change-Id: I6593f62569f31faee69e88a520a0f7f42009e05d
Wyverald added a commit that referenced this issue Apr 25, 2024
* Upgrade to `platforms` 0.0.9
* `--host_platform` now defaults to `@bazel_tools//tools:host_platform`, which is an alias of `@platforms//host`
* `local_config_platform` (the repo rule) now just outputs a thin wrapper; `@local_config_platform//:host` is an alias of `@platforms//host`, and `@local_config_platform//:constraints.bzl` re-exports `@platforms//host:constraints.bzl`
* Removed all test mocks of `local_config_platform`. A follow-up will guard prod usage behind a flag.

Work towards #8766 and #18285.

PiperOrigin-RevId: 620316973
Change-Id: I6593f62569f31faee69e88a520a0f7f42009e05d
Wyverald added a commit that referenced this issue Apr 29, 2024
* Upgrade to `platforms` 0.0.9
* `--host_platform` now defaults to `@bazel_tools//tools:host_platform`, which is an alias of `@platforms//host`
* `local_config_platform` (the repo rule) now just outputs a thin wrapper; `@local_config_platform//:host` is an alias of `@platforms//host`, and `@local_config_platform//:constraints.bzl` re-exports `@platforms//host:constraints.bzl`
* Removed all test mocks of `local_config_platform`. A follow-up will guard prod usage behind a flag.

Work towards #8766 and #18285.

PiperOrigin-RevId: 620316973
Change-Id: I6593f62569f31faee69e88a520a0f7f42009e05d
Wyverald added a commit that referenced this issue Apr 30, 2024
* Upgrade to `platforms` 0.0.9
* `--host_platform` now defaults to `@bazel_tools//tools:host_platform`, which is an alias of `@platforms//host`
* `local_config_platform` (the repo rule) now just outputs a thin wrapper; `@local_config_platform//:host` is an alias of `@platforms//host`, and `@local_config_platform//:constraints.bzl` re-exports `@platforms//host:constraints.bzl`
* Removed all test mocks of `local_config_platform`. A follow-up will guard prod usage behind a flag.

Work towards #8766 and #18285.

PiperOrigin-RevId: 620316973
Change-Id: I6593f62569f31faee69e88a520a0f7f42009e05d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Someone outside the Bazel team could own this P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request under investigation
Projects
None yet
Development

No branches or pull requests