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

Allow rules to detect tool configuration #14444

Open
rickeylev opened this issue Dec 16, 2021 · 26 comments
Open

Allow rules to detect tool configuration #14444

rickeylev opened this issue Dec 16, 2021 · 26 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request

Comments

@rickeylev
Copy link
Contributor

rickeylev commented Dec 16, 2021

Description of the problem / feature request:

Rule should be able to detect host/exec config

Feature requests: what underlying problem are you trying to solve with this feature?

Specifically, skipping --stamp behavior in host/exec config, and essentially reimplementing isToolConfig() from <can't remember the Java class in Bazel>

What operating system are you running Bazel on?

Linux

What's the output of bazel info release?

n/a

Have you found anything relevant by searching the web?

Closest related I could find are:

  • 11164: starlark observing --stamp value
  • Internal FR 170920173 (allow aspects to detect host/exec mode)

Today, this can be worked around by looking at ctx.bin_dir.path. For host most, it'll contain /host/, and exec mode will contain -exec-, but both of these seem like fragile ways to detect the config.

@aiuto
Copy link
Contributor

aiuto commented Dec 17, 2021

Let me see if I can rephrase the goal so I understand better.

  • you want to do bazel build --stamp //my:app
  • there are tools needed to make the above happen. But you don't want them to be time stampped.

I can see the value in that - why would you need a timestamp on the tools. Perhaps we can do that with a constraint on the exec platform. You could select on the constraint.

@rickeylev
Copy link
Contributor Author

Correct.

How do I select/detect the exec constraint? Or is that still a FR?

@aiuto
Copy link
Contributor

aiuto commented Dec 18, 2021

Still an FR, but it is one for Bazel. It needs to be done for each organiations build environment. They need to add somethig to their platform definitions.
FWIW, there is an internal thread in Google about exactly this kind of thing. It seems we are all converging on this.

@rickeylev
Copy link
Contributor Author

Another case that select()'ing on the tool config would make easier: A feature we're looking to add to the Python rules is having a flag that specifies what debugger to include into the deps of a binary. For tools, this extra dependency isn't necessary (and can contribute to causing circular dependencies). If we could do select(tool_config: [], default: //foo:bar), that would largely solve the problem.

@teeler
Copy link

teeler commented Aug 16, 2022

We ended up working around this in an aspect by inspecting ctx.bin_path, eg:

if '/host/' in ctx.bin_dir.path:
   # ... host config

(h/t @rickeylev )

@aiuto aiuto added team-Configurability platforms, toolchains, cquery, select(), config transitions untriaged labels Aug 17, 2022
@aiuto
Copy link
Contributor

aiuto commented Aug 17, 2022

I put it back on our team so we can see it in the next triage.

@teeler
Copy link

teeler commented Aug 17, 2022 via email

@aiuto
Copy link
Contributor

aiuto commented Aug 17, 2022

I'm not sure why we don't expost that. Perhaps, is_exec configuration would be more aligned with cfg=exec.

@katre Any thoughts?

@katre
Copy link
Member

katre commented Aug 19, 2022

There has been some discussion about how to handle exec transitions with Starlark flags: this would handle the issue of --stamp if --stamp were a Starlark flag. @gregestren, can you provide an update or a link to an issue?

However, --stamp is a native flag, and we already have support for that, and are in fact doing that: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java;drc=65388c330141736d505cec50b4cc02a5d65ed5de;l=938

So in an exec configuration, --stamp should already be disabled.

Was that simply an example, or are you actually having issues with tool binaries being stamped inappropriately?

I am very reluctant to expose the isToolConfiguration check to Starlark, because it tends to make rules brittle and difficult to reason about: we already have many cases where it's impossible to test rule interactions due to transitions and this will just make that even more so.

@fmeum
Copy link
Collaborator

fmeum commented Aug 19, 2022

@katre Whenever I thought I wanted ctx.is_tool_configuration, what I really wanted was a way to specify how a Starlark build setting's value changes when transitioning to the exec configuration. It would be great to have APIs for that.

@teeler
Copy link

teeler commented Aug 20, 2022

@katre a small anecdote to illustrate my difficulty.

is_tool_configuration (or something like it) is critical to understanding license compliance as it propagates through the graph.

Dependencies can carry different license obligations depending on how they're used. For example, as a compiler versus a runtime library (most compilers do not impose their licenses onto artifacts they produce, but we need to be able to at least detect the relationship).

@katre
Copy link
Member

katre commented Aug 29, 2022

@aiuto for license checking updates.

@teeler What are you using to check? An aspect? Another rule? Some sort of wrapper around blaze query?

@teeler
Copy link

teeler commented Aug 29, 2022

@katre an aspect (the license-checking aspect in particular…Tony and I have discussed this also).

I mean, ideally an aspect would be able to fully inspect a rule definition (eg inspect the properties of each of a rules attributes), this is just a proxy for that.

@aiuto
Copy link
Contributor

aiuto commented Aug 30, 2022 via email

@katre katre added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Sep 1, 2022
@katre
Copy link
Member

katre commented Sep 1, 2022

Thanks for all the feedback on this. I see the points raised, but unfortunately we aren't going to be able to prioritize this in the near future. If someone else wants to pick it up, I would like to see a design proposal before a PR, in order to ensure that the feature is cleanly implemented and interacts properly with other features (such as selects).

@gregestren
Copy link
Contributor

There has been some discussion about how to handle exec transitions with Starlark flags: this would handle the issue of --stamp if --stamp were a Starlark flag. @gregestren, can you provide an update or a link to an issue?

#13048 is the closest I know. I don't think we have a specific issue for the generalized API idea?

@katre
Copy link
Member

katre commented Oct 7, 2022

After thinking some more, I still think that the ability to select on "is tool configuration" is problematic: in general, if tools are being built with special logic and configuration, it becomes very difficult to test and debug them (since any testing has to be in the context of a full build that uses the tool).

Ideally, it would be possible to build any tool directly just by building for the correct execution platform, thus making it uch easier to investigate misbehaving builds.

@brentleyjones
Copy link
Contributor

@katre Here is another example of needing to know if we are building for the exex configuration or not: emulating flags like --host_swiftcopt: https://github.com/bazelbuild/rules_swift/pull/1139/files#r1406789259

@gregestren
Copy link
Contributor

gregestren commented Dec 4, 2023

@katre Whenever I thought I wanted ctx.is_tool_configuration, what I really wanted was a way to specify how a Starlark build setting's value changes when transitioning to the exec configuration. It would be great to have APIs for that.

This is, I think, a better model I hope we can adapt.

Since this comment we've fully starlarkized Bazel's exec transition (post-7.0), as part of standard exec configs.

There's currently no user-visible difference. What this does is free us from Bazel's internal hard-coded Java logic, just like any other native -> Starlarkization work. That opens up a path toward new and creative APIs to model what exec configs look like for different users, different builds, etc. I'm hoping it makes --exec_* / --host_* flags obsolete.

We're currently on phase 2: limit which --defines and Starlark flags propagate from target to exec configs. So exec configs don't fork pointlessly and make build graphs pointlessly larger. Right now we're exploring an unsupported hack API for this but we'd quickly have to follow up with a more usable API.

@goffrie
Copy link

goffrie commented Feb 15, 2024

I notice that there are already a number of hacks out there looking for "-exec-" in ctx.bin_dir to detect the exec configuration.
But with --experimental_platform_in_output_dir enabled and --experimental_exec_configuration_distinguisher=off (the latter of which is now the default), output dirs look something like bazel-out/osx_aarch64-opt-exec/. Which is nice and clean, but it notably does not contain -exec-. So the hacks are breaking.
I also see for example that bazelbuild/rules_rust#2349 recently showed up to work around this issue by changing -exec- to -exec, but this seems just as brittle. To me that underscores the need for this feature.

@katre
Copy link
Member

katre commented Oct 11, 2024

After discussion with the team, we've decided that (despite my misgivings in #14444 (comment)) we should prioritize getting a design and fixing this.

From internal discussions (and my memory), there are actually a few points that need to be addressed, and may all be orthogonal to each other:

  1. Rule implementations may want to know whether they are in a tool configuration.
    1. This is probably a context method like ctx.is_tool_configuration but might be more or less complex.
  2. Selects may want to change behavior based on the configuration.
    1. This might be an entirely new syntax, or a new magic key similar to //conditions:default (//conditions:tool_config?)
    2. This may or may not interact with ideas to allow more expressiveness in selects in general
      1. My suggestion was allowing custom rules to provide ConfigMatchingProvider, and use those in select keys
      2. I believe @lberki or @comius had a different idea
  3. Transitions may also want to be able to condition on this
    1. This might be as simple as exposing --is exec configuration to transitions, but that might also be complex
    2. Should we upgrade transitions to a full transition_ctx? The current (settings, attr) approach is already problematic.

Any proposals should address at least one, and ideally all three of these, but as you can see, they probably all have different mechanisms.

If anyone has any great ideas feel free to come forward, otherwise the Configurability team will try to prioritize this in Q4 or 2025 Q1.

@fmeum
Copy link
Collaborator

fmeum commented Oct 11, 2024

It would be very helpful to have a survey of potential (internal and external) use cases for this feature, especially those that aren't solved by allowing Starlark flags to specify how they should be propagated to the exec configuration. Maybe now is the time to create a GitHub discussion or doc to start collecting them?

@aiuto
Copy link
Contributor

aiuto commented Oct 11, 2024

The thing I am looking for is that an aspect can look at it's inputs and see if any of them are in the tool configuration. That way something scanning on '*' attributes can prune tool trees from the rest. Essentially, being able to get the equivalent of bazel cquery --notool_deps --noimplicit_deps ... in my own aspect.

@fmeum
Copy link
Collaborator

fmeum commented Oct 11, 2024

Are there any cases in which one would reasonably want to do this for rules rather than aspects? I'm thinking that restricting this new functionality to aspects could give us the benefits without making ordinary rules more complex.

@katre
Copy link
Member

katre commented Oct 11, 2024

@fmeum I recall some examples of rules behaving differently in the exec configuration.

One example is that Rust has an is_exec_configuration utility, but I can't get github to show me where it's used.

@rickeylev
Copy link
Contributor Author

use cases

Off the top of my head:

  • grep for python_is_target_config_internal_only_do_not_use within google for some cases that pre-date the setting in CoreOptions
  • Most of those uses were to avoid prod dependencies in exec tools
  • In the Python rules, we use it for similar: binaries have an implicit dep on a target that provides debugger client libraries. To prevent cycles and unnecessary deps, we make it an empty target for exec config. We also use the ctx.is_tool_config helper to guard against and minimize unnecessary execution-phase/runtime deps (in the case they aren't avoided at loading/analysis time).

My suggestion was allowing custom rules to provide ConfigMatchingProvider, and use those in select keys

FWIW, this sort of already exists today: config_common.FeatureFlagInfo. It basically allows a rule to, at analysis time, decide the value that config_setting() receives when matched. Thus, one can do something like:

def is_exec_config_impl(ctx):
  return FeatureFlagInfo(value="exec" in ctx.bin_dir.path)

config_setting(flag_values={":is_exec_config": True})

That said, ConfigMatchingInfo sounds nice -- makes this into more of a first-class feature (I think there are various things in the CC rules that do special things to allow config-matching of things that aren't necessarily just flags; might provide a way to more cleanly accomodate things like selects.config_setting_group or other "interesting" things rules do for config-state matching).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Development

No branches or pull requests

9 participants