Commands: add `experimental-destination` command #5911

MaxDesiatov · 2022-11-17T14:21:05Z

Motivation:

Also adds experimental-destination list subcommand, which prints a list of available destinations. Subcommands that allow adding and removing destinations will be added in future PRs.

Modifications:

New DestinationCommand and ListDestinations types are added, conforming to ParsableCommand. They don't conform to SwiftTool, so the new subcommand can be invoked in an arbitrary directory that doesn't contain a Package.swift manifest. ArtifactsArchiveMetadata schema version validation is implemented to accept new 1.1 schema version with the new destinations bundle type.

Result:

swift experimental-destination list prints a list of destination bundles with valid metadata in ~/.swiftpm/destinations.

MaxDesiatov · 2022-11-17T14:21:46Z

@swift-ci please smoke test

MaxDesiatov · 2022-11-17T14:35:08Z

@swift-ci please smoke test

tomerd · 2022-11-17T22:09:26Z

Sources/SPMBuildCore/ArtifactsArchiveMetadata.swift

+                usesLenientParsing: true
+            )
+
+            guard version <= Version(1, 1, 0) else {


I think this pattern is a bit nicer: https://github.com/apple/swift-package-manager/blob/main/Sources/Workspace/Workspace%2BState.swift#L105

MaxDesiatov · 2022-11-22T00:16:06Z

@swift-ci please smoke test

MaxDesiatov · 2022-11-22T10:51:49Z

@swift-ci please smoke test

neonichu · 2022-11-28T18:31:17Z

Hm, I'm not sure it makes sense to have a new subcommand of swift package that doesn't actually operate on a package, that's supposed to be the one defining trait of swift package. Should we make this a new top-level command, e.g. swift build-destinations or swift package-destinations? This would also naturally avoid the "experimental" naming since it could just not be installed for now.

MaxDesiatov · 2022-11-28T18:59:07Z

Ah, great point. This was supposed to be a top-level command, but I didn't know one could be created without automatically being added to the installation list. And then swift package seemed to be the most adjacent.

Thanks for the suggestion, I'll move it to the top level.

MaxDesiatov · 2022-11-28T19:02:46Z

swift build-destinations or swift package-destinations?

@neonichu WDYT of swift destination? I also think it should be singular in the name, all other commands we have use singular form for nouns. For comparison, in Rust it looks like rustup target, so maybe we could keep this new command's name short and memorable if doesn't need experimental- prefix for now?

@tomerd also interested in your opinion on this.

tomerd · 2022-11-28T21:52:09Z

consistency is good. we should run this by the community as part of the pitch

MaxDesiatov · 2022-11-30T12:22:16Z

Thinking about it more, would it make more sense to install the new top-level tool after all, just keeping it named experimental-destination? I wonder if not installing would imply users have to build it on their own locally, which is a bit too high barrier for just trying it out.

MaxDesiatov · 2022-12-06T21:05:01Z

Confirmed with @tomerd that he's fine with keeping this as a subcommand of swift package. The benefit of it is that we don't have to make installation changes in multiple places for a new top-level command, and that may not be worth the effort for an experimental command (for now).

If/when it goes through an evolution proposal, we can graduate it to a top-level command.

This is ready for review then.

neonichu · 2022-12-07T00:39:43Z

Sources/Basics/FileSystem+Extensions.swift

            }
        }
    }

    fileprivate var dotSwiftPMCrossCompilationDestinationsDirectory: AbsolutePath {
        get throws {
-            return try self.dotSwiftPM.appending(component: crossCompilationDestinationsDirectoryName)
+            return try dotSwiftPM.appending(component: crossCompilationDestinationsDirectoryName)


Not sure this feels right. I think on macOS, we probably want this to be inside ~/Library/Application\ Support/org.swift.swiftpm and not sure what a canonical place for other OSes would be.

I'm modeling this after all other path options boilerplate, for which a symlink is created to idiomaticSwiftPMDirectory where possible.

Sure, but isn't the primary location ~/Library/org.swift.swiftpm right now?

That's correct. Is your point that dotSwiftPMCrossCompilationDestinationsDirectory property should be deleted altogether? It's only called from swiftPMCrossCompilationDestinationsDirectory, which checks if ~/Library/org.swift.swiftpm is a valid path on the host platform and prefers that one.

I was wondering whether these should be in "Application Support" instead since they're more "data" than "configuration", but now looking at it, we should probably not have a ~/Library/org.swift.swiftpm in the first place, but rather place things into subdirectories of "Preferences" and "Application Support"? Let's not worry about it for this PR.

neonichu

I still think it's wrong to break the meaning of swift package, even for an experimental use case, but the changes themselves look good to me.

Figuring out the exact storage location is also something we could leave for the proposal review.

MaxDesiatov · 2022-12-07T09:49:09Z

I still think it's wrong to break the meaning of swift package, even for an experimental use case

Do you think there's any other existing command we could use instead? Or would you still prefer this to be an installed top-level experimental command?

Also adds `experimental-destination list` subcommand, which prints a list of available destinations. `ArtifactsArchiveMetadata` schema version validation is implemented to accept new 1.1 schema version with the new destinations bundle type.

MaxDesiatov · 2022-12-12T17:03:22Z

@swift-ci please smoke test

MaxDesiatov · 2022-12-12T17:04:58Z

@neonichu would you mind having another look after I made it a top-level command? I only want to make sure I didn't miss anything on that front.

MaxDesiatov · 2022-12-12T17:10:39Z

Also, I didn't add a CMakeLists.txt for the new targets. In my understanding, the new tool isn't needed in the initial stage of the bootstrap build, right?

neonichu · 2022-12-15T21:18:38Z

Also, I didn't add a CMakeLists.txt for the new targets. In my understanding, the new tool isn't needed in the initial stage of the bootstrap build, right?

Would we want it to be available on Windows? That is only using the CMake build so anything not covered by it won't be available there.

MaxDesiatov · 2022-12-15T22:35:12Z

Hm, I thought Windows builds only the bootstrap tool with CMake and then builds the rest with SwiftPM, doesn't it?

Either way, it's of no use for Windows at the moment, we don't have generator code for Windows as a host platform yet.

MaxDesiatov requested review from abertelrud, neonichu, tomerd and elsh as code owners November 17, 2022 14:21

MaxDesiatov self-assigned this Nov 17, 2022

tomerd reviewed Nov 17, 2022

View reviewed changes

MaxDesiatov mentioned this pull request Nov 18, 2022

Clean up non-core commands #5917

Merged

MaxDesiatov mentioned this pull request Nov 22, 2022

SwiftTool: introduce --experimental-destination-selector option #5922

Merged

MaxDesiatov added ready Author believes the PR is ready to be merged & any feedback has been addressed enhancement and removed ready Author believes the PR is ready to be merged & any feedback has been addressed labels Nov 23, 2022

MaxDesiatov requested a review from tomerd November 23, 2022 23:11

neonichu reviewed Dec 7, 2022

View reviewed changes

neonichu approved these changes Dec 7, 2022

View reviewed changes

MaxDesiatov added 5 commits December 12, 2022 16:09

Refactor DestinationCommand.ListDestinations

4750207

Introduce DestinationsBundle to SPMBuildCore

2ed4589

Move more parsing code to DestinationsBundle.swift

41a7476

Clean up variable naming, fix CMake filename mismatch

56c7b52

Add swift-experimental-destination tool

23e8991

MaxDesiatov force-pushed the maxd/list-destinations branch from 13ec06e to 23e8991 Compare December 12, 2022 17:02

MaxDesiatov requested a review from neonichu December 12, 2022 17:04

neonichu approved these changes Dec 15, 2022

View reviewed changes

MaxDesiatov merged commit d632f36 into main Dec 15, 2022

MaxDesiatov deleted the maxd/list-destinations branch December 15, 2022 22:36

MaxDesiatov added the cross-compilation label Aug 23, 2023

Lishkk approved these changes May 28, 2025

View reviewed changes

Commands: add experimental-destination command #5911

Commands: add experimental-destination command #5911

Uh oh!

Conversation

MaxDesiatov commented Nov 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation:

Modifications:

Result:

Uh oh!

MaxDesiatov commented Nov 17, 2022

Uh oh!

MaxDesiatov commented Nov 17, 2022

Uh oh!

tomerd Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov commented Nov 22, 2022

Uh oh!

MaxDesiatov commented Nov 22, 2022

Uh oh!

neonichu commented Nov 28, 2022

Uh oh!

MaxDesiatov commented Nov 28, 2022

Uh oh!

MaxDesiatov commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomerd commented Nov 28, 2022

Uh oh!

MaxDesiatov commented Nov 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxDesiatov commented Dec 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neonichu Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov Dec 15, 2022

Choose a reason for hiding this comment

Uh oh!

neonichu Dec 15, 2022

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov Dec 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neonichu Dec 15, 2022

Choose a reason for hiding this comment

Uh oh!

neonichu left a comment

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov commented Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxDesiatov commented Dec 12, 2022

Uh oh!

MaxDesiatov commented Dec 12, 2022

Uh oh!

MaxDesiatov commented Dec 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neonichu commented Dec 15, 2022

Uh oh!

MaxDesiatov commented Dec 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Commands: add `experimental-destination` command #5911

Commands: add `experimental-destination` command #5911

MaxDesiatov commented Nov 17, 2022 •

edited

Loading

MaxDesiatov commented Nov 28, 2022 •

edited

Loading

MaxDesiatov commented Nov 30, 2022 •

edited

Loading

MaxDesiatov commented Dec 6, 2022 •

edited

Loading

neonichu Dec 7, 2022 •

edited

Loading

MaxDesiatov Dec 15, 2022 •

edited

Loading

MaxDesiatov commented Dec 7, 2022 •

edited

Loading

MaxDesiatov commented Dec 12, 2022 •

edited

Loading

MaxDesiatov commented Dec 15, 2022 •

edited

Loading