bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.02k stars 4.03k forks source link

Allow rules to detect tool configuration #14444

Open rickeylev opened 2 years ago

rickeylev commented 2 years ago

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:

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 commented 2 years ago

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

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 commented 2 years ago

Correct.

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

aiuto commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

teeler commented 2 years ago

Tony - do you think we could expose is_tool_configuration to non-builtins?

(That would help with the license aspect as well!)

On Tue, Aug 16, 2022 at 7:09 PM aiuto @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazel/issues/14444#issuecomment-1217375720, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALCU7ZDAEG2E4WWUXA7HTVZRCXDANCNFSM5KHFYR4Q . You are receiving this because you commented.Message ID: @.***>

aiuto commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@aiuto for license checking updates.

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

teeler commented 2 years ago

@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 commented 2 years ago

+1 to what @teeler's point. The ideal case is that we can tell a source dep from a data dep, from a tool dep based on properties of the attribute declaration itself. And, it's not just those 3, we would eventually want nuances like "this data is for running the artifact, but that data is user-replaceable configuration files"

Until that day, simply knowing of something is a tool vs a dep that gets linked in is a good proxy.

On Mon, Aug 29, 2022 at 11:07 AM Tyler @.***> wrote:

@katre https://github.com/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.

— Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazel/issues/14444#issuecomment-1230444031, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXHHHCCTMRYHT4IR2DIVSDV3TGZ5ANCNFSM5KHFYR4Q . You are receiving this because you were mentioned.Message ID: @.***>

katre commented 2 years ago

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 commented 2 years ago

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?

https://github.com/bazelbuild/bazel/issues/13048 is the closest I know. I don't think we have a specific issue for the generalized API idea?

katre commented 1 year ago

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 commented 9 months ago

@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 commented 9 months ago

@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 commented 7 months ago

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 https://github.com/bazelbuild/rules_rust/pull/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.