Skip to content

[cxx-interop] Lower macOS deployment target version for the Cxx module #69512

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 2 commits into from
Nov 2, 2023

Conversation

egorzhdan
Copy link
Contributor

@egorzhdan egorzhdan commented Oct 30, 2023

The Cxx module does not rely on any modern OS features. It can be deployed to older macOS versions.

rdar://117699474

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Oct 30, 2023
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@hyp
Copy link
Contributor

hyp commented Oct 30, 2023

@egorzhdan what about the CxxStdlib module?

The CMake flag `DEPLOYMENT_VERSION_OSX` is currently only passed to the C compiler. This change makes sure it is also passed to the Swift compiler, e.g. when building Swift-only targets like Cxx or CxxStdlib.

rdar://117699474
The Cxx module does not rely on any modern OS features. It can be deployed to older macOS versions.

rdar://117699474
@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-lower-deployment-target branch from e12339f to 00cdb66 Compare October 30, 2023 19:22
@egorzhdan egorzhdan requested a review from a team as a code owner October 30, 2023 19:22
@egorzhdan
Copy link
Contributor Author

what about the CxxStdlib module?

Right, we should apply the same CMake flag to CxxStdlib as well

@egorzhdan egorzhdan requested a review from etcwilde October 30, 2023 19:24
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Windows

@hyp
Copy link
Contributor

hyp commented Nov 1, 2023

@egorzhdan please pick this to 5.10 as well.

Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

Mechanically, yeah, I think this drills all of the appropriate holes to plumb through a different deployment target. Will we need to be able to alter the deployment target on other platforms too, or is this guaranteed to only be a macOS thing?

@@ -236,6 +237,10 @@ function(_add_target_variant_swift_compile_flags

if("${sdk}" IN_LIST SWIFT_DARWIN_PLATFORMS)
set(sdk_deployment_version "${SWIFT_SDK_${sdk}_DEPLOYMENT_VERSION}")
if("${sdk}" STREQUAL "OSX" AND DEFINED VARIANT_DEPLOYMENT_VERSION_OSX)
Copy link
Member

Choose a reason for hiding this comment

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

Will we need this configuration for each Darwin platform, or only macOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might potentially need this on other Darwin platforms in the future, but for now we only expect to use this for macOS. I can add the corresponding CMake logic for iOS, etc. if you think it's worth having it in tree.

@egorzhdan egorzhdan merged commit 07c4a44 into main Nov 2, 2023
@egorzhdan egorzhdan deleted the egorzhdan/cxx-lower-deployment-target branch November 2, 2023 12:46
egorzhdan added a commit that referenced this pull request Nov 6, 2023
egorzhdan added a commit that referenced this pull request Nov 8, 2023
…rces

See #69512

rdar://117699474

(cherry picked from commit f8b5143)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants