[5.9] Cross-compilation: fix bundles not unpacked on installation (#6362)

MaxDesiatov · web-flow · commit c18b8c47b776 · 2023-04-03T21:22:22.000+01:00
`swift experimental-destination install` subcommand works with bundle directories, but fails to unpack bundle archives.

We should pass an instance of an archiver to the installation function to unpack destination bundle archives if needed. Since `withTemporaryDirectory` is `async`, `SwiftDestinationTool` had to be converted to `AsyncParsableCommand` and also gain `@main` attribute for that to work. Additionally, `@main` attribute requires `-parse-as-library` passed in CMake.

rdar://107367895
diff --git a/Sources/Basics/Archiver.swift b/Sources/Basics/Archiver.swift
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import _Concurrency
 import TSCBasic
 
 /// The `Archiver` protocol abstracts away the different operations surrounding archives.
@@ -51,3 +52,14 @@ public protocol Archiver {
         completion: @escaping (Result<Bool, Error>) -> Void
     )
 }
+
+extension Archiver {
+    public func extract(
+        from archivePath: AbsolutePath,
+        to destinationPath: AbsolutePath
+    ) async throws {
+        try await withCheckedThrowingContinuation {
+            self.extract(from: archivePath, to: destinationPath, completion: $0.resume(with:))
+        }
+    }
+}
diff --git a/Sources/Basics/CMakeLists.txt b/Sources/Basics/CMakeLists.txt
@@ -26,6 +26,7 @@ add_library(Basics
   FileSystem/AsyncFileSystem.swift
   FileSystem/FileSystem+Extensions.swift
   FileSystem/Path+Extensions.swift
+  FileSystem/TemporaryFile.swift
   FileSystem/VFSOverlay.swift
   HTTPClient/HTTPClient.swift
   HTTPClient/HTTPClientConfiguration.swift
diff --git a/Sources/CrossCompilationDestinationsTool/Configuration/ConfigureDestination.swift b/Sources/CrossCompilationDestinationsTool/Configuration/ConfigureDestination.swift
@@ -12,8 +12,8 @@
 
 import ArgumentParser
 
-struct ConfigureDestination: ParsableCommand {
-    static let configuration = CommandConfiguration(
+public struct ConfigureDestination: ParsableCommand {
+    public static let configuration = CommandConfiguration(
         commandName: "configuration",
         abstract: """
         Manages configuration options for installed cross-compilation destinations.
@@ -24,4 +24,6 @@ struct ConfigureDestination: ParsableCommand {
             ShowConfiguration.self,
         ]
     )
+
+    public init() {}
 }
diff --git a/Sources/CrossCompilationDestinationsTool/DestinationCommand.swift b/Sources/CrossCompilationDestinationsTool/DestinationCommand.swift
@@ -22,7 +22,7 @@ import var TSCBasic.localFileSystem
 import var TSCBasic.stdoutStream
 
 /// A protocol for functions and properties common to all destination subcommands.
-protocol DestinationCommand: ParsableCommand {
+protocol DestinationCommand: AsyncParsableCommand {
     /// Common locations options provided by ArgumentParser.
     var locations: LocationOptions { get }
 
@@ -35,7 +35,7 @@ protocol DestinationCommand: ParsableCommand {
         buildTimeTriple: Triple,
         _ destinationsDirectory: AbsolutePath,
         _ observabilityScope: ObservabilityScope
-    ) throws
+    ) async throws
 }
 
 extension DestinationCommand {
@@ -62,7 +62,7 @@ extension DestinationCommand {
         return destinationsDirectory
     }
 
-    public func run() throws {
+    public func run() async throws {
         let observabilityHandler = SwiftToolObservabilityHandler(outputStream: stdoutStream, logLevel: .info)
         let observabilitySystem = ObservabilitySystem(observabilityHandler)
         let observabilityScope = observabilitySystem.topScope
@@ -73,7 +73,7 @@ extension DestinationCommand {
 
         var commandError: Error? = nil
         do {
-            try self.run(buildTimeTriple: triple, destinationsDirectory, observabilityScope)
+            try await self.run(buildTimeTriple: triple, destinationsDirectory, observabilityScope)
             if observabilityScope.errorsReported {
                 throw ExitCode.failure
             }
diff --git a/Sources/CrossCompilationDestinationsTool/InstallDestination.swift b/Sources/CrossCompilationDestinationsTool/InstallDestination.swift
@@ -21,8 +21,8 @@ import var TSCBasic.localFileSystem
 import var TSCBasic.stdoutStream
 import func TSCBasic.tsc_await
 
-struct InstallDestination: DestinationCommand {
-    static let configuration = CommandConfiguration(
+public struct InstallDestination: DestinationCommand {
+    public static let configuration = CommandConfiguration(
         commandName: "install",
         abstract: """
         Installs a given destination artifact bundle to a location discoverable by SwiftPM. If the artifact bundle \
@@ -36,15 +36,18 @@ struct InstallDestination: DestinationCommand {
     @Argument(help: "A local filesystem path or a URL of an artifact bundle to install.")
     var bundlePathOrURL: String
 
+    public init() {}
+
     func run(
         buildTimeTriple: Triple,
         _ destinationsDirectory: AbsolutePath,
         _ observabilityScope: ObservabilityScope
-    ) throws {
-        try DestinationBundle.install(
+    ) async throws {
+        try await DestinationBundle.install(
             bundlePathOrURL: bundlePathOrURL,
             destinationsDirectory: destinationsDirectory,
             self.fileSystem,
+            ZipArchiver(fileSystem: self.fileSystem),
             observabilityScope
         )
     }
diff --git a/Sources/CrossCompilationDestinationsTool/ListDestinations.swift b/Sources/CrossCompilationDestinationsTool/ListDestinations.swift
@@ -18,7 +18,7 @@ import SPMBuildCore
 
 import struct TSCBasic.AbsolutePath
 
-struct ListDestinations: DestinationCommand {
+public struct ListDestinations: DestinationCommand {
     public static let configuration = CommandConfiguration(
         commandName: "list",
         abstract:
@@ -30,6 +30,8 @@ struct ListDestinations: DestinationCommand {
     @OptionGroup()
     var locations: LocationOptions
 
+    public init() {}
+
     func run(
         buildTimeTriple: Triple,
         _ destinationsDirectory: AbsolutePath,
diff --git a/Sources/CrossCompilationDestinationsTool/RemoveDestination.swift b/Sources/CrossCompilationDestinationsTool/RemoveDestination.swift
@@ -17,8 +17,8 @@ import PackageModel
 
 import struct TSCBasic.AbsolutePath
 
-struct RemoveDestination: DestinationCommand {
-    static let configuration = CommandConfiguration(
+public struct RemoveDestination: DestinationCommand {
+    public static let configuration = CommandConfiguration(
         commandName: "remove",
         abstract: """
         Removes a previously installed destination artifact bundle from the filesystem.
@@ -31,6 +31,8 @@ struct RemoveDestination: DestinationCommand {
     @Argument(help: "Name of the destination artifact bundle or ID of the destination to remove from the filesystem.")
     var destinationIDOrBundleName: String
 
+    public init() {}
+
     func run(
         buildTimeTriple: Triple,
         _ destinationsDirectory: AbsolutePath,
diff --git a/Sources/CrossCompilationDestinationsTool/SwiftDestinationTool.swift b/Sources/CrossCompilationDestinationsTool/SwiftDestinationTool.swift
@@ -13,7 +13,7 @@
 import ArgumentParser
 import Basics
 
-public struct SwiftDestinationTool: ParsableCommand {
+public struct SwiftDestinationTool: AsyncParsableCommand {
     public static let configuration = CommandConfiguration(
         commandName: "experimental-destination",
         _superCommandName: "swift",
diff --git a/Sources/PackageModel/Destination.swift b/Sources/PackageModel/Destination.swift
@@ -27,6 +27,9 @@ public enum DestinationError: Swift.Error {
     /// The schema version is invalid.
     case invalidSchemaVersion
 
+    /// Name of the destination bundle is not valid.
+    case invalidBundleName(String)
+
     /// No valid destinations were decoded from a destination file.
     case noDestinationsDecoded(AbsolutePath)
 
@@ -58,6 +61,10 @@ extension DestinationError: CustomStringConvertible {
             return "unsupported destination file schema version"
         case .invalidInstallation(let problem):
             return problem
+        case .invalidBundleName(let name):
+            return """
+            invalid bundle name `\(name)`, unpacked destination bundles are expected to have `.artifactbundle` extension
+            """
         case .noDestinationsDecoded(let path):
             return "no valid destinations were decoded from a destination file at path `\(path)`"
         case .pathIsNotDirectory(let path):
diff --git a/Sources/PackageModel/DestinationBundle.swift b/Sources/PackageModel/DestinationBundle.swift
@@ -11,9 +11,12 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import TSCBasic
 
+import func TSCBasic.tsc_await
+import protocol TSCBasic.FileSystem
 import struct Foundation.URL
+import struct TSCBasic.AbsolutePath
+import struct TSCBasic.RegEx
 
 /// Represents an `.artifactbundle` on the filesystem that contains cross-compilation destinations.
 public struct DestinationBundle {
@@ -126,57 +129,110 @@ public struct DestinationBundle {
     /// - Parameters:
     ///   - bundlePathOrURL: A string passed on the command line, which is either an absolute or relative to a current
     ///   working directory path, or a URL to a destination artifact bundle.
-    ///   - destinationsDirectory: a directory where the destination artifact bundle should be installed.
-    ///   - fileSystem: file system on which all of the file operations should run.
-    ///   - observabilityScope: observability scope for reporting warnings and errors.
+    ///   - destinationsDirectory: A directory where the destination artifact bundle should be installed.
+    ///   - fileSystem: File system on which all of the file operations should run.
+    ///   - observabilityScope: Observability scope for reporting warnings and errors.
     public static func install(
         bundlePathOrURL: String,
         destinationsDirectory: AbsolutePath,
         _ fileSystem: some FileSystem,
+        _ archiver: some Archiver,
         _ observabilityScope: ObservabilityScope
-    ) throws {
-        let installedBundlePath: AbsolutePath
-
-        if
-            let bundleURL = URL(string: bundlePathOrURL),
-            let scheme = bundleURL.scheme,
-            scheme == "http" || scheme == "https"
-        {
-            let response = try tsc_await { (completion: @escaping (Result<HTTPClientResponse, Error>) -> Void) in
-                let client = LegacyHTTPClient()
-                client.execute(
-                    .init(method: .get, url: bundleURL),
+    ) async throws {
+        _ = try await withTemporaryDirectory(
+            fileSystem: fileSystem,
+            removeTreeOnDeinit: true
+        ) { temporaryDirectory in
+            let bundlePath: AbsolutePath
+
+            if
+                let bundleURL = URL(string: bundlePathOrURL),
+                let scheme = bundleURL.scheme,
+                scheme == "http" || scheme == "https"
+            {
+                let bundleName = bundleURL.lastPathComponent
+                let downloadedBundlePath = temporaryDirectory.appending(component: bundleName)
+
+                let client = HTTPClient()
+                var request = HTTPClientRequest.download(
+                    url: bundleURL,
+                    fileSystem: AsyncFileSystem { fileSystem },
+                    destination: downloadedBundlePath
+                )
+                request.options.validResponseCodes = [200]
+                _ = try await client.execute(
+                    request,
                     observabilityScope: observabilityScope,
-                    progress: nil,
-                    completion: completion
+                    progress: nil
                 )
-            }
 
-            guard let body = response.body else {
-                throw StringError("No downloadable data available at URL `\(bundleURL)`.")
-            }
+                bundlePath = downloadedBundlePath
 
-            let fileName = bundleURL.lastPathComponent
-            installedBundlePath = destinationsDirectory.appending(component: fileName)
+                print("Destination artifact bundle successfully downloaded from `\(bundleURL)`.")
+            } else if
+                let cwd = fileSystem.currentWorkingDirectory,
+                let originalBundlePath = try? AbsolutePath(validating: bundlePathOrURL, relativeTo: cwd)
+            {
+                bundlePath = originalBundlePath
+            } else {
+                throw DestinationError.invalidPathOrURL(bundlePathOrURL)
+            }
 
-            try fileSystem.writeFileContents(installedBundlePath, data: body)
-        } else if
-            let cwd = fileSystem.currentWorkingDirectory,
-            let originalBundlePath = try? AbsolutePath(validating: bundlePathOrURL, relativeTo: cwd)
-        {
-            try installIfValid(
-                bundlePath: originalBundlePath,
+            try await installIfValid(
+                bundlePath: bundlePath,
                 destinationsDirectory: destinationsDirectory,
+                temporaryDirectory: temporaryDirectory,
                 fileSystem,
+                archiver,
                 observabilityScope
             )
-        } else {
-            throw DestinationError.invalidPathOrURL(bundlePathOrURL)
+        }.value
+
+        print("Destination artifact bundle at `\(bundlePathOrURL)` successfully installed.")
+    }
+
+    /// Unpacks a destination bundle if it has an archive extension in its filename.
+    /// - Parameters:
+    ///   - bundlePath: Absolute path to a destination bundle to unpack if needed.
+    ///   - temporaryDirectory: Absolute path to a temporary directory in which the bundle can be unpacked if needed.
+    ///   - fileSystem: A file system to operate on that contains the given paths.
+    ///   - archiver: Archiver to use for unpacking.
+    /// - Returns: Path to an unpacked destination bundle if unpacking is needed, value of `bundlePath` is returned
+    /// otherwise.
+    private static func unpackIfNeeded(
+        bundlePath: AbsolutePath,
+        destinationsDirectory: AbsolutePath,
+        temporaryDirectory: AbsolutePath,
+        _ fileSystem: some FileSystem,
+        _ archiver: some Archiver
+    ) async throws -> AbsolutePath {
+        let regex = try RegEx(pattern: "(.+\\.artifactbundle).*")
+
+        guard let bundleName = bundlePath.components.last else {
+            throw DestinationError.invalidPathOrURL(bundlePath.pathString)
         }
 
-        observabilityScope.emit(info: "Destination artifact bundle at `\(bundlePathOrURL)` successfully installed.")
+        guard let unpackedBundleName = regex.matchGroups(in: bundleName).first?.first else {
+            throw DestinationError.invalidBundleName(bundleName)
+        }
+
+        let installedBundlePath = destinationsDirectory.appending(component: unpackedBundleName)
+        guard !fileSystem.exists(installedBundlePath) else {
+            throw DestinationError.destinationBundleAlreadyInstalled(bundleName: unpackedBundleName)
+        }
+
+        print("\(bundleName) is assumed to be an archive, unpacking...")
+
+        // If there's no archive extension on the bundle name, assuming it's not archived and returning the same path.
+        guard unpackedBundleName != bundleName else {
+            return bundlePath
+        }
+
+        try await archiver.extract(from: bundlePath, to: temporaryDirectory)
+
+        return temporaryDirectory.appending(component: unpackedBundleName)
     }
-    
+
     /// Installs an unpacked destination bundle to a destinations installation directory.
     /// - Parameters:
     ///   - bundlePath: absolute path to an unpacked destination bundle directory.
@@ -186,23 +242,30 @@ public struct DestinationBundle {
     private static func installIfValid(
         bundlePath: AbsolutePath,
         destinationsDirectory: AbsolutePath,
+        temporaryDirectory: AbsolutePath,
         _ fileSystem: some FileSystem,
+        _ archiver: some Archiver,
         _ observabilityScope: ObservabilityScope
-    ) throws {
+    ) async throws {
+        let unpackedBundlePath = try await unpackIfNeeded(
+            bundlePath: bundlePath,
+            destinationsDirectory: destinationsDirectory,
+            temporaryDirectory: temporaryDirectory,
+            fileSystem,
+            archiver
+        )
+
         guard
-            fileSystem.isDirectory(bundlePath),
-            let bundleName = bundlePath.components.last
+            fileSystem.isDirectory(unpackedBundlePath),
+            let bundleName = unpackedBundlePath.components.last
         else {
             throw DestinationError.pathIsNotDirectory(bundlePath)
         }
 
         let installedBundlePath = destinationsDirectory.appending(component: bundleName)
-        guard !fileSystem.exists(installedBundlePath) else {
-            throw DestinationError.destinationBundleAlreadyInstalled(bundleName: bundleName)
-        }
 
         let validatedBundle = try Self.parseAndValidate(
-            bundlePath: bundlePath,
+            bundlePath: unpackedBundlePath,
             fileSystem: fileSystem,
             observabilityScope: observabilityScope
         )
@@ -226,7 +289,7 @@ public struct DestinationBundle {
             }
         }
 
-        try fileSystem.copy(from: bundlePath, to: installedBundlePath)
+        try fileSystem.copy(from: unpackedBundlePath, to: installedBundlePath)
     }
 
     /// Parses metadata of an `.artifactbundle` and validates it as a bundle containing
diff --git a/Sources/swift-experimental-destination/CMakeLists.txt b/Sources/swift-experimental-destination/CMakeLists.txt
diff --git a/Sources/swift-experimental-destination/SwiftDestinationTool.swift b/Sources/swift-experimental-destination/SwiftDestinationTool.swift
diff --git a/Sources/swift-experimental-destination/main.swift b/Sources/swift-experimental-destination/main.swift
diff --git a/Tests/PackageModelTests/DestinationBundleTests.swift b/Tests/PackageModelTests/DestinationBundleTests.swift

Original file line number	Diff line number	Diff line change
`@@ -12,8 +12,8 @@`
`12`	`12`
`13`	`13`	`import ArgumentParser`
`14`	`14`
`15`		`-struct ConfigureDestination: ParsableCommand {`
`16`		`- static let configuration = CommandConfiguration(`
	`15`	`+public struct ConfigureDestination: ParsableCommand {`
	`16`	`+ public static let configuration = CommandConfiguration(`
`17`	`17`	`commandName: "configuration",`
`18`	`18`	`abstract: """`
`19`	`19`	`Manages configuration options for installed cross-compilation destinations.`
`@@ -24,4 +24,6 @@ struct ConfigureDestination: ParsableCommand {`
`24`	`24`	`ShowConfiguration.self,`
`25`	`25`	`]`
`26`	`26`	`)`
	`27`	`+`
	`28`	`+ public init() {}`
`27`	`29`	`}`