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

XcodeConfigInfo.macosMinimumOsVersion is incorrectly set for host configuration #12988

Closed
brentleyjones opened this issue Feb 10, 2021 · 8 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: apple team-Rules-CPP Issues for C++ rules type: bug

Comments

@brentleyjones
Copy link
Contributor

brentleyjones commented Feb 10, 2021

Description of the problem / feature request:

When building a target with the host configuration, the macosMinimumOsVersion is being set to the SDK version instead of what is set for --macos_minimum_os. The relevant lines are:

DottedVersion macosMinimumOsVersion =
(appleOptions.macosMinimumOs != null)
? DottedVersion.maybeUnwrap(appleOptions.macosMinimumOs)
: macosSdkVersion;

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

Build a genrule that has a swift_binary tool, with --macos_minimum_os lower than the SDK version (say 10.15 when using Xcode 12.4.0, which has its SDK version at 11.1). The tool will have an incorrect -target (x86_64-apple-macosx11.1 for the example given).

What operating system are you running Bazel on?

macOS 11.2.1

What's the output of bazel info release?

release 4.0.0

Have you found anything relevant by searching the web?

Nothing.

@keith
Copy link
Member

keith commented Feb 10, 2021

I think the issue is that the minimum version isn't set for the host configuration here?

// Set options needed in the host configuration.
host.xcodeVersionConfig = xcodeVersionConfig;
host.xcodeVersion = xcodeVersion;
host.iosSdkVersion = iosSdkVersion;
host.watchOsSdkVersion = watchOsSdkVersion;
host.tvOsSdkVersion = tvOsSdkVersion;
host.macOsSdkVersion = macOsSdkVersion;
host.appleBitcodeMode = appleBitcodeMode;
// The host apple platform type will always be MACOS, as no other apple platform type can
// currently execute build actions. If that were the case, a host_apple_platform_type flag might
// be needed.
host.applePlatformType = PlatformType.MACOS;
host.configurationDistinguisher = ConfigurationDistinguisher.UNKNOWN;

But maybe that's intentional

@brentleyjones
Copy link
Contributor Author

If it's intentional, could we get a --host_minimum_os flag? Hopefully it's not intentional though 😄.

@brentleyjones
Copy link
Contributor Author

I just realized this might be semi-intentional, since we only need to support a minimum OS of the host, not any lower. In my example though, by using Xcode 12.4, the macOsSdkVersion is 11.1, while my OS version (in a previous test) was 10.15. Ideally we wouldn't try targeting an OS version higher than the current one?

@keith
Copy link
Member

keith commented Feb 11, 2021

minimum OS of the host

To ensure caching across developers even this isn't particularly true is it? I get that you won't back deploy the binary directly, but it could result in a different output

@brentleyjones
Copy link
Contributor Author

To ensure caching across developers even this isn't particularly true is it? I get that you won't back deploy the binary directly, but it could result in a different output

True. So then I would want to set a minimum OS version that might be lower than the OS version of the host.

keith added a commit to keith/bazel that referenced this issue Feb 11, 2021
This flag makes sure we set the minimum macOS version when compiling
host tools. Otherwise you can end up not sharing caches across
different OS versions.

Fixes bazelbuild#12988
@keith
Copy link
Member

keith commented Feb 11, 2021

#13001

@aiuto
Copy link
Contributor

aiuto commented Feb 12, 2021

cc @susinmotion Any thoughts on #13001?

@susinmotion
Copy link
Contributor

It's in good hands with trybka and allevato!

@aiuto aiuto assigned allevato and unassigned aiuto Feb 13, 2021
keith added a commit to keith/bazel that referenced this issue Feb 23, 2021
This flag makes sure we set the minimum macOS version when compiling
host tools. Otherwise you can end up not sharing caches across
different OS versions.

Fixes bazelbuild#12988
@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) type: bug and removed untriaged labels May 10, 2021
keith added a commit to keith/bazel that referenced this issue Sep 21, 2021
This flag makes sure we set the minimum macOS version when compiling
host tools. Otherwise you can end up not sharing caches across
different OS versions.

Fixes bazelbuild#12988
keith added a commit to keith/bazel that referenced this issue Oct 4, 2021
This flag makes sure we set the minimum macOS version when compiling
host tools. Otherwise you can end up not sharing caches across
different OS versions.

Fixes bazelbuild#12988
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) platform: apple team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants