Merge 7b6e570c33 into 1a0d3353b9

Xcodes/Backend/AppState+Install.swift

@@ -250,8 +250,17 @@ extension AppState {
 
     func unarchiveAndMoveXIP(availableXcode: AvailableXcode, at source: URL, to destination: URL) -> AnyPublisher<URL, Swift.Error> {
         self.setInstallationStep(of: availableXcode.version, to: .unarchiving)
-        
-        return unxipOrUnxipExperiment(source)
+
+        // Use a version-specific extraction directory to prevent conflicts when
+        // multiple Xcode versions are installed concurrently. Without this, all
+        // XIP files extract to the same parent directory as "Xcode.app", causing
+        // a race condition where one version’s extracted app gets renamed to
+        // another version’s destination path.
+        let extractionDirectory = source.deletingLastPathComponent()
+            .appendingPathComponent("Xcode-\(availableXcode.version)-extract")
+        try? Current.files.createDirectory(at: extractionDirectory, withIntermediateDirectories: true, attributes: nil)
+
+        return unxipOrUnxipExperiment(source, extractionDirectory: extractionDirectory)
             .catch { error -> AnyPublisher<ProcessOutput, Swift.Error> in
                 if let executionError = error as? ProcessExecutionError {
                    if executionError.standardError.contains("damaged and can’t be expanded") {
@@ -269,8 +278,8 @@ extension AppState {
         .tryMap { output -> URL in
             self.setInstallationStep(of: availableXcode.version, to: .moving(destination: destination.path))
 
-            let xcodeURL = source.deletingLastPathComponent().appendingPathComponent("Xcode.app")
-            let xcodeBetaURL = source.deletingLastPathComponent().appendingPathComponent("Xcode-beta.app")
+            let xcodeURL = extractionDirectory.appendingPathComponent("Xcode.app")
+            let xcodeBetaURL = extractionDirectory.appendingPathComponent("Xcode-beta.app")
             if Current.files.fileExists(atPath: xcodeURL.path) {
                 try Current.files.moveItem(at: xcodeURL, to: destination)
             }
@@ -278,6 +287,7 @@ extension AppState {
                 try Current.files.moveItem(at: xcodeBetaURL, to: destination)
             }
 
+            try? Current.files.removeItem(at: extractionDirectory)
             return destination
         }
         .handleEvents(receiveCancel: {
@@ -287,17 +297,20 @@ extension AppState {
             if Current.files.fileExists(atPath: destination.path) {
                 try? Current.files.removeItem(destination)
             }
+            if Current.files.fileExists(atPath: extractionDirectory.path) {
+                try? Current.files.removeItem(extractionDirectory)
+            }
         })
         .eraseToAnyPublisher()
     }
-    
-    func unxipOrUnxipExperiment(_ source: URL) -> AnyPublisher<ProcessOutput, Error> {
+
+    func unxipOrUnxipExperiment(_ source: URL, extractionDirectory: URL) -> AnyPublisher<ProcessOutput, Error> {
         if unxipExperiment {
             // All hard work done by https://github.com/saagarjha/unxip
             // Compiled to binary with `swiftc -parse-as-library -O unxip.swift`
-            return Current.shell.unxipExperiment(source)
+            return Current.shell.unxipExperiment(source, extractionDirectory)
         } else {
-            return Current.shell.unxip(source)
+            return Current.shell.unxip(source, extractionDirectory)
         }
     }
 

Xcodes/Backend/Environment.swift

@@ -25,7 +25,7 @@ public struct Environment {
 public var Current = Environment()
 
 public struct Shell {
-    public var unxip: (URL) -> AnyPublisher<ProcessOutput, Error> = { Process.run(Path.root.usr.bin.xip, workingDirectory: $0.deletingLastPathComponent(), "--expand", "\($0.path)") }
+    public var unxip: (URL, URL) -> AnyPublisher<ProcessOutput, Error> = { xipURL, workingDirectory in Process.run(Path.root.usr.bin.xip, workingDirectory: workingDirectory, "--expand", "\(xipURL.path)") }
     public var spctlAssess: (URL) -> AnyPublisher<ProcessOutput, Error> = { Process.run(Path.root.usr.sbin.spctl, "--assess", "--verbose", "--type", "execute", "\($0.path)") }
     public var codesignVerify: (URL) -> AnyPublisher<ProcessOutput, Error> = { Process.run(Path.root.usr.bin.codesign, "-vv", "-d", "\($0.path)") }
     public var buildVersion: () -> AnyPublisher<ProcessOutput, Error> = { Process.run(Path.root.usr.bin.sw_vers, "-buildVersion") }
@@ -191,9 +191,9 @@ public struct Shell {
     }
     
     
-    public var unxipExperiment: (URL) -> AnyPublisher<ProcessOutput, Error> = { url in
+    public var unxipExperiment: (URL, URL) -> AnyPublisher<ProcessOutput, Error> = { xipURL, workingDirectory in
         let unxipPath = Path(url: Bundle.main.url(forAuxiliaryExecutable: "unxip")!)!
-        return Process.run(unxipPath.url, workingDirectory: url.deletingLastPathComponent(), ["\(url.path)"])
+        return Process.run(unxipPath.url, workingDirectory: workingDirectory, ["\(xipURL.path)"])
     }
     
     public var downloadRuntime: (String, String, String?) -> AsyncThrowingStream<Progress, Error> = { platform, version, architecture in

XcodesTests/AppStateTests.swift

@@ -291,7 +291,7 @@ class AppStateTests: XCTestCase {
     }
 
     func test_Install_NotEnoughFreeSpace() throws {
-        Current.shell.unxip = { _ in
+        Current.shell.unxip = { _, _ in
             Fail(error: ProcessExecutionError(
                     process: Process(),
                     standardOutput: "xip: signing certificate was \"Development Update\" (validation not attempted)", 
@@ -325,4 +325,135 @@ class AppStateTests: XCTestCase {
             XCTFail() 
         }        
     }
+
+    func test_UnarchiveAndMoveXIP_ConcurrentInstalls_DoNotShareExtractionDirectory() throws {
+        enum CollisionError: Error {
+            case extractionDirectoryCollision(String)
+        }
+
+        let stateQueue = DispatchQueue(label: "AppStateTests.ConcurrentInstallState")
+        var existingPaths = Set<String>()
+        var activeExtractionDirectories = Set<String>()
+        var usedExtractionDirectories: [String] = []
+        var failures: [Error] = []
+        var movedDestinations: [String] = []
+        var cancellables = Set<AnyCancellable>()
+
+        Current.files.createDirectory = { directoryURL, _, _ in
+            stateQueue.sync {
+                existingPaths.insert(directoryURL.path)
+            }
+        }
+        Current.files.fileExistsAtPath = { path in
+            stateQueue.sync {
+                existingPaths.contains(path)
+            }
+        }
+        Current.files.moveItem = { source, destination in
+            try stateQueue.sync {
+                guard existingPaths.remove(source.path) != nil else {
+                    throw CocoaError(.fileNoSuchFile)
+                }
+                existingPaths.insert(destination.path)
+            }
+        }
+        Current.files.removeItem = { url in
+            stateQueue.sync {
+                existingPaths.remove(url.path)
+                existingPaths = Set(existingPaths.filter { !$0.hasPrefix(url.path + "/") })
+            }
+        }
+
+        Current.shell.unxip = { _, extractionDirectory in
+            Deferred {
+                Future { promise in
+                    let hasCollision = stateQueue.sync { () -> Bool in
+                        usedExtractionDirectories.append(extractionDirectory.path)
+                        if activeExtractionDirectories.contains(extractionDirectory.path) {
+                            return true
+                        }
+                        activeExtractionDirectories.insert(extractionDirectory.path)
+                        return false
+                    }
+
+                    if hasCollision {
+                        promise(.failure(CollisionError.extractionDirectoryCollision(extractionDirectory.path)))
+                        return
+                    }
+
+                    DispatchQueue.global().asyncAfter(deadline: .now() + 0.05) {
+                        stateQueue.sync {
+                            existingPaths.insert(extractionDirectory.appendingPathComponent("Xcode.app").path)
+                            activeExtractionDirectories.remove(extractionDirectory.path)
+                        }
+                        promise(.success((0, "", "")))
+                    }
+                }
+            }
+            .eraseToAnyPublisher()
+        }
+
+        let sourceDirectory = URL(fileURLWithPath: "/tmp/xcodes-tests", isDirectory: true)
+        let availableXcode16_0 = AvailableXcode(
+            version: Version("16.0.0")!,
+            url: URL(string: "https://developer.apple.com/download/Xcode-16.0.xip")!,
+            filename: "Xcode-16.0.xip",
+            releaseDate: nil
+        )
+        let availableXcode16_1 = AvailableXcode(
+            version: Version("16.1.0")!,
+            url: URL(string: "https://developer.apple.com/download/Xcode-16.1.xip")!,
+            filename: "Xcode-16.1.xip",
+            releaseDate: nil
+        )
+
+        let firstDestination = URL(fileURLWithPath: "/Applications/Xcode-16.0.app")
+        let secondDestination = URL(fileURLWithPath: "/Applications/Xcode-16.1.app")
+
+        let finished = expectation(description: "Both unarchive operations finished")
+        finished.expectedFulfillmentCount = 2
+
+        func subscribe(_ publisher: AnyPublisher<URL, Error>) {
+            publisher
+                .sink(receiveCompletion: { completion in
+                    if case let .failure(error) = completion {
+                        stateQueue.sync {
+                            failures.append(error)
+                        }
+                    }
+                    finished.fulfill()
+                }, receiveValue: { movedURL in
+                    stateQueue.sync {
+                        movedDestinations.append(movedURL.path)
+                    }
+                })
+                .store(in: &cancellables)
+        }
+
+        subscribe(
+            subject.unarchiveAndMoveXIP(
+                availableXcode: availableXcode16_0,
+                at: sourceDirectory.appendingPathComponent("Xcode-16.0.xip"),
+                to: firstDestination
+            )
+        )
+        subscribe(
+            subject.unarchiveAndMoveXIP(
+                availableXcode: availableXcode16_1,
+                at: sourceDirectory.appendingPathComponent("Xcode-16.1.xip"),
+                to: secondDestination
+            )
+        )
+
+        wait(for: [finished], timeout: 2.0)
+
+        XCTAssertTrue(failures.isEmpty, "Expected no extraction directory collisions, but got \(failures)")
+        XCTAssertEqual(Set(movedDestinations), Set([firstDestination.path, secondDestination.path]))
+
+        let expectedExtractionDirectories = Set([
+            sourceDirectory.appendingPathComponent("Xcode-\(availableXcode16_0.version)-extract").path,
+            sourceDirectory.appendingPathComponent("Xcode-\(availableXcode16_1.version)-extract").path
+        ])
+        XCTAssertEqual(Set(usedExtractionDirectories), expectedExtractionDirectories)
+    }
 }

XcodesTests/Environment+Mock.swift

@@ -18,7 +18,7 @@ extension Shell {
     static var processOutputMock: ProcessOutput = (0, "", "")
 
     static var mock = Shell(
-        unxip: { _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() },
+        unxip: { _, _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() },
         spctlAssess: { _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() },
         codesignVerify: { _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() },
         buildVersion: { return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() },