-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Mirror what native build system does for enabling clang modules #9188
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
base: main
Are you sure you want to change the base?
Conversation
daveinglis
commented
Sep 25, 2025
- the native builder only enables clang modules on Darwin, so we will need to do the same with swift-build.
- the native builder only enables clang modules on Darwin, so we will need to do the same with swift-build.
@swift-ci test |
|
||
func configureProjectBuildSettings(_ buildSettings: inout ProjectModel.BuildSettings) { | ||
/* empty */ | ||
// This is parity to the native build, But we should investigate what it would take to get this working on platforms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of checking the current platform in BuildParameters, we should use platform conditionals here so that the PIF remains platform-agnostic. The subscript on buildSettings has a for
parameter you can use to add platform conditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably try at some point to avoid giving the PIF builder access to build parameters to help prevent this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. The package model will likely need enhancement so that we can properly express all the conditionals users will need, especially around platforms, but that's in the works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that using the platform conditions is okay for now:
buildSettings.platformSpecificSettings[.linux]![.FOO] = "bar"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Why are we doing this in the PIF delegate implementation for SwiftPM? To be clear, we should reserve the delegate for SwiftPM only patches.
If this is a general patch (eg, Xcode, etc) we should hardcode it in PackagePIFBuilder.addProjectBuildSettings
instead. For instance, Xcode has it's own independent PackagePIFBuilder.BuildDelegate
implementation and, as such, won't inherit such configuration as is.
} | ||
|
||
settings[.USE_HEADERMAP] = "NO" | ||
settings[.OTHER_SWIFT_FLAGS].lazilyInitializeAndMutate(initialValue: ["$(inherited)"]) { $0.append("-DXcode") } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should address this separately and conditionalize it on a new field in the PIFBuilderParameters