bazelbuild / bazel

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

$DEVELOPER_DIR stopped getting set on macOS when using C++ toolchain environment #20638

Open illicitonion opened 9 months ago

illicitonion commented 9 months ago

Description of the bug:

When looking up action environments from a C++ toolchain, in Bazel 6.4.0 this would lead to $XCODE_VERSION_OVERRIDE, $APPLE_SDK_VERSION_OVERRIDE and $APPLE_SDK_PLATFORM getting set.

This would then lead to the XcodeLocalEnvProvider setting $DEVELOPER_DIR and $SDKROOT when running in a DarwinSandboxedSpawnRunner.

https://github.com/bazelbuild/bazel/commit/699e40373f95e42390a85f29dfa1098636336103 moved the C++ toolchain for Apple platforms out of Bazel, and has brought a regression because these variables are now not set automatically.

This is a significant break in the rules API - if there's some workaround that should be applied (e.g. a new attribute that needs setting), it would be good to document that.

As far as I can tell from searching around, any replacement ways of making this work involve manually setting env vars, and possibly registering new repository rules to do so, which feels like a significant regression. Hopefully there's some easy workaround or fix I just haven't found yet!

Which category does this issue belong to?

C++/Objective-C Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Clone https://github.com/illicitonion/repro-bazel-cc-toolchain-xcode-vars on a macOS machine.

Run:

% USE_BAZEL_VERSION=6.4.0 bazel build //:dd

To see it work.

Run:

% USE_BAZEL_VERSION=7.0.0 bazel build //:dd

to see it fail.

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.0.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 master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

https://github.com/bazelbuild/bazel/commit/699e40373f95e42390a85f29dfa1098636336103

Have you found anything relevant by searching the web?

No response

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

Note that this is the root cause of https://github.com/bazelbuild/rules_rust/issues/2207

illicitonion commented 9 months ago

/cc @keith @brentleyjones

keith commented 9 months ago

Interesting yea. The solution here is to depend on the apple_support toolchain since now the C++ toolchain doesn't do anything specific for Xcode

illicitonion commented 9 months ago

The solution here is to depend on the apple_support toolchain

Does that transparently forward to a C++ toolchain if used on Linux? Or will we need to write "if macOS" code in the rules to handle that we may need to get our C++ toolchain from two different toolchain types?

keith commented 9 months ago

yes that toolchain is just ignored on linux

fmeum commented 9 months ago

@keith Could you clarify which of the following statements are correct? I'm trying to be as precise as possible here, so please let me know if I missed any requirements.

  1. A ruleset that doesn't rely on any macOS-specific functionality, but needs to perform C/C++ compilation, can continue to use the cc_common API and ctx.actions.run as long as it sets env to whatever cc_common.get_environment_variables returns as long as the user is using the apple_support toolchain.
  2. A ruleset that doesn't rely on any macOS-specific functionality, but needs to perform C/C++ compilation and requires custom passing of environment variables (see e.g. rules_graalvm) needs to go through apple_support.run.
  3. The Bazel-provided generic Unix toolchain works with Homebrew installations of vanilla LLVM toolchains.

If all three are correct, then as far as I understand rulesets wouldn't have to do anything special, but macOS users will likely have to register the apple_support toolchain for everything to "just work".

illicitonion commented 9 months ago

I think your summary is currect @fmeum (though I haven't validated the Homebrew install question).

A question I'd like to answer in response to this is: Whose responsibility is it to register that toolchain?

I think that things like rules_rust and rules_go should work out of the box on "standard" machine configurations, and I think "running on macOS with Xcode installed" is a reasonably standard machine configuration. In particular, I found this problem because bazelci was broken for rules_rust which feels like a very standard configuration that should be well-supported.

Some answers I can see, in descending order of my personal preference:

  1. Bazel should maybe in an apple_support and register its toolchain, like how it maybes in rules_cc if it's not explicitly registered. I like that this Just Works, but maintains the decoupling that Apple-specific knowledge is abstracted away from Bazel itself and can be updated out of band.
  2. Any ruleset which may invoke any tool which may use Xcode if it's installed should depend on apple_support. This feels like a (small) burden for rulesets, and is easy to overlook, but if we have clear guidance on it doesn't feel like the end of the world.

I don't think it's correct for us to require that every consumer of rules_rust/rules_go themselves register apple_support if they or any of their users happen to be running macOS with Xcode installed.

keith commented 9 months ago

as long as it sets env to whatever cc_common.get_environment_variables returns as long as the user is using the apple_support toolchain.

This part isn't a blanket requirement, it depends on what you're doing if you need this. If you require DEVELOPER_DIR or SDKROOT or any other apple specific thing like this then yes you'd need the apple_support toolchain.

The Bazel-provided generic Unix toolchain works with Homebrew installations of vanilla LLVM toolchains.

Because of what I said above the default toolchain does also work fine with Xcode.

A question I'd like to answer in response to this is: Whose responsibility is it to register that toolchain?

So bazel is actually registering the apple_support toolchain now, but it's doing so after the default toolchain, and first one wins (I don't think we'd want to flip this because then we'd have the same problem but for users who didn't want the Xcode requirement). We had a similar discussion on https://github.com/bazelbuild/bazel/issues/20537 this week, but the rules_rust case does feel worse than the example there where the user explicitly depended on rules_apple.

I don't think it's correct for us to require that every consumer of rules_rust/rules_go themselves register apple_support if they or any of their users happen to be running macOS with Xcode installed.

I agree, in the other issue the potential solutions we came up were:

anyone have some other good ideas on a potential solution?

brentleyjones commented 9 months ago

https://bazelbuild.slack.com/archives/CD3QY5C2X/p1703259606000369?thread_ts=1703173291.572179&channel=CD3QY5C2X&message_ts=1703259606.000369 and the comments before it. We can have the rulesets coordinate and only register one or the other.