wrapped_clang is not built with the requested Xcode versions #8902

keith · 2019-07-16T01:26:56Z

Description of the problem / feature request:

Currently wrapped_clang is built as part of the local_config_cc setup:

Lines 111 to 121 in 0b5f341

    
           xcrun_result = repository_ctx.execute([ 
        
               "env", 
        
               "-i", 
        
               "xcrun", 
        
               "clang", 
        
               "-std=c++11", 
        
               "-lc++", 
        
               "-o", 
        
               "wrapped_clang", 
        
               wrapped_clang_src_path, 
        
           ], 30)

Currently it doesn't take into account what you pass for --xcode_version. Because of this if you change your xcode-select on macOS and build with a different version of Xcode than is requested with this flag, you produce a different wrapped_clang. This ends up breaking remote cache compatibility with other versions even if the build would otherwise be done with the correct Xcode version.

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

sudo xcode-select -s /Applications/Xcode-10.2.1.app
bazel build some_ios_target --xcode_version=10.2.1 --remote_http_cache=...
bazel clean --expunge
sudo xcode-select -s /Applications/Xcode-11beta3.app
bazel build some_ios_target --xcode_version=10.2.1 --remote_http_cache=...

With the second build, you will get nearly no remote cache hits because wrapped_clang differs as an input to every action, even though you requested the same Xcode version from the command line.

What operating system are you running Bazel on?

macOS

What's the output of `bazel info release`?

release 0.28.0

The text was updated successfully, but these errors were encountered:

keith · 2019-07-16T01:29:52Z

This would be less of an issue if changing Xcode versions globally with xcode-select invalidated wrapped_clang, but this requires a clean --expunge to recover from

keith · 2019-08-16T03:06:51Z

@scentini do you have a suggestion for how to fix this? Having to clean --expunge whenever switching versions here is a pain for our large team

scentini · 2019-08-23T12:45:58Z

cc/ @hlopko

keith · 2019-11-21T00:13:03Z

@hlopko do you have any suggestion for how to fix this?

brentleyjones · 2020-07-06T17:36:51Z

This should also potentially consider the DEVELOPER_DIR environment variable, per my duped issue: #11716.

keith · 2020-07-06T17:40:46Z

Would passing that through cause invalidation when the user's Xcode version changed? Maybe if we correctly accessed it in the workspace rule it could help?

brentleyjones · 2020-07-06T17:42:32Z

If the user set their Xcode version that way, then yes. But if all they did was change xcode-select -s, I don't believe so.

keith · 2020-07-06T17:44:10Z

Yea, I was at least thinking that in your bazel wrapper you could always specify that var if it would fix this

brentleyjones · 2020-07-06T17:45:02Z

Ohh, yeah, it could read the set value and then export it. The workspace rule can state that it should invalidate based on the environment variable. I think that would work nicely.

It would differ from --xcode_version though.

Resolves #11716, which partially addresses #8902. With this change, and some careful environment manipulation (probably in a wrapper script) you can ensure that `wrapped_clang` is built with the correct version of Xcode. Closes #11719. PiperOrigin-RevId: 338465098

oquenchil · 2020-11-19T19:19:58Z

If the PR above didn't fix this issue, please re-open.

brentleyjones · 2020-11-19T19:38:45Z

We can specify the DEVELOPER_DIR in a bazel wrapper now after #11719, but I'm not sure if @keith's original issue was fixed.

keith · 2020-11-19T19:41:33Z

Using DEVELOPER_DIR in a wrapper around bazel definitely works around the original issue, but it would be nice if it was solved for everyone without having to manually do that.

keith · 2021-01-05T21:46:25Z

One thing I realized with the DEVELOPER_DIR workaround is that if the user overwrites their Xcode.app with a newer version, the DEVELOPER_DIR will not change, but the version of Xcode will. So that's a case where this workaround isn't enough 😞

jerrymarino · 2021-01-11T21:17:41Z

@keith one way to sidestep this is to just use a custom local_config_cc. e.g.

bazel fetch :*  && ditto $(bazel info execution_root)/../../external/local_config_cc tools/local_config_cc

# WORKSPACE
local_repository(name="local_config_cc", path="tools/local_config_cc")

YMMV for different host arch than x86_64 🤔

It might be possible to factor out the binaries into their own input to the cc rules rather than compiling in the repo rule, but I don't know the long term plans for this / overlap with rules_cc off the top of my head!

keith · 2021-01-11T21:44:53Z

copying that in is definitely an interesting idea, we also run some things in our build on Linux so I think we'd have to do that in a cross platform way though. Definitely handling the arch problem (although you could check fat binaries in there instead if you only supported macOS) would be necessary too

I think doing the input route is easier said than done because it'd be a bit of a circular dependency

jerrymarino · 2021-01-11T23:22:01Z

@keith in order to make the binary an input to the toolchain w/o a circle you'll need another rule to compile it: e.g. a rule that consumes only the relevant xcode config bits and not the custom toolchain.

As the toolchain will be specific to macOS, you might need to override local_config_cc conditionally when on macOS to use the hardcoded one. You might be able to get away with switching the entire generated toolchain with --override_repository instead of having to create a cross platform one.

jerrymarino · 2021-01-11T23:59:56Z

For what it's worth, it seems like making wrapped_clang binaries an input to the toolchain seems like good way to fix this outside of forking the generated one

This compiles wrapped_clang and wrapped_clang_pp inside of an action. bazelbuild#8902 has a larger definition of the issue. This action needs the DEVELOPER_DIR as an input in order to select the correct clang. This is passed in from apple_common.XcodeVersionConfig. Note that this works for local execution and needs to address the TODO for cc_wrapper.sh in tools/cpp/osx_cc_configure.bzl.

jerrymarino · 2021-02-10T18:25:03Z

I've put up a WIP commit to test out compiling the wrappers with a rule that depends on the Xcode env. It looks like we can compile this with the apple_common.apple_host_system_env( and then point the tool path at the output of the rule.

It seems like this is the right approach - plus fixing the last TODO is addressed on that commit. Does anyone see any downsides to compiling this with a rule? If not I can prepare that commit for a PR cc @keith @hlopko

brentleyjones · 2021-02-10T18:33:15Z

apple_host_system_env is nice, since it includes XCODE_VERSION_OVERRIDE, would would be different for different Xcode versions, even with the same DEVELOPER_DIR.

jerrymarino · 2021-02-12T18:40:53Z

@brentleyjones yep passing the entire Xcode env is how I was looking to solve it for good. This is similar to how other C compiler rules work 👍 #11716 alone won't help the use case where people use Xcode mac app store to install Xcode to `/Applications/Xcode.app

The 3 ways I know of to fix this are:

clean the repo cache when the Xcode version changes
manage the toolchain binaries in another way ( e.g. commit them ) - likely this is what others are doing
compile them with the apple_host_system_env and pass paths into the toolchain

This is three changes: - On the command-line, set `--xcode_version` to whatever `xcode-select` is set to - In Xcode, set `--xcode_version` to `$XCODE_PRODUCT_BUILD_VERSION` - For both, set `--repo_env=USE_CLANG_CL= $XCODE_PRODUCT_BUILD_VERSION` to work around bazelbuild/bazel#8902

While `bazel_build.sh` should still have `--xcode_version`, to ensure that Bazel uses the version of Bazel you are building inside of, setting `--xcode_version` based on `xcode-select` can counter-act some people's use of `--xcode_config`. We leave the `--repo_env` though to at least try to fix bazelbuild/bazel#8902.

While `bazel_build.sh` should still have `--xcode_version`, to ensure that Bazel uses the version of Xcode you are building inside of, setting `--xcode_version` based on `xcode-select` can counter-act some people's use of `--xcode_config`. We leave the `--repo_env` though to at least try to fix bazelbuild/bazel#8902.

…iveFoundation#1099) This is three changes: - On the command-line, set `--xcode_version` to whatever `xcode-select` is set to - In Xcode, set `--xcode_version` to `$XCODE_PRODUCT_BUILD_VERSION` - For both, set `--repo_env=USE_CLANG_CL= $XCODE_PRODUCT_BUILD_VERSION` to work around bazelbuild/bazel#8902

While `bazel_build.sh` should still have `--xcode_version`, to ensure that Bazel uses the version of Xcode you are building inside of, setting `--xcode_version` based on `xcode-select` can counter-act some people's use of `--xcode_config`. We leave the `--repo_env` though to at least try to fix bazelbuild/bazel#8902.

brentleyjones · 2022-10-26T19:52:25Z

To recap since this has bitten me again: if you have --xcode_version set, you need to make sure DEVELOPER_DIR or xcode-select -s is set to the same version. Otherwise your wrapped_clang binary might mismatch. On a mismatch you probably won't get any remote cache hits.

We probably want a fix that uses apple_host_system_env as mentioned above.

github-actions · 2024-01-01T01:31:21Z

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

brentleyjones · 2024-01-02T15:51:13Z

@bazelbuild/triage not stale

keith changed the title ~~Build wrapped_clang with different Xcode versions invalidates cache~~ wrapped_clang is not built with the requested Xcode versions Jul 16, 2019

iirina added team-Rules-CPP Issues for C++ rules untriaged labels Jul 16, 2019

scentini added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Jul 22, 2019

scentini added P2 We'll consider working on this in future. (Assignee optional) and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Aug 23, 2019

keith mentioned this issue Jul 6, 2020

wrapper_clang should be compiled with DEVELOPER_DIR in the environment #11716

Closed

brentleyjones mentioned this issue Jul 6, 2020

Consider DEVELOPER_DIR when building wrapped_clang #11719

Closed

oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) type: bug and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 19, 2020

oquenchil closed this as completed Nov 19, 2020

oquenchil reopened this Nov 23, 2020

brentleyjones mentioned this issue Jan 11, 2022

External repositories don't regenerate in response to XCode changes #14551

Open

brentleyjones mentioned this issue Sep 14, 2022

Ensure Bazel uses the correct Xcode version when compiling MobileNativeFoundation/rules_xcodeproj#1099

Merged

brentleyjones mentioned this issue Sep 14, 2022

Revert runner set --xcode_version MobileNativeFoundation/rules_xcodeproj#1100

Merged

oquenchil added the platform: apple label Oct 27, 2022

github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jan 1, 2024

iancha1992 added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Jan 2, 2024

wrapped_clang is not built with the requested Xcode versions #8902

wrapped_clang is not built with the requested Xcode versions #8902

Comments

keith commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the problem / feature request:

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

What operating system are you running Bazel on?

What's the output of bazel info release?

keith commented Jul 16, 2019

Uh oh!

keith commented Aug 16, 2019

Uh oh!

scentini commented Aug 23, 2019

Uh oh!

keith commented Nov 21, 2019

Uh oh!

brentleyjones commented Jul 6, 2020

Uh oh!

keith commented Jul 6, 2020

Uh oh!

brentleyjones commented Jul 6, 2020

Uh oh!

keith commented Jul 6, 2020

Uh oh!

brentleyjones commented Jul 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oquenchil commented Nov 19, 2020

Uh oh!

brentleyjones commented Nov 19, 2020

Uh oh!

keith commented Nov 19, 2020

Uh oh!

keith commented Jan 5, 2021

Uh oh!

jerrymarino commented Jan 11, 2021

Uh oh!

keith commented Jan 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jerrymarino commented Jan 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jerrymarino commented Jan 11, 2021

Uh oh!

jerrymarino commented Feb 10, 2021

Uh oh!

brentleyjones commented Feb 10, 2021

Uh oh!

jerrymarino commented Feb 12, 2021

Uh oh!

brentleyjones commented Oct 26, 2022

Uh oh!

github-actions bot commented Jan 1, 2024

Uh oh!

brentleyjones commented Jan 2, 2024

Uh oh!

keith commented Jul 16, 2019 •

edited

Loading

What's the output of `bazel info release`?

brentleyjones commented Jul 6, 2020 •

edited

Loading

keith commented Jan 11, 2021 •

edited

Loading

jerrymarino commented Jan 11, 2021 •

edited

Loading