Skip to content

Adopt language mode naming in PackageDescription API #7620

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

Merged
merged 16 commits into from
Jul 24, 2024
Merged

Adopt language mode naming in PackageDescription API #7620

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4a74d64
Rename SwiftVersion to SwiftLanguageMode
dempseyatgithub May 31, 2024
3d80f9c
Change swiftLanguageVersion build setting to swiftLanguageMode
dempseyatgithub May 31, 2024
50f4e0f
Add Package init method with language mode terminology.
dempseyatgithub May 31, 2024
64180d7
Rename enum case to swiftLanguageMode in target build setting descrip…
dempseyatgithub May 31, 2024
25c29b7
Rename swiftVersions to swiftLanguageModes
dempseyatgithub Jun 1, 2024
54029d0
Update test fixture manifest to use updated API
dempseyatgithub Jun 2, 2024
b650dcb
Address review feedback
dempseyatgithub Jun 2, 2024
0112629
Rename parameter name to 'mode' following established convention
dempseyatgithub Jun 2, 2024
c59d5ae
Update documentation comment
dempseyatgithub Jun 2, 2024
95af338
Add Package init method with language mode terminology.
dempseyatgithub May 31, 2024
b53eb42
Rename swiftVersions to swiftLanguageModes
dempseyatgithub Jun 1, 2024
3f484ff
Address review feedback
dempseyatgithub Jun 2, 2024
2d93cd4
[PackageDescription] Restore @_spi annotation on an initializer that …
xedin Jul 16, 2024
fbdbb1d
Update to match proposed interfaces in SE-0441
dempseyatgithub Jul 21, 2024
262664c
Implement LSG feedback to deprecate existing init method instead of m…
dempseyatgithub Jul 24, 2024
83c96a1
Use @_disfavoredOverload instead of second init method to resolve amb…
dempseyatgithub Jul 24, 2024
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
2 changes: 1 addition & 1 deletion Fixtures/Miscellaneous/Plugins/DependentPlugins/Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ let package = Package(
]
),
],
swiftLanguageVersions: [.v5]
swiftLanguageModes: [.v5]
)
19 changes: 18 additions & 1 deletion Sources/PackageDescription/BuildSettings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,14 +416,31 @@ public struct SwiftSetting: Sendable {
/// - Parameters:
/// - version: The Swift language version to use.
/// - condition: A condition that restricts the application of the build setting.
@available(_PackageDescription, introduced: 6.0)
@available(_PackageDescription, introduced: 6.0, deprecated: 6.0, renamed: "swiftLanguageMode(_:_:)")
public static func swiftLanguageVersion(
_ version: SwiftVersion,
_ condition: BuildSettingCondition? = nil
) -> SwiftSetting {
return SwiftSetting(
name: "swiftLanguageVersion", value: [.init(describing: version)], condition: condition)
}

/// Defines a `-language-mode` to pass to the
/// corresponding build tool.
///
/// - Since: First available in PackageDescription 6.0.
///
/// - Parameters:
/// - mode: The Swift language mode to use.
/// - condition: A condition that restricts the application of the build setting.
@available(_PackageDescription, introduced: 6.0)
public static func swiftLanguageMode(
Copy link
Contributor

@MaxDesiatov MaxDesiatov Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, sorry I missed that, makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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 🤷

_ mode: SwiftLanguageMode,
_ condition: BuildSettingCondition? = nil
) -> SwiftSetting {
return SwiftSetting(
name: "swiftLanguageMode", value: [.init(describing: mode)], condition: condition)
}
}

/// A linker build setting.
Expand Down
11 changes: 7 additions & 4 deletions Sources/PackageDescription/LanguageStandardSettings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,8 @@ public enum CXXLanguageStandard: String {
case gnucxx2b = "gnu++2b"
}

/// The version of the Swift language you use to compile Swift sources in the
/// package.
public enum SwiftVersion {
/// The Swift language mode used to compile Swift sources in the package
public enum SwiftLanguageMode {
/// The identifier for the Swift 3 language version.
@available(_PackageDescription, introduced: 4, obsoleted: 5)
case v3
Expand All @@ -177,7 +176,7 @@ public enum SwiftVersion {
case version(String)
}

extension SwiftVersion: CustomStringConvertible {
extension SwiftLanguageMode: CustomStringConvertible {
public var description: String {
switch self {
case .v3: "3"
Expand All @@ -189,3 +188,7 @@ extension SwiftVersion: CustomStringConvertible {
}
}
}

/// Type alias to previous name for backward source compatibility
@available(_PackageDescription, deprecated: 6, renamed:"SwiftLanguageMode")
public typealias SwiftVersion = SwiftLanguageMode
117 changes: 107 additions & 10 deletions Sources/PackageDescription/PackageDescription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,15 @@ public final class Package {
/// The list of package dependencies.
public var dependencies: [Dependency]

/// The list of Swift versions with which this package is compatible.
public var swiftLanguageVersions: [SwiftVersion]?
/// The list of Swift language modes with which this package is compatible.
public var swiftLanguageModes: [SwiftLanguageMode]?

/// Legacy property name, accesses value of `swiftLanguageModes`
@available(_PackageDescription, deprecated: 6, renamed: "swiftLanguageModes")
public var swiftLanguageVersions: [SwiftVersion]? {
get { swiftLanguageModes }
set { swiftLanguageModes = newValue }
}

/// The C language standard to use for all C targets in this package.
public var cLanguageStandard: CLanguageStandard?
Expand Down Expand Up @@ -151,7 +158,7 @@ public final class Package {
self.dependencies = dependencies
self.targets = targets
self.traits = []
self.swiftLanguageVersions = swiftLanguageVersions.map{ $0.map{ .version("\($0)") } }
self.swiftLanguageModes = swiftLanguageVersions.map{ $0.map{ .version("\($0)") } }
self.cLanguageStandard = cLanguageStandard
self.cxxLanguageStandard = cxxLanguageStandard
registerExitHandler()
Expand Down Expand Up @@ -190,7 +197,7 @@ public final class Package {
self.dependencies = dependencies
self.targets = targets
self.traits = []
self.swiftLanguageVersions = swiftLanguageVersions
self.swiftLanguageModes = swiftLanguageVersions
self.cLanguageStandard = cLanguageStandard
self.cxxLanguageStandard = cxxLanguageStandard
registerExitHandler()
Expand Down Expand Up @@ -232,7 +239,7 @@ public final class Package {
self.dependencies = dependencies
self.targets = targets
self.traits = []
self.swiftLanguageVersions = swiftLanguageVersions
self.swiftLanguageModes = swiftLanguageVersions
self.cLanguageStandard = cLanguageStandard
self.cxxLanguageStandard = cxxLanguageStandard
registerExitHandler()
Expand All @@ -254,6 +261,51 @@ public final class Package {
/// - cLanguageStandard: The C language standard to use for all C targets in this package.
/// - 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(
name: String,
defaultLocalization: LanguageTag? = nil,
platforms: [SupportedPlatform]? = nil,
pkgConfig: String? = nil,
providers: [SystemPackageProvider]? = nil,
products: [Product] = [],
dependencies: [Dependency] = [],
targets: [Target] = [],
swiftLanguageVersions: [SwiftVersion]?,
cLanguageStandard: CLanguageStandard? = nil,
cxxLanguageStandard: CXXLanguageStandard? = nil
) {
self.name = name
self.defaultLocalization = defaultLocalization
self.platforms = platforms
self.pkgConfig = pkgConfig
self.providers = providers
self.products = products
self.dependencies = dependencies
self.targets = targets
self.traits = []
self.swiftLanguageModes = swiftLanguageVersions
self.cLanguageStandard = cLanguageStandard
self.cxxLanguageStandard = cxxLanguageStandard
registerExitHandler()
}

/// Initializes a Swift package with configuration options you provide.
///
/// - Parameters:
/// - name: The name of the Swift package, or `nil` to use the package's Git URL to deduce the name.
/// - defaultLocalization: The default localization for resources.
/// - platforms: The list of supported platforms with a custom deployment target.
/// - pkgConfig: The name to use for C modules. If present, Swift Package Manager searches for a
/// `<name>.pc` file to get the additional flags required for a system target.
/// - providers: The package providers for a system target.
/// - products: The list of products that this package makes available for clients to use.
/// - dependencies: The list of package dependencies.
/// - targets: The list of targets that are part of this package.
/// - cLanguageStandard: The C language standard to use for all C targets in this package.
/// - 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(
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's fine to use responsibly :)

name: String,
defaultLocalization: LanguageTag? = nil,
Expand All @@ -263,7 +315,6 @@ public final class Package {
products: [Product] = [],
dependencies: [Dependency] = [],
targets: [Target] = [],
swiftLanguageVersions: [SwiftVersion]? = nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

cLanguageStandard: CLanguageStandard? = nil,
cxxLanguageStandard: CXXLanguageStandard? = nil
) {
Expand All @@ -276,12 +327,58 @@ public final class Package {
self.dependencies = dependencies
self.targets = targets
self.traits = []
self.swiftLanguageVersions = swiftLanguageVersions
self.swiftLanguageModes = []
self.cLanguageStandard = cLanguageStandard
self.cxxLanguageStandard = cxxLanguageStandard
registerExitHandler()
}


/// Initializes a Swift package with configuration options you provide.
///
/// - Parameters:
/// - name: The name of the Swift package, or `nil` to use the package's Git URL to deduce the name.
/// - defaultLocalization: The default localization for resources.
/// - platforms: The list of supported platforms with a custom deployment target.
/// - pkgConfig: The name to use for C modules. If present, Swift Package Manager searches for a
/// `<name>.pc` file to get the additional flags required for a system target.
/// - providers: The package providers for a system target.
/// - products: The list of products that this package makes available for clients to use.
/// - dependencies: The list of package dependencies.
/// - targets: The list of targets that are part of this package.
/// - swiftLanguageModes: The list of Swift language modes with which this package is compatible.
/// - cLanguageStandard: The C language standard to use for all C targets in this package.
/// - cxxLanguageStandard: The C++ language standard to use for all C++ targets in this package.
@available(_PackageDescription, introduced: 6)
public init(
name: String,
defaultLocalization: LanguageTag? = nil,
platforms: [SupportedPlatform]? = nil,
pkgConfig: String? = nil,
providers: [SystemPackageProvider]? = nil,
products: [Product] = [],
dependencies: [Dependency] = [],
targets: [Target] = [],
swiftLanguageModes: [SwiftLanguageMode]? = nil,
cLanguageStandard: CLanguageStandard? = nil,
cxxLanguageStandard: CXXLanguageStandard? = nil
) {
self.name = name
self.defaultLocalization = defaultLocalization
self.platforms = platforms
self.pkgConfig = pkgConfig
self.providers = providers
self.products = products
self.dependencies = dependencies
self.targets = targets
self.traits = []
self.swiftLanguageModes = swiftLanguageModes
self.cLanguageStandard = cLanguageStandard
self.cxxLanguageStandard = cxxLanguageStandard
registerExitHandler()
}


/// Initializes a Swift package with configuration options you provide.
///
/// - Parameters:
Expand All @@ -295,7 +392,7 @@ public final class Package {
/// - traits: The set of traits of this package.
/// - dependencies: The list of package dependencies.
/// - targets: The list of targets that are part of this package.
/// - swiftLanguageVersions: The list of Swift versions with which this package is compatible.
/// - swiftLanguageModes: The list of Swift language modes with which this package is compatible.
/// - cLanguageStandard: The C language standard to use for all C targets in this package.
/// - cxxLanguageStandard: The C++ language standard to use for all C++ targets in this package.
@_spi(ExperimentalTraits)
Expand All @@ -310,7 +407,7 @@ public final class Package {
traits: Set<Trait> = [],
dependencies: [Dependency] = [],
targets: [Target] = [],
swiftLanguageVersions: [SwiftVersion]? = nil,
swiftLanguageModes: [SwiftLanguageMode]? = nil,
cLanguageStandard: CLanguageStandard? = nil,
cxxLanguageStandard: CXXLanguageStandard? = nil
) {
Expand All @@ -323,7 +420,7 @@ public final class Package {
self.traits = traits
self.dependencies = dependencies
self.targets = targets
self.swiftLanguageVersions = swiftLanguageVersions
self.swiftLanguageModes = swiftLanguageModes
self.cLanguageStandard = cLanguageStandard
self.cxxLanguageStandard = cxxLanguageStandard
registerExitHandler()
Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageDescription/PackageDescriptionSerializationConversion.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ extension Serialization.Package {
self.products = package.products.map { .init($0) }
self.traits = Set(package.traits.map { Serialization.Trait($0) })
self.dependencies = package.dependencies.map { .init($0) }
self.swiftLanguageVersions = package.swiftLanguageVersions?.map { .init($0) }
self.swiftLanguageVersions = package.swiftLanguageModes?.map { .init($0) }
self.cLanguageStandard = package.cLanguageStandard.map { .init($0) }
self.cxxLanguageStandard = package.cxxLanguageStandard.map { .init($0) }
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/PackageLoading/ManifestJSONParser.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ extension TargetBuildSettingDescription.Kind {
case "unsafeFlags":
return .unsafeFlags(values)

case "swiftLanguageVersion":
case "swiftLanguageMode":
guard let rawVersion = values.first else {
throw InternalError("invalid (empty) build settings value")
}
Expand All @@ -553,7 +553,7 @@ extension TargetBuildSettingDescription.Kind {
throw InternalError("unknown swift language version: \(rawVersion)")
}

return .swiftLanguageVersion(version)
return .swiftLanguageMode(version)
default:
throw InternalError("invalid build setting \(name)")
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageLoading/PackageBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,7 @@ public final class PackageBuilder {

values = ["-enable-experimental-feature", value]

case .swiftLanguageVersion(let version):
case .swiftLanguageMode(let version):
switch setting.tool {
case .c, .cxx, .linker:
throw InternalError("only Swift supports swift language version")
Expand Down
4 changes: 2 additions & 2 deletions Sources/PackageModel/Manifest/TargetBuildSettingDescription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ public enum TargetBuildSettingDescription {

case unsafeFlags([String])

case swiftLanguageVersion(SwiftLanguageVersion)
case swiftLanguageMode(SwiftLanguageVersion)

public var isUnsafeFlags: Bool {
switch self {
case .unsafeFlags(let flags):
// If `.unsafeFlags` is used, but doesn't specify any flags, we treat it the same way as not specifying it.
return !flags.isEmpty
case .headerSearchPath, .define, .linkedLibrary, .linkedFramework, .interoperabilityMode,
.enableUpcomingFeature, .enableExperimentalFeature, .swiftLanguageVersion:
.enableUpcomingFeature, .enableExperimentalFeature, .swiftLanguageMode:
return false
}
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/PackageModel/ManifestSourceGeneration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ fileprivate extension SourceCodeFragment {
params.append(SourceCodeFragment(from: condition))
}
self.init(enum: setting.kind.name, subnodes: params)
case .swiftLanguageVersion(let version):
case .swiftLanguageMode(let version):
params.append(SourceCodeFragment(from: version))
if let condition = setting.condition {
params.append(SourceCodeFragment(from: condition))
Expand Down Expand Up @@ -685,8 +685,8 @@ extension TargetBuildSettingDescription.Kind {
return "enableUpcomingFeature"
case .enableExperimentalFeature:
return "enableExperimentalFeature"
case .swiftLanguageVersion:
return "swiftLanguageVersion"
case .swiftLanguageMode:
return "swiftLanguageMode"
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions Tests/BuildTests/BuildPlanTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4158,12 +4158,12 @@ final class BuildPlanTests: XCTestCase {
),
.init(
tool: .swift,
kind: .swiftLanguageVersion(.v4),
kind: .swiftLanguageMode(.v4),
condition: .init(platformNames: ["macos"])
),
.init(
tool: .swift,
kind: .swiftLanguageVersion(.v5),
kind: .swiftLanguageMode(.v5),
condition: .init(platformNames: ["linux"])
),
.init(tool: .linker, kind: .linkedLibrary("sqlite3")),
Expand Down Expand Up @@ -4196,8 +4196,8 @@ final class BuildPlanTests: XCTestCase {
name: "t1",
settings: [
.init(tool: .swift, kind: .define("DEP")),
.init(tool: .swift, kind: .swiftLanguageVersion(.v4), condition: .init(platformNames: ["linux"])),
.init(tool: .swift, kind: .swiftLanguageVersion(.v5), condition: .init(platformNames: ["macos"])),
.init(tool: .swift, kind: .swiftLanguageMode(.v4), condition: .init(platformNames: ["linux"])),
.init(tool: .swift, kind: .swiftLanguageMode(.v5), condition: .init(platformNames: ["macos"])),
.init(tool: .linker, kind: .linkedLibrary("libz")),
]
),
Expand Down
3 changes: 2 additions & 1 deletion Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ final class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
products: [],
targets: [
.target(name: "Foo"),
]
],
swiftLanguageVersions: [.v5]
)
"""

Expand Down
6 changes: 3 additions & 3 deletions Tests/PackageLoadingTests/PackageBuilderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3038,14 +3038,14 @@ final class PackageBuilderTests: XCTestCase {
try TargetDescription(
name: "foo",
settings: [
.init(tool: .swift, kind: .swiftLanguageVersion(.v5))
.init(tool: .swift, kind: .swiftLanguageMode(.v5))
]
),
try TargetDescription(
name: "bar",
settings: [
.init(tool: .swift, kind: .swiftLanguageVersion(.v3), condition: .init(platformNames: ["linux"])),
.init(tool: .swift, kind: .swiftLanguageVersion(.v4), condition: .init(platformNames: ["macos"], config: "debug"))
.init(tool: .swift, kind: .swiftLanguageMode(.v3), condition: .init(platformNames: ["linux"])),
.init(tool: .swift, kind: .swiftLanguageMode(.v4), condition: .init(platformNames: ["macos"], config: "debug"))
]
),
]
Expand Down
Loading