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

[Musl] Add back missing autolink arguments, fix CFPosixSpawnFileActionsChdir. #5143

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Dec 5, 2024

We need a handful more autolink arguments; these were in a previous PR, but seem to have gone missing at some point during the re-core.

Additionally, Musl has the posix_spawn_file_actions_addchdir_np() function rather than the non-_np() version.

Fixes #5089.

rdar://140914176

…nsChdir.

We need a handful more autolink arguments; these were in a previous PR,
but seem to have gone missing at some point during the re-core.

Additionally, Musl has the `posix_spawn_file_actions_addchdir_np()`
function rather than the non-`_np()` version.

Fixes swiftlang#5089.
@al45tair
Copy link
Contributor Author

al45tair commented Dec 5, 2024

@swift-ci Please test Linux platform

@al45tair
Copy link
Contributor Author

al45tair commented Dec 5, 2024

@swift-ci Please test Windows platform

@al45tair
Copy link
Contributor Author

al45tair commented Dec 5, 2024

This was previously cherry-picked to release/6.0.2, but not to release/6.0. Its absence in the latter is causing the oss-swift-6.0-package-static-sdk build to fail.

@al45tair
Copy link
Contributor Author

al45tair commented Dec 5, 2024

Explanation: We need to add the autolink options in order for programs built with the Static SDK to work without having to add extra linker arguments. Also, Musl has the _np() version of posix_spawn_file_actions_addchdir, at least in the version we're using.
Risk: Low. We already have these changes on main, and they only affect the Static SDK for Linux.
Original PR: #5097
Reviewed by: @parkera @jmschonfeld
Resolves: rdar://140914176
Tests: The fixes here have been manually tested.

@@ -57,6 +57,7 @@ set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
set(CMAKE_Swift_MODULE_DIRECTORY ${CMAKE_BINARY_DIR}/swift)

option(BUILD_SHARED_LIBS "build shared libraries" ON)
option(BUILD_FULLY_STATIC "build fully static" NO)
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should prefer this to be spelt FOUNDATION_FULLY_STATIC. BUILD_SHARED_LIBS is a standard CMake option that we are changing the default value for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we're going to make that change, we should do so separately; this is already merged in other branches and is just a cherry pick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Changing that will also require a matching change to the scripts in the swift-docker repo too.)

@al45tair al45tair merged commit 26a599c into swiftlang:release/6.0 Dec 6, 2024
2 checks passed
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