Skip to content

[PackageModel] SwiftTarget: Rename swiftVersion into toolsSwiftVersion #7544

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 2 commits into from
May 10, 2024
Merged

[PackageModel] SwiftTarget: Rename swiftVersion into toolsSwiftVersion #7544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2dbd031
[PackageModel] SwiftTarget: Rename `swiftVersion` into `toolsSwiftVer…
xedin May 9, 2024
e4f814b
[BuildDescription] SwiftTargetBuildDescription: Rename `swiftVersion`…
xedin May 9, 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
8 changes: 4 additions & 4 deletions Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ package final class SwiftTargetBuildDescription {
/// Any addition flags to be added. These flags are expected to be computed during build planning.
var additionalFlags: [String] = []

/// The swift version for this target.
var swiftVersion: SwiftLanguageVersion {
self.swiftTarget.swiftVersion
/// The swift language version that is computed for this target based on tools version of the manifest.
var toolsSwiftVersion: SwiftLanguageVersion {
self.swiftTarget.toolSwiftVersion
}

/// Describes the purpose of a test target, including any special roles such as containing a list of discovered
Expand Down Expand Up @@ -577,7 +577,7 @@ package final class SwiftTargetBuildDescription {

// Fallback to package wide setting if there is no target specific version.
if args.firstIndex(of: "-swift-version") == nil {
args += ["-swift-version", self.swiftVersion.rawValue]
args += ["-swift-version", self.toolsSwiftVersion.rawValue]
}

// Add the output for the `.swiftinterface`, if requested or if library evolution has been enabled some other
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildPlan/BuildPlan+Test.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ private extension PackageModel.SwiftTarget {
sources: sources,
dependencies: dependencies,
packageAccess: packageAccess,
swiftVersion: .v5,
toolsSwiftVersion: .v5,
usesUnsafeFlags: false
)
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/PackageLoading/PackageBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ public final class PackageBuilder {
others: others,
dependencies: dependencies,
packageAccess: potentialModule.packageAccess,
swiftVersion: self.swiftVersion(),
toolsSwiftVersion: self.toolsSwiftVersion(),
declaredSwiftVersions: self.declaredSwiftVersions(),
buildSettings: buildSettings,
buildSettingsDescription: manifestTarget.settings,
Expand Down Expand Up @@ -1237,7 +1237,7 @@ public final class PackageBuilder {
}

/// Computes the swift version to use for this manifest.
private func swiftVersion() throws -> SwiftLanguageVersion {
private func toolsSwiftVersion() throws -> SwiftLanguageVersion {
if let swiftVersion = self.swiftVersionCache {
return swiftVersion
}
Expand Down Expand Up @@ -1738,7 +1738,7 @@ extension PackageBuilder {
sources: sources,
dependencies: dependencies,
packageAccess: false,
swiftVersion: self.swiftVersion(),
toolsSwiftVersion: self.toolsSwiftVersion(),
buildSettings: buildSettings,
buildSettingsDescription: targetDescription.settings,
usesUnsafeFlags: false
Expand Down
18 changes: 9 additions & 9 deletions Sources/PackageModel/Target/SwiftTarget.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public final class SwiftTarget: Target {
}

public init(name: String, dependencies: [Target.Dependency], packageAccess: Bool, testDiscoverySrc: Sources) {
self.swiftVersion = .v5
self.toolSwiftVersion = .v5
self.declaredSwiftVersions = []

super.init(
Expand All @@ -40,8 +40,8 @@ public final class SwiftTarget: Target {
)
}

/// The swift version of this target.
public let swiftVersion: SwiftLanguageVersion
/// The swift language version that is computed for this target based on tools version of the manifest.
public let toolSwiftVersion: SwiftLanguageVersion

/// The list of swift versions declared by the manifest.
public let declaredSwiftVersions: [SwiftLanguageVersion]
Expand All @@ -57,14 +57,14 @@ public final class SwiftTarget: Target {
others: [AbsolutePath] = [],
dependencies: [Target.Dependency] = [],
packageAccess: Bool,
swiftVersion: SwiftLanguageVersion,
toolsSwiftVersion: SwiftLanguageVersion,
declaredSwiftVersions: [SwiftLanguageVersion] = [],
buildSettings: BuildSettings.AssignmentTable = .init(),
buildSettingsDescription: [TargetBuildSettingDescription.Setting] = [],
pluginUsages: [PluginUsage] = [],
usesUnsafeFlags: Bool
) {
self.swiftVersion = swiftVersion
self.toolSwiftVersion = toolsSwiftVersion
self.declaredSwiftVersions = declaredSwiftVersions
super.init(
name: name,
Expand Down Expand Up @@ -103,8 +103,8 @@ public final class SwiftTarget: Target {
// We need to select the latest Swift language version that can
// satisfy the current tools version but there is not a good way to
// do that currently.
self.swiftVersion = swiftTestTarget?
.swiftVersion ?? SwiftLanguageVersion(string: String(SwiftVersion.current.major)) ?? .v4
self.toolSwiftVersion = swiftTestTarget?
.toolSwiftVersion ?? SwiftLanguageVersion(string: String(SwiftVersion.current.major)) ?? .v4
self.declaredSwiftVersions = []
let sources = Sources(paths: [testEntryPointPath], root: testEntryPointPath.parentDirectory)

Expand All @@ -129,14 +129,14 @@ public final class SwiftTarget: Target {

override public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(self.swiftVersion, forKey: .swiftVersion)
try container.encode(self.toolSwiftVersion, forKey: .swiftVersion)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaxDesiatov is it okay to rename case in CodingKeys here? I'm not sure how much do we care about backward compatibility here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still unsure where this Codable conformance is used, maybe @neonichu could clarify? I wonder if this could be used in workspace serialization, but also don't know how easily we can break that

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 remember.

But removing the Encodable conformance from PackageModel.Package (which is the only thing broken by removing the conformance from Target) doesn't seem to break anything, it seems like this is potentially something that's no longer being used?

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 can do this in a follow up!

try container.encode(self.declaredSwiftVersions, forKey: .declaredSwiftVersions)
try super.encode(to: encoder)
}

public required init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.swiftVersion = try container.decode(SwiftLanguageVersion.self, forKey: .swiftVersion)
self.toolSwiftVersion = try container.decode(SwiftLanguageVersion.self, forKey: .swiftVersion)
self.declaredSwiftVersions = try container.decode([SwiftLanguageVersion].self, forKey: .declaredSwiftVersions)
try super.init(from: decoder)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SPMTestSupport/ResolvedTarget+Mock.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ extension ResolvedModule {
sources: Sources(paths: [], root: "/"),
dependencies: [],
packageAccess: false,
swiftVersion: .v4,
toolsSwiftVersion: .v4,
usesUnsafeFlags: false
),
dependencies: deps.map { .target($0, conditions: conditions) },
Expand Down
6 changes: 3 additions & 3 deletions Sources/XCBuildSupport/PIFBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1584,19 +1584,19 @@ extension SwiftTarget {
})
// If we were able to determine the list of versions supported by XCBuild, cross-reference with the package's
// Swift versions in case the preferred version isn't available.
if !supportedSwiftVersions.isEmpty, !supportedSwiftVersions.contains(self.swiftVersion) {
if !supportedSwiftVersions.isEmpty, !supportedSwiftVersions.contains(self.toolSwiftVersion) {
let declaredVersions = Array(normalizedDeclaredVersions.intersection(supportedSwiftVersions)).sorted(by: >)
if let swiftVersion = declaredVersions.first {
return swiftVersion
} else {
throw PIFGenerationError.unsupportedSwiftLanguageVersion(
targetName: self.name,
version: self.swiftVersion,
version: self.toolSwiftVersion,
supportedVersions: supportedSwiftVersions
)
}
}
return self.swiftVersion
return self.toolSwiftVersion
}
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/PackageLoadingTests/PackageBuilderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3303,7 +3303,7 @@ final class PackageBuilderTester {
guard case let swiftTarget as SwiftTarget = target else {
return XCTFail("\(target) is not a swift target", file: file, line: line)
}
XCTAssertEqual(SwiftLanguageVersion(string: swiftVersion)!, swiftTarget.swiftVersion, file: file, line: line)
XCTAssertEqual(SwiftLanguageVersion(string: swiftVersion)!, swiftTarget.toolSwiftVersion, file: file, line: line)
}

func check(pluginCapability: PluginCapability, file: StaticString = #file, line: UInt = #line) {
Expand Down