Adopt language mode naming in `PackageDescription` API #7620

dempseyatgithub · 2024-05-31T21:18:05Z

This PR implements the package manager changes in Swift evolution proposal SE-0441.

dempseyatgithub · 2024-05-31T21:26:18Z

dempseyatgithub · 2024-05-31T23:06:03Z

xedin

Overall LGTM, I'm not sure about new initializer though, it seems like that could some unfortunate ambiguity issues because renamed parameter is defaulted.

Sources/PackageDescription/BuildSettings.swift

Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift

Sources/PackageDescription/PackageDescription.swift

dempseyatgithub · 2024-06-01T23:00:19Z

@swift-ci please test

dempseyatgithub · 2024-06-02T06:01:38Z

@swift-ci please test

dempseyatgithub · 2024-06-02T14:10:20Z

Overall LGTM, I'm not sure about new initializer though, it seems like that could some unfortunate ambiguity issues because renamed parameter is defaulted.

Yes, the proposal describes this. Looking back at all the existing init methods, every time a new Package init method is introduced, it is made available in a particular release. The existing init method is obsoleted in the same release, so there is no ambiguity.

When adding parameters, there is no breaking change. Changing a parameter name, as happens here, introduces a breaking change when moving to swift tools version 6.0. The renamed annotation allows the compiler to provide a fix-it.

dempseyatgithub · 2024-06-02T14:12:42Z

@swift-ci please test package compatibility

dempseyatgithub · 2024-06-02T14:14:14Z

@swift-ci please test

dempseyatgithub · 2024-06-02T14:51:17Z

@xedin I believe all of the Swift PM changes in the proposal are included and the Linux and macOS tests pass.

(Something related to the CI itself seems to have gone wrong in the latest macOS test, I will re-run it)

Three questions:

Would it be desirable for me to clean up the commits for this PR and then force push it, so the commits and commit messages are nicer? If so, I can do that.
Is there anything else I need to do before this PR can be considered a valid implementation of the proposal? (At least the Swift PM piece of the proposal)
How do you kick off Windows Platform ci tests?

Thank you for your help with this.

dempseyatgithub · 2024-06-03T11:48:48Z

@swift-ci please test

xedin · 2024-06-03T17:41:23Z

Would it be desirable for me to clean up the commits for this PR and then force push it, so the commits and commit messages are nicer? If so, I can do that.

We can squash merge everything so you can add all the information you'd like in the commit message to the PR description.

Is there anything else I need to do before this PR can be considered a valid implementation of the proposal? (At least the Swift PM piece of the proposal)

I think this is pretty much it.

How do you kick off Windows Platform ci tests?

@swift-ci please test Windows platform

MaxDesiatov · 2024-06-04T10:50:45Z

@swift-ci please test Windows platform

both please and platform are redundant, and it's case-insensitive, so plain @swift-ci test windows also works.

dempseyatgithub · 2024-06-08T17:03:29Z

@swift-ci test

dempseyatgithub · 2024-06-08T18:47:44Z

@swift-ci test windows

MaxDesiatov · 2024-06-27T11:28:16Z

Sources/PackageDescription/BuildSettings.swift

    ///   - condition: A condition that restricts the application of the build setting.
    @available(_PackageDescription, introduced: 6.0)
-    public static func swiftLanguageVersion(
-      _ version: SwiftVersion,
+    public static func swiftLanguageMode(


This PR needs to introduce a new function and mark the old one as deprecated, otherwise existing Package.swift manifests will break.

I'm willing to do so, but this would be an API change within the same release cycle. No shipping release version of Swift PM has the original API.

Ah, sorry I missed that, makes sense.

I think we should do this anyway - it would be nice not to break packages that have already started using this in their conversion to Swift 6. I admit it's a little weird, but IMO worth handling.

Assuming this proposal does get reviewed in the Swift 6.0 timeframe, I could completely see adding an 'obsoleted in 6 / renamed in 6' for swiftLanguageVersion to give folks an easy migration to move over during pre-release Swift 6.0 with a fix-it - but removing that for the final version so it doesn't have to be baked into the API for all time.

but removing that for the final version so it doesn't have to be baked into the API for all time

We can just deprecate it and then remove when we remove the other swiftLanguageVersion 🤷

API compatibility concerns addressed.

MaxDesiatov · 2024-07-03T14:40:52Z

@swift-ci test

dempseyatgithub · 2024-07-09T19:38:02Z

@swift-ci test

dempseyatgithub · 2024-07-09T22:19:07Z

@swift-ci test

In the PackageDescription Module: - Rename SwiftVersion enum to SwiftLanguageMode - Use new name in extension declaration - Add SwiftVersion as a type alias for SwiftLanguageMode The SwiftVersion type alias provides backwards compatibilty. The existing, now obsoleted init methods use the type alias for clarity with the parameter name matching the parameter type.

Note this only changes the API, the plumbing including the name of the Swift setting generated by the method, still uses the term "swiftLanguageVersion". Updating the plubming will be a follow-on PR.

- Add init method with swiftLanguageModes parameter instead of swiftVersions - Mark existing init method as obsoleted in Swift 6 - Add renamed annotation to existing init method - Update one test of 5.2 package manifests that found new init methods ambiguous

…tion - Update build setting string to "swiftLanguageMode" - Rename Kind enum case in TargetBuildSettingDescription - Update naming in source and tests

- Add public swiftLanguageModes property to Package - Add computed property swiftVersions that accesses new property - Make swiftVersions obsolete for swift tools version 6 - Update to use new property name

MaxDesiatov · 2024-07-19T09:57:58Z

@swift-ci test

- Change 'obsoleted' to 'deprecated' where possible - Add back and deprecate swiftLanguageVersion() Swift setting - Deprecate SwiftVersion type alias

dempseyatgithub · 2024-07-21T10:53:32Z

@swift-ci test

dempseyatgithub · 2024-07-21T17:45:11Z

@swift-ci test windows

feedback addressed

…aking it obsoleted

dempseyatgithub · 2024-07-24T00:11:06Z

@swift-ci test

xedin · 2024-07-24T00:22:07Z

Sources/PackageDescription/PackageDescription.swift

@@ -271,7 +271,7 @@ public final class Package {
        products: [Product] = [],
        dependencies: [Dependency] = [],
        targets: [Target] = [],
-        swiftLanguageVersions: [SwiftVersion]? = nil,


I think in thise scheme new overload with swiftLanguageModes needs default removed as well so we have to versions that accept explicit swiftLanguage* parameter and one that sets a default.

xedin · 2024-07-24T00:26:13Z

Sources/PackageDescription/PackageDescription.swift

+    ///   - cxxLanguageStandard: The C++ language standard to use for all C++ targets in this package.
+    @available(_PackageDescription, introduced: 5.3)
+    @available(_PackageDescription, deprecated: 6, renamed:"init(name:defaultLocalization:platforms:pkgConfig:providers:products:dependencies:targets:swiftLanguageModes:cLanguageStandard:cxxLanguageStandard:)")
+    public init(


I think we can get the same effect by removing this overload and using @_disfavoredOverload on the old one, fewer overloads is usually easier to deal with, do you have a strong reference with this one?

I didn't realize that @_disfavoredOverload was acceptable in public API.

But since it seems to be acceptable, I think @_disfavoredOverload would be a better way to go.

It would let us avoid adding a completely new init method just to have the overloads work out right.

Yeah, it's fine to use responsibly :)

…iguity

dempseyatgithub · 2024-07-24T01:05:08Z

@swift-ci test

dempseyatgithub · 2024-07-24T03:11:36Z

@swift-ci test windows

bnbarham · 2024-07-24T05:01:45Z

@dempseyatgithub given the relatively short runway for 6.0, I'm happy to merge this (and the 6.0 cherry-pick #7813) now. We can implement any requested changes or revert on top as necessary, and this way we can actually test it out on the nightlies.

Thank you for all your work here 🙏!

dempseyatgithub · 2024-07-24T09:10:53Z

@dempseyatgithub given the relatively short runway for 6.0, I'm happy to merge this (and the 6.0 cherry-pick #7813) now. We can implement any requested changes or revert on top as necessary, and this way we can actually test it out on the nightlies.

@bnbarham This sounds good to me. I've brought the PR out of draft mode.

I'm not well versed in the nuts and bolts of the release process for the project, so I am fine with whatever you think will work best.

Thank you for all your work here 🙏!

You're very welcome and thank you and @xedin for all of your help!

dempseyatgithub · 2024-07-24T10:33:44Z

@swift-ci please test package compatibility

…tLanguageVersion` Follow-up to swiftlang#7620 This is important for the manifest cache - make sure that older manifests that used original spelling of the build setting are still deserializable.

…tLanguageVersion` Follow-up to swiftlang#7620 This is important for the manifest cache - make sure that older manifests that used original spelling of the build setting are still deserializable. (cherry picked from commit 5189086)

…tLanguageVersion` Follow-up to #7620 This is important for the manifest cache - make sure that older manifests that used original spelling of the build setting are still deserializable. (cherry picked from commit 5189086)

#7827) …tLanguageVersion` Follow-up to #7620 This is important for the manifest cache - make sure that older manifests that used original spelling of the build setting are still deserializable.

dji1admin

🙂

xedin reviewed Jun 1, 2024

View reviewed changes

xedin approved these changes Jun 3, 2024

View reviewed changes

MaxDesiatov changed the title ~~Adopt language mode naming in Swift PM API~~ Adopt language mode naming in PackageDescription API Jun 4, 2024

dempseyatgithub mentioned this pull request Jun 5, 2024

Proposal to formalize 'language mode' terminology swiftlang/swift-evolution#2474

Merged

MaxDesiatov previously requested changes Jun 27, 2024

View reviewed changes

dempseyatgithub force-pushed the adopt-language-mode-naming branch from 7ec5369 to 7481db7 Compare July 9, 2024 19:07

dempseyatgithub added 7 commits July 14, 2024 08:58

Change swiftLanguageVersion build setting to swiftLanguageMode

3d80f9c

Note this only changes the API, the plumbing including the name of the Swift setting generated by the method, still uses the term "swiftLanguageVersion". Updating the plubming will be a follow-on PR.

Rename enum case to swiftLanguageMode in target build setting descrip…

64180d7

…tion - Update build setting string to "swiftLanguageMode" - Rename Kind enum case in TargetBuildSettingDescription - Update naming in source and tests

Rename swiftVersions to swiftLanguageModes

25c29b7

- Add public swiftLanguageModes property to Package - Add computed property swiftVersions that accesses new property - Make swiftVersions obsolete for swift tools version 6 - Update to use new property name

Update test fixture manifest to use updated API

54029d0

Address review feedback

b650dcb

Update to match proposed interfaces in SE-0441

fbdbb1d

- Change 'obsoleted' to 'deprecated' where possible - Add back and deprecate swiftLanguageVersion() Swift setting - Deprecate SwiftVersion type alias

xedin mentioned this pull request Jul 23, 2024

[6.0][PackageDescription] SE-0441: Rename swiftLanguageVersion to `swift… #7813

Merged

Implement LSG feedback to deprecate existing init method instead of m…

262664c

…aking it obsoleted

xedin reviewed Jul 24, 2024

View reviewed changes

Use @_disfavoredOverload instead of second init method to resolve amb…

83c96a1

…iguity

xedin approved these changes Jul 24, 2024

View reviewed changes

dempseyatgithub marked this pull request as ready for review July 24, 2024 09:04

dempseyatgithub requested review from jakepetroules and francescomikulis as code owners July 24, 2024 09:04

bnbarham merged commit 12c1422 into swiftlang:main Jul 24, 2024
6 checks passed

glbrntt mentioned this pull request Jul 25, 2024

Invalid build setting swiftLanguageVersion #7823

Closed

1 task

xedin mentioned this pull request Jul 26, 2024

[PackageLoading] Support deserialization of manifests that used `swif… #7827

Merged

xedin mentioned this pull request Jul 26, 2024

[6.0][PackageLoading] Support deserialization of manifests that used `swif… #7828

Merged

dji1admin reviewed Aug 13, 2024

View reviewed changes

kkiermasz mentioned this pull request Sep 18, 2024

Replace deprecrated swiftLanguageVersion with swiftLanguageMode apple/swift-migration-guide#140

Merged

Adopt language mode naming in PackageDescription API #7620

Adopt language mode naming in PackageDescription API #7620

Conversation

dempseyatgithub commented May 31, 2024 • edited Loading

dempseyatgithub commented May 31, 2024

dempseyatgithub commented May 31, 2024

xedin left a comment

Choose a reason for hiding this comment

dempseyatgithub commented Jun 1, 2024

dempseyatgithub commented Jun 2, 2024

dempseyatgithub commented Jun 2, 2024

dempseyatgithub commented Jun 2, 2024

dempseyatgithub commented Jun 2, 2024

dempseyatgithub commented Jun 2, 2024 • edited Loading

dempseyatgithub commented Jun 3, 2024

xedin commented Jun 3, 2024

MaxDesiatov commented Jun 4, 2024

dempseyatgithub commented Jun 8, 2024

dempseyatgithub commented Jun 8, 2024

MaxDesiatov Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxDesiatov commented Jul 3, 2024

dempseyatgithub commented Jul 9, 2024

dempseyatgithub commented Jul 9, 2024

MaxDesiatov commented Jul 19, 2024

dempseyatgithub commented Jul 21, 2024

dempseyatgithub commented Jul 21, 2024

dempseyatgithub commented Jul 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dempseyatgithub commented Jul 24, 2024

dempseyatgithub commented Jul 24, 2024

bnbarham commented Jul 24, 2024 • edited Loading

dempseyatgithub commented Jul 24, 2024 • edited Loading

dempseyatgithub commented Jul 24, 2024

dji1admin left a comment

Choose a reason for hiding this comment

Adopt language mode naming in `PackageDescription` API #7620

Adopt language mode naming in `PackageDescription` API #7620

dempseyatgithub commented May 31, 2024 •

edited

Loading

dempseyatgithub commented Jun 2, 2024 •

edited

Loading

MaxDesiatov Jun 27, 2024 •

edited

Loading

bnbarham commented Jul 24, 2024 •

edited

Loading

dempseyatgithub commented Jul 24, 2024 •

edited

Loading