Skip to content

Add $(inherited) to OTHER_SWIFT_FLAGS. #2143

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add $(inherited) to OTHER_SWIFT_FLAGS. #2143

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Sources/Xcodeproj/pbxproj().swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ func xcodeProject(

// Also set the `Xcode` build preset in Swift to let code conditionalize on
// being built in Xcode.
projectSettings.common.OTHER_SWIFT_FLAGS += ["-DXcode"]
projectSettings.common.OTHER_SWIFT_FLAGS = ["$(inherited)"]
+ (projectSettings.common.OTHER_SWIFT_FLAGS ?? [])
+ ["-DXcode"]
Copy link
Contributor

Choose a reason for hiding this comment

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

We can move this line before we append the additional compiler flags to simplify this to projectSettings.common.OTHER_SWIFT_FLAGS = ["$(inherited)", "-DXcode"]

Copy link
Author

Choose a reason for hiding this comment

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

I had considered this but decided against it because I didn't know if the order of settings needed to be maintained.

For example, making the change indicated would have the effect of moving "-DXcode" to no longer be at the end, in the case that options.flags.swiftCompilerFlags isn't empty. Additionally, if the current or future implementation of Xcode.Project() were to set OTHER_SWIFT_FLAGS, for some reason, those would be lost with this change.

While I know that the position of -DXcode probably doesn't matter and the Xcode.Project() probably won't set OTHER_SWIFT_FLAGS, I felt the code I wrote best preserved the existing semantics and protected against these small possible outlier issues I described.

Copy link
Contributor

Choose a reason for hiding this comment

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

We usually start with assigning a value to these arrays and then use += to add more content into them. In this case, the order doesn't really matter and it'll be more confusing that we're not appending in later calls.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback on this.

projectSettings.common.MACOSX_DEPLOYMENT_TARGET = "10.10"

// Prevent Xcode project upgrade warnings.
Expand Down