fix package.resolved interplay with mirrors

Sources/Commands/SwiftPackageTool.swift

@@ -788,7 +788,7 @@ extension SwiftPackageTool.Config {
                 throw ExitCode.failure
             }
 
-            if let mirror = config.mirrors.getMirror(forURL: originalURL) {
+            if let mirror = config.mirrors.mirrorURL(for: originalURL) {
                 print(mirror)
             } else {
                 stderrStream <<< "not found\n"

Sources/PackageGraph/DependencyMirrors.swift

@@ -22,25 +22,30 @@ public final class DependencyMirrors {
         case mirrorNotFound
     }
 
-    private var storage: [String: String]
+    private var index: [String: String]
+    private var reverseIndex: [String: String]
 
     private init(_ mirrors: [Mirror]) {
-        self.storage = Dictionary(mirrors.map({ ($0.original, $0.mirror) }), uniquingKeysWith: { first, _ in first })
+        self.index = Dictionary(mirrors.map({ ($0.original, $0.mirror) }), uniquingKeysWith: { first, _ in first })
+        self.reverseIndex = Dictionary(mirrors.map({ ($0.mirror, $0.original) }), uniquingKeysWith: { first, _ in first })
     }
 
     /// Sets a mirror URL for the given URL.
     public func set(mirrorURL: String, forURL url: String) {
-        storage[url] = mirrorURL
+        self.index[url] = mirrorURL
+        self.reverseIndex[mirrorURL] = url
     }
 
     /// Unsets a mirror for the given URL.
     /// - Parameter originalOrMirrorURL: The original URL or the mirrored URL
     /// - Throws: `Error.mirrorNotFound` if no mirror exists for the provided URL.
     public func unset(originalOrMirrorURL: String) throws {
-        if storage.keys.contains(originalOrMirrorURL) {
-            storage[originalOrMirrorURL] = nil
-        } else if let mirror = storage.first(where: { $0.value == originalOrMirrorURL }) {
-            storage[mirror.key] = nil
+        if let value = self.index[originalOrMirrorURL] {
+            self.index[originalOrMirrorURL] = nil
+            self.reverseIndex[value] = nil
+        } else if let mirror = self.index.first(where: { $0.value == originalOrMirrorURL }) {
+            self.index[mirror.key] = nil
+            self.reverseIndex[originalOrMirrorURL] = nil
         } else {
             throw Error.mirrorNotFound
         }
@@ -49,36 +54,44 @@ public final class DependencyMirrors {
     /// Returns the mirrored URL for a package dependency URL.
     /// - Parameter url: The original URL
     /// - Returns: The mirrored URL, if one exists.
-    public func getMirror(forURL url: String) -> String? {
-        return storage[url]
+    public func mirrorURL(for url: String) -> String? {
+        return self.index[url]
     }
 
     /// Returns the effective URL for a package dependency URL.
     /// - Parameter url: The original URL
     /// - Returns: The mirrored URL if it exists, otherwise the original URL.
-    public func effectiveURL(forURL url: String) -> String {
-        return getMirror(forURL: url) ?? url
+    public func effectiveURL(for url: String) -> String {
+        return self.mirrorURL(for: url) ?? url
     }
+
+    /// Returns the original URL for a mirrored package dependency URL.
+    /// - Parameter url: The mirror URL
+    /// - Returns: The original URL, if one exists.
+    public func originalURL(for url: String) -> String? {
+        return self.reverseIndex[url]
+    }
+
 }
 
 extension DependencyMirrors: Collection {
     public typealias Index = Dictionary<String, String>.Index
     public typealias Element = String
 
     public var startIndex: Index {
-        storage.startIndex
+        self.index.startIndex
     }
 
     public var endIndex: Index {
-        storage.endIndex
+        self.index.endIndex
     }
 
     public subscript(index: Index) -> Element {
-        storage[index].value
+        self.index[index].value
     }
 
     public func index(after index: Index) -> Index {
-        storage.index(after: index)
+        self.index.index(after: index)
     }
 }
 
@@ -94,7 +107,7 @@ extension DependencyMirrors: JSONMappable, JSONSerializable {
     }
 
     public func toJSON() -> JSON {
-        let mirrors = storage.map { Mirror(original: $0.key, mirror: $0.value) }
+        let mirrors = self.index.map { Mirror(original: $0.key, mirror: $0.value) }
         return .array(mirrors.sorted(by: { $0.original < $1.mirror }).map { $0.toJSON() })
     }
 }

Sources/PackageGraph/PinsStore.swift

@@ -38,10 +38,12 @@ public final class PinsStore {
     static let schemaVersion: Int = 1
 
     /// The path to the pins file.
-    fileprivate let pinsFile: AbsolutePath
+    private let pinsFile: AbsolutePath
 
     /// The filesystem to manage the pin file on.
-    fileprivate var fileSystem: FileSystem
+    private var fileSystem: FileSystem
+
+    private let mirrors: DependencyMirrors
 
     /// The pins map.
     public fileprivate(set) var pinsMap: PinsMap
@@ -58,15 +60,16 @@ public final class PinsStore {
     /// - Parameters:
     ///   - pinsFile: Path to the pins file.
     ///   - fileSystem: The filesystem to manage the pin file on.
-    public init(pinsFile: AbsolutePath, fileSystem: FileSystem) throws {
+    public init(pinsFile: AbsolutePath, fileSystem: FileSystem, mirrors: DependencyMirrors) throws {
         self.pinsFile = pinsFile
         self.fileSystem = fileSystem
         self.persistence = SimplePersistence(
             fileSystem: fileSystem,
             schemaVersion: PinsStore.schemaVersion,
             statePath: pinsFile,
             prettyPrint: true)
-        pinsMap = [:]
+        self.pinsMap = [:]
+        self.mirrors = mirrors
         do {
             _ = try self.persistence.restoreState(self)
         } catch SimplePersistence.Error.restoreFailure(_, let error) {
@@ -85,7 +88,7 @@ public final class PinsStore {
         packageRef: PackageReference,
         state: CheckoutState
     ) {
-        pinsMap[packageRef.identity] = Pin(
+        self.pinsMap[packageRef.identity] = Pin(
             packageRef: packageRef,
             state: state
         )
@@ -95,24 +98,24 @@ public final class PinsStore {
     ///
     /// This will replace any previous pin with same package name.
     public func add(_ pin: Pin) {
-        pinsMap[pin.packageRef.identity] = pin
+        self.pinsMap[pin.packageRef.identity] = pin
     }
 
-    /// Unpin all of the currently pinnned dependencies.
+    /// Unpin all of the currently pinned dependencies.
     ///
     /// This method does not automatically write to state file.
     public func unpinAll() {
         // Reset the pins map.
-        pinsMap = [:]
+        self.pinsMap = [:]
     }
 
     public func saveState() throws {
-        if pinsMap.isEmpty {
+        if self.pinsMap.isEmpty {
             // Remove the pins file if there are zero pins to save.
             //
             // This can happen if all dependencies are path-based or edited
             // dependencies.
-            return try fileSystem.removeFileTree(pinsFile)
+            return try self.fileSystem.removeFileTree(pinsFile)
         }
 
         try self.persistence.saveState(self)
@@ -124,8 +127,15 @@ public final class PinsStore {
 extension PinsStore: JSONSerializable {
     /// Saves the current state of pins.
     public func toJSON() -> JSON {
+        // rdar://52529014, rdar://52529011: pin file should store the original location but remap when loading
+        let pinsWithOriginalLocations = self.pins.map { pin -> Pin in
+            let url = self.mirrors.originalURL(for: pin.packageRef.location) ?? pin.packageRef.location
+            let identity = PackageIdentity(url: url) // FIXME: pin store should also encode identity
+            return Pin(packageRef: .remote(identity: identity, location: url), state: pin.state)
+        }
+
         return JSON([
-            "pins": pins.sorted(by: { $0.packageRef.identity < $1.packageRef.identity }).toJSON(),
+            "pins": pinsWithOriginalLocations.sorted(by: { $0.packageRef.identity < $1.packageRef.identity }).toJSON(),
         ])
     }
 }
@@ -144,9 +154,9 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable {
     /// Convert the pin to JSON.
     public func toJSON() -> JSON {
         return .init([
-            "package": packageRef.name.toJSON(),
-            "repositoryURL": packageRef.location,
-            "state": state
+            "package": self.packageRef.name.toJSON(),
+            "repositoryURL": self.packageRef.location,
+            "state": self.state
         ])
     }
 }
@@ -155,6 +165,13 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable {
 
 extension PinsStore: SimplePersistanceProtocol {
     public func restore(from json: JSON) throws {
-        self.pinsMap = try Dictionary(json.get("pins").map({ ($0.packageRef.identity, $0) }), uniquingKeysWith: { first, _ in throw StringError("duplicated entry for package \"\(first.packageRef.name)\"") })
+        let pins: [Pin] = try json.get("pins")
+        // rdar://52529014, rdar://52529011: pin file should store the original location but remap when loading
+        let pinsWithMirroredLocations = pins.map { pin -> Pin in
+            let url = self.mirrors.effectiveURL(for: pin.packageRef.location)
+            let identity = PackageIdentity(url: url) // FIXME: pin store should also encode identity
+            return Pin(packageRef: .remote(identity: identity, location: url), state: pin.state)
+        }
+        self.pinsMap = try Dictionary(pinsWithMirroredLocations.map({ ($0.packageRef.identity, $0) }), uniquingKeysWith: { first, _ in throw StringError("duplicated entry for package \"\(first.packageRef.name)\"") })
     }
 }

Sources/SPMTestSupport/MockWorkspace.swift

@@ -53,7 +53,7 @@ public final class MockWorkspace {
         self.archiver = archiver
         self.checksumAlgorithm = checksumAlgorithm
         self.config = try config ?? Workspace.Configuration(path: sandbox.appending(component: "swiftpm"), fs: fs)
-        self.identityResolver = DefaultIdentityResolver(locationMapper: self.config.mirrors.effectiveURL(forURL:))
+        self.identityResolver = DefaultIdentityResolver(locationMapper: self.config.mirrors.effectiveURL(for:))
         self.roots = roots
         self.packages = packages
 

Sources/Workspace/Workspace.swift

@@ -307,7 +307,7 @@ public class Workspace {
         self.checkoutsPath = self.dataPath.appending(component: "checkouts")
         self.artifactsPath = self.dataPath.appending(component: "artifacts")
 
-        self.identityResolver = identityResolver ?? DefaultIdentityResolver(locationMapper: config.mirrors.effectiveURL(forURL:))
+        self.identityResolver = identityResolver ?? DefaultIdentityResolver(locationMapper: config.mirrors.effectiveURL(for:))
 
         self.containerProvider = RepositoryPackageContainerProvider(
             repositoryManager: repositoryManager,
@@ -319,7 +319,7 @@ public class Workspace {
         self.fileSystem = fileSystem
 
         self.pinsStore = LoadableResult {
-            try PinsStore(pinsFile: pinsFile, fileSystem: fileSystem)
+            try PinsStore(pinsFile: pinsFile, fileSystem: fileSystem, mirrors: config.mirrors)
         }
         self.state = WorkspaceState(dataPath: dataPath, fileSystem: fileSystem)
     }
@@ -1120,7 +1120,7 @@ extension Workspace {
             requiredIdentities = inputIdentities.union(requiredIdentities)
 
             let availableIdentities: Set<PackageReference> = Set(manifestsMap.map {
-                let url = workspace.config.mirrors.effectiveURL(forURL: $0.1.packageLocation)
+                let url = workspace.config.mirrors.effectiveURL(for: $0.1.packageLocation)
                 return PackageReference(identity: $0.key, kind: $0.1.packageKind, location: url)
             })
             // We should never have loaded a manifest we don't need.

Tests/CommandsTests/PackageToolTests.swift

@@ -691,7 +691,7 @@ final class PackageToolTests: XCTestCase {
             // Try to pin bar at a branch.
             do {
                 try execute("resolve", "bar", "--branch", "YOLO")
-                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
+                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem, mirrors: .init())
                 let state = CheckoutState(revision: yoloRevision, branch: "YOLO")
                 let identity = PackageIdentity(path: barPath)
                 XCTAssertEqual(pinsStore.pinsMap[identity]?.state, state)
@@ -700,7 +700,7 @@ final class PackageToolTests: XCTestCase {
             // Try to pin bar at a revision.
             do {
                 try execute("resolve", "bar", "--revision", yoloRevision.identifier)
-                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
+                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem, mirrors: .init())
                 let state = CheckoutState(revision: yoloRevision)
                 let identity = PackageIdentity(path: barPath)
                 XCTAssertEqual(pinsStore.pinsMap[identity]?.state, state)
@@ -741,7 +741,7 @@ final class PackageToolTests: XCTestCase {
 
             // Test pins file.
             do {
-                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
+                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem, mirrors: .init())
                 XCTAssertEqual(pinsStore.pins.map{$0}.count, 2)
                 for pkg in ["bar", "baz"] {
                     let path = try SwiftPMProduct.packagePath(for: pkg, packageRoot: fooPath)
@@ -760,7 +760,7 @@ final class PackageToolTests: XCTestCase {
             // Try to pin bar.
             do {
                 try execute("resolve", "bar")
-                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
+                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem, mirrors: .init())
                 let identity = PackageIdentity(path: barPath)
                 XCTAssertEqual(pinsStore.pinsMap[identity]?.state.version, "1.2.3")
             }
@@ -784,7 +784,7 @@ final class PackageToolTests: XCTestCase {
             // We should be able to revert to a older version.
             do {
                 try execute("resolve", "bar", "--version", "1.2.3")
-                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
+                let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem, mirrors: .init())
                 let identity = PackageIdentity(path: barPath)
                 XCTAssertEqual(pinsStore.pinsMap[identity]?.state.version, "1.2.3")
                 try checkBar(5)

Tests/FunctionalTests/DependencyResolutionTests.swift

@@ -123,10 +123,11 @@ class DependencyResolutionTests: XCTestCase {
                 XCTAssertTrue(output.stdout.contains("barmirror<\(prefix.pathString)/BarMirror@unspecified"), output.stdout)
                 XCTAssertFalse(output.stdout.contains("bar<\(prefix.pathString)/Bar@unspecified"), output.stdout)
 
+                // rdar://52529014 mirrors should not be reflected in pins file
                 let pins = try String(bytes: localFileSystem.readFileContents(appPinsPath).contents, encoding: .utf8)!
                 XCTAssertTrue(pins.contains("\"\(prefix.pathString)/Foo\""), pins)
-                XCTAssertTrue(pins.contains("\"\(prefix.pathString)/BarMirror\""), pins)
-                XCTAssertFalse(pins.contains("\"\(prefix.pathString)/Bar\""), pins)
+                XCTAssertTrue(pins.contains("\"\(prefix.pathString)/Bar\""), pins)
+                XCTAssertFalse(pins.contains("\"\(prefix.pathString)/BarMirror\""), pins)
 
                 XCTAssertBuilds(appPath)
             }

Tests/PackageGraphTests/PackageGraphTests.swift

@@ -1275,8 +1275,8 @@ class PackageGraphTests: XCTestCase {
         """)
 
         let fs = InMemoryFileSystem(emptyFiles: [])
-        let store = try PinsStore(pinsFile: AbsolutePath("/pins"), fileSystem: fs)
-        XCTAssertThrows(StringError("duplicated entry for package \"Yams\""), { try store.restore(from: json) })
+        let store = try PinsStore(pinsFile: AbsolutePath("/pins"), fileSystem: fs, mirrors: .init())
+        XCTAssertThrows(StringError("duplicated entry for package \"yams\""), { try store.restore(from: json) })
     }
 
     func testTargetDependencies_Pre52() throws {

Tests/PackageGraphTests/PubgrubTests.swift

@@ -2201,7 +2201,7 @@ class DependencyGraphBuilder {
     /// Creates a pins store with the given pins.
     func create(pinsStore pins: [String: (CheckoutState, ProductFilter)]) -> PinsStore {
         let fs = InMemoryFileSystem()
-        let store = try! PinsStore(pinsFile: AbsolutePath("/tmp/Package.resolved"), fileSystem: fs)
+        let store = try! PinsStore(pinsFile: AbsolutePath("/tmp/Package.resolved"), fileSystem: fs, mirrors: .init())
 
         for (package, pin) in pins {
             store.pin(packageRef: reference(for: package), state: pin.0)