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

Retrieve single arch cpu from cc_toolchain instead of from apple_fragment #694

Conversation

thii
Copy link
Member

@thii thii commented Oct 4, 2021

apple_fragment.single_arch_cpu was returning the default macos cpu for
the host platform for tools while it should return the cpu of the
execution platform.

@google-cla google-cla bot added the cla: yes label Oct 4, 2021
@thii thii marked this pull request as ready for review October 4, 2021 08:06
@brentleyjones
Copy link
Collaborator

Do you feel this is a bug with apple_fragment.single_arch_cpu?

@bazelbuild bazelbuild deleted a comment from lyft-lint-bot Oct 4, 2021
@thii
Copy link
Member Author

thii commented Oct 5, 2021

It could be, but I think it's safe to use this as it's the one cc_library and objc_library use.

@thii thii force-pushed the retrieve-single-arch-cpu-from-cc_toolchain-instead-of-from-apple_fragment branch from 8accbc9 to 96c3009 Compare November 11, 2021 08:39
@thii thii requested a review from keith November 11, 2021 08:40
…ragment`

`apple_fragment.single_arch_cpu` is returning the default macos cpu for
the host platform for tools while it should return the cpu of the
execution platform.
@thii thii force-pushed the retrieve-single-arch-cpu-from-cc_toolchain-instead-of-from-apple_fragment branch from 96c3009 to a76e7d3 Compare November 11, 2021 08:41
@keith
Copy link
Member

keith commented Nov 17, 2021

I think now that this might be fixed by the change like this bazelbuild/rules_apple#1154 we should try that first

@brentleyjones
Copy link
Collaborator

Now that bazelbuild/rules_apple#1154 is merged, is this still an issue?

@thii
Copy link
Member Author

thii commented Nov 29, 2021

Yes, still. I think this is a Bazel issue, since rules_swift doesn't depend on rules_apple.

@brentleyjones
Copy link
Collaborator

Yep, we have to use this patch as well until Bazel fixes it's stuff. Doesn't look like anyone is looking at it over there so low chance of a 5.0 fix?

@thii
Copy link
Member Author

thii commented Dec 14, 2021

Looks like it has been fixed as of bazelbuild/bazel@c873525.

Do you have any idea which commit fixes this? I think it should be cherry-picked into 5.0.

Edit: See other comments below.

@thii
Copy link
Member Author

thii commented Dec 14, 2021

Interesting that the baseline of 5.0 doesn't have this issue, but 5.0.0rc2 does.

@thii
Copy link
Member Author

thii commented Dec 14, 2021

Sorry, above two comments were incorrect. I was testing it wrong due to a bug in bazelisk - it is pulling x86_64 binaries for unreleased commit on M1 bazelbuild/bazelisk#260

Update: It has not been fixed at HEAD.

@brentleyjones
Copy link
Collaborator

brentleyjones commented Dec 14, 2021

I say we merge this, to fix the issue, and revert when Bazel gets a fix.

@thii
Copy link
Member Author

thii commented Jan 24, 2022

Does no one else have this problem? We're still not able to build with universal tools without this patch.

@brentleyjones
Copy link
Collaborator

Yes, we need this patch to build universal tools. My comment still stands, I think we should merge it for now.

@thii
Copy link
Member Author

thii commented Jan 25, 2022

Thanks 🙏
I'm taking this in for now

@thii thii merged commit 918a4d6 into bazelbuild:master Jan 25, 2022
@thii thii deleted the retrieve-single-arch-cpu-from-cc_toolchain-instead-of-from-apple_fragment branch January 25, 2022 01:20
keith added a commit that referenced this pull request Jan 25, 2022
keith added a commit that referenced this pull request Jan 25, 2022
tymurmustafaiev pushed a commit to tymurmustafaiev/rules_swift that referenced this pull request Jul 19, 2023
…ragment` (bazelbuild#694)

`apple_fragment.single_arch_cpu` is returning the default macos cpu for
the host platform for tools while it should return the cpu of the
execution platform.
tymurmustafaiev pushed a commit to tymurmustafaiev/rules_swift that referenced this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants