From 603471f8661d0627a152b448c83aede7ea19828a Mon Sep 17 00:00:00 2001
From: Yuta Saito <kateinoigakukun@gmail.com>
Date: Mon, 5 Feb 2024 20:43:13 +0900
Subject: [PATCH] Fix non-tarballed SDK installation with remote URL (#7312)

### Motivation:

4714ea92a introduced a regression where non-tarball SDKs could not be
installed from a remote URL due to the wrong assumption that the
downloaded file would always be a tarball.

This issue exists in 5.10 release branch, so will cherry-pick after
merging this PR.

### Modifications:

This commit fixes the issue by checking the file extension part of the
URL for every supported archive format, then falling back to the tarball
format if no supported extension is found.

### Result:

Non-tarball SDK artifact bundles can be installed from remote URL.
---
 .../SwiftSDKs/test-sdk.artifactbundle.zip     | Bin 0 -> 1056 bytes
 .../SwiftSDKs/SwiftSDKBundleStore.swift       |   3 +-
 .../SwiftSDKBundleTests.swift                 |  74 ++++++++++--------
 3 files changed, 43 insertions(+), 34 deletions(-)
 create mode 100644 Fixtures/SwiftSDKs/test-sdk.artifactbundle.zip

diff --git a/Fixtures/SwiftSDKs/test-sdk.artifactbundle.zip b/Fixtures/SwiftSDKs/test-sdk.artifactbundle.zip
new file mode 100644
index 0000000000000000000000000000000000000000..e6c3a41a9de7d2625e0396b107b7b18d78bc94b7
GIT binary patch
literal 1056
zcmWIWW@h1H0D<lr?{F{!N=PusFqEVgm*^I!Wa}jsm1L$RCYL0Y=B4DM>W7AKGBAHT
zpqBbcWN&I|1vdjD%L`@(1~3r-G*JX-1_y(Bt#^1pf_B_;ATNrMfk6?EiJ5t6`FdH!
z`FUWYr4C^=I<?=HzsW$vwY-kCa<l3L3HEez_ulTpibf^gh*{r4O?dyuW;}L(+WY*@
zz1h1|=JEwhUa?Vy`K4Luyh^`20@bH)u>KFJIhMc{csX%<d#KEg;#?Q~TD9|^)<m!b
za8F76A+z%5ou(h|Tq`*@hyD)#aZ+M+$okH3cioWVygq*$R-8`ToEElilf267mM#2c
zQ^oYUrx_~6%m0u_J2-`R|HGHhXS+^wHd50Eg#aW>uQobGAcv_8o)7>AGbCI=Vag_o
zCtP;|W9oyTvUV!a=sqCUB4BE9d1e|U8sP!#cMxj;uYMN9z_j$ylb$Pgj%>KH=+T)!
zHmcgJs=~I+s<It1o=JgKS)Lw&X^wtbh5_rOba<wD`RI6^GcdlWd-n9X)B5KxpFMd-
zd+i0^^V+U~Y#S9-tu^fTt0(~DAjNLNYL@VT)2xcwi~-(^OmfV)5}gDzyaX8DI)a$E
zlOQW331TEakm<M*AjEWFT4Y$#7=*`kq~r*+29zAJT7xT=kv$Otv<;UhAaM?~0c$=&
mcmy-Xkv+1CfDP#J53~;y{}?`EWdj8`3lL5Rnpn&P;sF4z$}+A1

literal 0
HcmV?d00001

diff --git a/Sources/PackageModel/SwiftSDKs/SwiftSDKBundleStore.swift b/Sources/PackageModel/SwiftSDKs/SwiftSDKBundleStore.swift
index ae2d8ec2bbb..a0cc23569dc 100644
--- a/Sources/PackageModel/SwiftSDKs/SwiftSDKBundleStore.swift
+++ b/Sources/PackageModel/SwiftSDKs/SwiftSDKBundleStore.swift
@@ -151,9 +151,10 @@ public final class SwiftSDKBundleStore {
             {
                 let bundleName: String
                 let fileNameComponent = bundleURL.lastPathComponent
-                if fileNameComponent.hasSuffix(".tar.gz") {
+                if archiver.supportedExtensions.contains(where: { fileNameComponent.hasSuffix($0) }) {
                     bundleName = fileNameComponent
                 } else {
+                    // Assume that the bundle is a tarball if it doesn't have a recognized extension.
                     bundleName = "bundle.tar.gz"
                 }
                 let downloadedBundlePath = temporaryDirectory.appending(component: bundleName)
diff --git a/Tests/PackageModelTests/SwiftSDKBundleTests.swift b/Tests/PackageModelTests/SwiftSDKBundleTests.swift
index 55439f4be68..24295d3a91b 100644
--- a/Tests/PackageModelTests/SwiftSDKBundleTests.swift
+++ b/Tests/PackageModelTests/SwiftSDKBundleTests.swift
@@ -113,11 +113,11 @@ private func generateTestFileSystem(bundleArtifacts: [MockArtifact]) throws -> (
 private let arm64Triple = try! Triple("arm64-apple-macosx13.0")
 private let i686Triple = try! Triple("i686-apple-macosx13.0")
 
-private let fixtureArchivePath = try! AbsolutePath(validating: #file)
+private let fixtureSDKsPath = try! AbsolutePath(validating: #file)
     .parentDirectory
     .parentDirectory
     .parentDirectory
-    .appending(components: ["Fixtures", "SwiftSDKs", "test-sdk.artifactbundle.tar.gz"])
+    .appending(components: ["Fixtures", "SwiftSDKs"])
 
 final class SwiftSDKBundleTests: XCTestCase {
     func testInstallRemote() async throws {
@@ -126,43 +126,51 @@ final class SwiftSDKBundleTests: XCTestCase {
         #endif
 
         let system = ObservabilitySystem.makeForTesting()
-        var output = [SwiftSDKBundleStore.Output]()
         let observabilityScope = system.topScope
         let cancellator = Cancellator(observabilityScope: observabilityScope)
         let archiver = UniversalArchiver(localFileSystem, cancellator)
 
-        let httpClient = HTTPClient { request, _ in
-            guard case let .download(_, downloadPath) = request.kind else {
-                XCTFail("Unexpected HTTPClient.Request.Kind")
-                return .init(statusCode: 400)
+        let fixtureAndURLs: [(url: String, fixture: String)] = [
+            ("https://localhost/archive?test=foo", "test-sdk.artifactbundle.tar.gz"),
+            ("https://localhost/archive.tar.gz", "test-sdk.artifactbundle.tar.gz"),
+            ("https://localhost/archive.zip", "test-sdk.artifactbundle.zip"),
+        ]
+
+        for (bundleURLString, fixture) in fixtureAndURLs {
+            let httpClient = HTTPClient { request, _ in
+                guard case let .download(_, downloadPath) = request.kind else {
+                    XCTFail("Unexpected HTTPClient.Request.Kind")
+                    return .init(statusCode: 400)
+                }
+                let fixturePath = fixtureSDKsPath.appending(component: fixture)
+                try localFileSystem.copy(from: fixturePath, to: downloadPath)
+                return .init(statusCode: 200)
             }
-            try localFileSystem.copy(from: fixtureArchivePath, to: downloadPath)
-            return .init(statusCode: 200)
-        }
 
-        try await withTemporaryDirectory(fileSystem: localFileSystem, removeTreeOnDeinit: true) { tmpDir in
-            let store = SwiftSDKBundleStore(
-                swiftSDKsDirectory: tmpDir,
-                fileSystem: localFileSystem,
-                observabilityScope: observabilityScope,
-                outputHandler: {
-                    output.append($0)
-                }
-            )
-            let bundleURLString = "https://localhost/archive?test=foo"
-            try await store.install(bundlePathOrURL: bundleURLString, archiver, httpClient)
-            
-            let bundleURL = URL(string: bundleURLString)!
-            XCTAssertEqual(output, [
-                .downloadStarted(bundleURL),
-                .downloadFinishedSuccessfully(bundleURL),
-                .unpackingArchive(bundlePathOrURL: bundleURLString),
-                .installationSuccessful(
-                    bundlePathOrURL: bundleURLString,
-                    bundleName: "test-sdk.artifactbundle"
-                ),
-            ])
-        }.value
+            try await withTemporaryDirectory(fileSystem: localFileSystem, removeTreeOnDeinit: true) { tmpDir in
+                var output = [SwiftSDKBundleStore.Output]()
+                let store = SwiftSDKBundleStore(
+                    swiftSDKsDirectory: tmpDir,
+                    fileSystem: localFileSystem,
+                    observabilityScope: observabilityScope,
+                    outputHandler: {
+                        output.append($0)
+                    }
+                )
+                try await store.install(bundlePathOrURL: bundleURLString, archiver, httpClient)
+
+                let bundleURL = URL(string: bundleURLString)!
+                XCTAssertEqual(output, [
+                    .downloadStarted(bundleURL),
+                    .downloadFinishedSuccessfully(bundleURL),
+                    .unpackingArchive(bundlePathOrURL: bundleURLString),
+                    .installationSuccessful(
+                        bundlePathOrURL: bundleURLString,
+                        bundleName: "test-sdk.artifactbundle"
+                    ),
+                ])
+            }.value
+        }
     }
 
     func testInstall() async throws {