Skip to content

[build] explicitly set OSX_ARCHITECTURES for targets in stdlib #38415

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

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

edymtt
Copy link
Contributor

@edymtt edymtt commented Jul 15, 2021

Our custom build code already sets the architecture of compilation
commands by different means, and so far we were relying on setting
CMAKE_OSX_ARCHITECTURES to an empty value to avoid CMake doing the
same.

However, on Apple Silicon, CMake 3.19+ enforces a default value in this
scenario
(https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5291/diffs),
and this would result in the inability to compile code for x86_64.

Addresses SR-14035, rdar://80613594

Our custom build code already sets the architecture of compilation
commands by different means, and so far we were relying on setting
`CMAKE_OSX_ARCHITECTURES` to an empty value to avoid CMake doing the
same.

However, on Apple Silicon, CMake 3.19+ enforces a default value in this
scenario
(https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5291/diffs),
and this would result in the inability to compile code for x86_64.

Addresses SR-14035, rdar://80613594
@edymtt
Copy link
Contributor Author

edymtt commented Jul 15, 2021

@swift-ci please test

@edymtt
Copy link
Contributor Author

edymtt commented Jul 15, 2021

@swift-ci test Apple Silicon

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments seem to be wrapped oddly, but the change itself LGTM.

@edymtt
Copy link
Contributor Author

edymtt commented Jul 16, 2021

@swift-ci please test Windows

@edymtt
Copy link
Contributor Author

edymtt commented Jul 16, 2021

The preset buildbot_incremental,tools=RA,stdlib=RA,apple_silicon only builds for arm64, since it sets swift-darwin-supported-archs=arm64 -- so that does not show properly that my changes work as intended.

@edymtt
Copy link
Contributor Author

edymtt commented Jul 16, 2021

The macOS platform tests show that, as a result of this change, the architecture parameter is specified twice and it has the same value (which is a good indicator of what would happen for a build on Apple Silicon without swift-darwin-supported-archs set) -- e.g.

<snip> -arch armv7k -isysroot /Applications/Xcode.app/...
<snip> -arch armv7k -F/Applications/Xcode.app/...
<snip> -o stdlib/toolchain/Compatibility50/CMakeFiles/swiftCompatibility50-watchos-armv7k.dir/ProtocolConformance.cpp.o

@edymtt
Copy link
Contributor Author

edymtt commented Jul 16, 2021

@swift-ci please test Windows

@edymtt edymtt merged commit d731aa2 into swiftlang:main Jul 16, 2021
edymtt added a commit to edymtt/swift that referenced this pull request Jul 16, 2021
edymtt added a commit to edymtt/swift that referenced this pull request Jul 16, 2021
Our custom build code already sets the architecture of compilation
commands by different means, and so far we were relying on setting
`CMAKE_OSX_ARCHITECTURES` to an empty value to avoid CMake doing the
same.

However, on Apple Silicon, CMake 3.19+ enforces a default value in this
scenario
(https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5291/diffs),
and this would result in the inability to compile code for x86_64.

Cherry-pick of swiftlang#38415, swiftlang#38444

Addresses SR-14035, rdar://80699579
edymtt added a commit that referenced this pull request Jul 19, 2021
@edymtt edymtt deleted the explicitly-set-osx-architecture-macos branch July 19, 2021 15:12
edymtt added a commit to edymtt/swift that referenced this pull request Jan 28, 2022
Previously we were setting `-arch` explicitly and unsetting
`CMAKE_OSX_ARCHITECTURES`; however this approach does not work when
building on Apple Silicon, since in there CMake 3.19+ enforces a default
value
(https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5291/diffs),
and this would result in the inability to compile code for x86_64.

This is a similar approach used for stdlib targets in swiftlang#38415

Addresses rdar://88100025
edymtt added a commit that referenced this pull request Jan 31, 2022
Previously we were setting `-arch` explicitly and unsetting
`CMAKE_OSX_ARCHITECTURES`; however this approach does not work when
building on Apple Silicon, since in there CMake 3.19+ enforces a default
value
(https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5291/diffs),
and this would result in the inability to compile code for x86_64.

This is a similar approach used for stdlib targets in #38415

Addresses rdar://88100025
edymtt added a commit to edymtt/swift that referenced this pull request Jul 6, 2022
This way we configure correctly `libBlocksRuntime`, which is not using
`add_swift_target_library` (where the code for setting `OSX_ARCHITECTURES`
currently lives, see swiftlang#38415 and swiftlang#38956)

Addresses rdar://96469791
edymtt added a commit that referenced this pull request Jul 8, 2022
…59922)

This way we configure correctly `libBlocksRuntime`, which is not using
`add_swift_target_library` (where the code for setting `OSX_ARCHITECTURES`
currently lives, see #38415 and #38956)

Addresses rdar://96469791
hborla pushed a commit to hborla/swift that referenced this pull request Jul 21, 2022
…wiftlang#59922)

This way we configure correctly `libBlocksRuntime`, which is not using
`add_swift_target_library` (where the code for setting `OSX_ARCHITECTURES`
currently lives, see swiftlang#38415 and swiftlang#38956)

Addresses rdar://96469791
edymtt added a commit to edymtt/swift that referenced this pull request Sep 8, 2022
Rely instead on setting the `OSX_ARCHITECTURES` property (swiftlang#38415)

Address rdar://96087734
edymtt added a commit to edymtt/swift that referenced this pull request Sep 27, 2022
The underlying issue that required restricting to `arm64` has been fixed
in swiftlang#38415

Addresses rdar://77191256
edymtt added a commit that referenced this pull request Sep 28, 2022
The underlying issue that required restricting to `arm64` has been fixed
in #38415

Addresses rdar://77191256
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.

2 participants