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

[SE-0362] Add enableUpcomingFeature and enableExperimentalFeature Swift settings #5632

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Jun 28, 2022

@DougGregor
Copy link
Member Author

@swift-ci please test

@tomerd
Copy link
Contributor

tomerd commented Jun 28, 2022

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

I believe from the code that these flags are being treat as "unsafe", which we didn't want for enableFutureFeature. But it seems rather invasive to change.

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

Will adding the assignments to PackageBuilder automatically get them added to the PIF generated when using --build-system xcbuild, or would that need to be added separately? From my reading that should work but it's hard to tell.

/// - name: The name of the future feature, e.g., ConciseMagicFile.
/// - condition: A condition that restricts the application of the build
/// setting.
@available(_PackageDescription, introduced: 5.7)
Copy link
Member Author

Choose a reason for hiding this comment

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

These should be 999.0, since this won't make 5.7

Comment on lines +952 to +1003
values = _values.precedeElements(
with: "-enable-experimental-feature")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think precedeElements(with:) is needed, if the new APIs are implemented like the existing .define API, which only stores a single value.

case .enableExperimentalFeature(let value):
    values = ["-enable-experimental-feature", value]
    switch setting.tool {

@hassila
Copy link

hassila commented Nov 30, 2022

Just a friendly ping: Out of curiosity, is this going to be part of 5.8? We plan to prepare all our repos for Swift 6 features now and try to decide whether to wait for a 5.x with this nice feature enabled or whether to use the various other mechanisms to enable them.

@DougGregor
Copy link
Member Author

Thanks for the ping! Yes, I plan to get this into 5.8.

@DougGregor
Copy link
Member Author

(I still need to rename "future" to "upcoming", per the proposal's review discussion)

@hassila
Copy link

hassila commented Nov 30, 2022

Super, thanks! Then we'll look forward to testing it out and plan accordingly!

@DougGregor DougGregor force-pushed the future-and-experimental-build-settings branch from b1d126e to 0840562 Compare December 1, 2022 19:37
@DougGregor DougGregor changed the title Add enableFutureFeature and enableExperimentalFeature Swift settings Add enableUpcomingFeature and enableExperimentalFeature Swift settings Dec 1, 2022
@DougGregor DougGregor changed the title Add enableUpcomingFeature and enableExperimentalFeature Swift settings [SE-0362] Add enableUpcomingFeature and enableExperimentalFeature Swift settings Dec 1, 2022
@DougGregor
Copy link
Member Author

@swift-ci please test

@neonichu
Copy link
Contributor

neonichu commented Dec 1, 2022

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@neonichu how's my implementation look?

@DougGregor DougGregor merged commit ef2934d into main Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants