Skip to content

Add Swift SDK target triples to swift sdk subcommands #9036

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Swift SDK target triples to swift sdk subcommands #9036

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

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions Sources/PackageModel/SwiftSDKs/SwiftSDKBundle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,31 @@ extension [SwiftSDKBundle] {
public var sortedArtifactIDs: [String] {
self.flatMap(\.artifacts.keys).sorted()
}

/// List all Swift SDKs matching a given host triple from a `self` array of available Swift SDKs.
/// - Parameters:
/// - hostTriple: triple of the host building with these Swift SDKs.
/// - Returns: a dictionary of installed ``SwiftSDK`` artifact IDs, each with an array of target triples, or an empty dictionary.
public func listSwiftSDKs(hostTriple: Triple) -> [String : [Triple]] {
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is this meant to be adopted outside of SwiftPM? If not, it shouldn't be public. Also, please add a doc string to the function declaration to clarify if host triples or target triples are returned.

Suggested change
public func listSwiftSDKs(hostTriple: Triple) -> [String : [Triple]] {
package func listSwiftSDKs(hostTriple: Triple) -> [String : [Triple]] {
Copy link
Member Author

@finagolfin finagolfin Aug 20, 2025

Choose a reason for hiding this comment

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

I simply followed what the sorted list of sortedArtifactIDs was doing above by making it public, as I felt this was more useful than that, will add the doc.
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 20, 2025

Choose a reason for hiding this comment

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

Existing code shouldn't be used as precedent for selecting access control for new code. There's a very high bar for introducing new public APIs in SwiftPM, as evolving it or removing in the future is a high maintenance cost. Unless we know of the exact use case and client adopting this new API, we should not introduce this as a public API in the first place.
Copy link
Member Author

@finagolfin finagolfin Aug 20, 2025

Choose a reason for hiding this comment

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

I feel that this is so generally useful that it should be a public API, do you disagree?
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

SwiftPM has been overly generous with providing public APIs over the years, and given that it isn't semantically versioned none of these APIs could be removed or changed without breaking downstream clients, which in itself is very undesirable experience that it makes it very hard to change or refactor anything in the project.

Unless we have an exact client/project/package that we can point to that absolutely needs this API as public and doesn't have an alternative way of implementing it, nothing should be made public only because it's "generally useful".
Copy link
Member Author

@finagolfin finagolfin Aug 20, 2025

Choose a reason for hiding this comment

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

Alright, will change it.
var sdkTargets: [String: [Triple]] = [:]
for bundle in self {
for (artifactID, variants) in bundle.artifacts {
var triples : [Triple] = []
for variant in variants {
guard variant.isSupporting(hostTriple: hostTriple) else {
continue
}
for sdk in variant.swiftSDKs {
if let triple = sdk.targetTriple {
triples.append(triple)
}
}
}
if !triples.isEmpty {
sdkTargets[artifactID] = triples.sorted(by: { $0.tripleString < $1.tripleString })
}
}
}
return sdkTargets
}
}
1 change: 1 addition & 0 deletions Sources/PackageModel/SwiftSDKs/SwiftSDKConfigurationStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ public final class SwiftSDKConfigurationStore {
}

if showConfiguration {
print("\nswiftSDK: \(targetTriple)")
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 20, 2025

Choose a reason for hiding this comment

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

Why is the key called swiftSDK if its value is target triple?

Suggested change
print("\nswiftSDK: \(targetTriple)")
print("\ntargetTriple: \(targetTriple)")
Copy link
Member Author

@finagolfin finagolfin Aug 20, 2025

Choose a reason for hiding this comment

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

Good point, I chose to stick with the currently used terminology, as the command to actually use the SDK is swift build --swift-sdk aarch64-swift-linux-musl, as can be seen in the static linux doc. Since @al45tair or others chose to use the triple as the selector for the SDKs without using the target triple terminology, eg in the help:

> ./swift-6.2-DEVELOPMENT-SNAPSHOT-2025-08-13-a-fedora39/usr/bin/swift build --help |ag "swift-sdk " --swift-sdk <swift-sdk> Filter for selecting a specific Swift SDK to build

I simply followed the language they used, so as not to confuse users.

However, I now see that the sdk configure help does mention target triples, so I can change this to match that.
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 20, 2025

Choose a reason for hiding this comment

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

Please do change this to triples, this is an opportunity to clean this up in preparation for Swift SDK aliases.
print(swiftSDK.pathsConfiguration)
continue
}
Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftSDKCommand/Configuration/ShowConfiguration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ struct ShowConfiguration: ConfigurationSubcommand {
_ swiftSDKsDirectory: AbsolutePath,
_ observabilityScope: ObservabilityScope
) throws {
print("\nswiftSDK: \(targetTriple)")
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 20, 2025

Choose a reason for hiding this comment

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

ditto

Suggested change
print("\nswiftSDK: \(targetTriple)")
print("\ntargetTriple: \(targetTriple)")
Copy link
Member Author

@finagolfin finagolfin Aug 20, 2025

Choose a reason for hiding this comment

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

Same as above.
print(swiftSDK.pathsConfiguration)
}
}
11 changes: 9 additions & 2 deletions Sources/SwiftSDKCommand/ListSwiftSDKs.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,15 @@ package struct ListSwiftSDKs: SwiftSDKSubcommand {
return
}

for artifactID in validBundles.sortedArtifactIDs {
print(artifactID)
let swiftSDKBundles = validBundles.listSwiftSDKs(hostTriple: hostTriple)
for bundle in validBundles.sortedArtifactIDs {
if let targetTriples = swiftSDKBundles[bundle], !targetTriples.isEmpty {
print("Swift SDK bundle: \(bundle)")
print("Swift SDKs:")
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 20, 2025

Choose a reason for hiding this comment

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

Suggested change
print("Swift SDKs:")
print("Target triples:")
Copy link
Member Author

@finagolfin finagolfin Aug 20, 2025

Choose a reason for hiding this comment

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

On this one I disagree, for the reasons given above, ie users are going to just use the swift sdk list command and plug that into swift build --swift-sdk most of the time, so using the same terminology is better.
Copy link
Contributor

@MaxDesiatov MaxDesiatov Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Retrospectively, enabling use of triples as arguments for --swift-sdk was a mistake that led to confusing and inconsistent behavior. It's already not possible to use triples when targeting Wasm, as we have to ship multiple Swift SDKs with the same triple in the same bundle. We should not dig a deeper hole, but take this as an opportunity to slightly clean it up. In the future I expect vast majority of users to rely on Swift SDK aliases instead of Swift SDK IDs or triples.
Copy link
Member Author

@finagolfin finagolfin Aug 20, 2025

Choose a reason for hiding this comment

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

OK, if I'm gating this behind a flag anyway, I can change the label too.
for triple in targetTriples {
print(" \(triple)")
}
}
}
}
}
4 changes: 4 additions & 0 deletions Tests/CommandsTests/SwiftSDKCommandTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ final class SwiftSDKCommandTests: CommandsTestCase {

XCTAssertEqual(stdout,
"""

swiftSDK: aarch64-unknown-linux-gnu
sdkRootPath: \(fixturePath.appending(components: sdkSubpath))
swiftResourcesPath: not set
swiftStaticResourcesPath: not set
Expand Down Expand Up @@ -223,6 +225,8 @@ final class SwiftSDKCommandTests: CommandsTestCase {

XCTAssertEqual(stdout,
"""

swiftSDK: aarch64-unknown-linux-gnu
sdkRootPath: \(fixturePath.appending(components: sdkSubpath).pathString)
swiftResourcesPath: \(fixturePath.appending("foo"))
swiftStaticResourcesPath: not set
Expand Down