From b12ab5f68ae47584bdb27c1bd682b5eec63d9288 Mon Sep 17 00:00:00 2001 From: tom doron Date: Fri, 3 Sep 2021 16:47:02 -0700 Subject: [PATCH] update the resolved file format motivation: resolved file now encodes the package name which is redundant. it also does not include the identity of the package so that needs to be computed frmo the url and could be incorrect in case of "creative" mirroring. it will also pose issues as we transition to registry based dependencies changes: * create V2 of the resolved file format * in new format, encode identity and not name * adjust tests --- Sources/PackageGraph/PinsStore.swift | 99 +++++++++++++++++------ Tests/WorkspaceTests/PinsStoreTests.swift | 11 ++- 2 files changed, 82 insertions(+), 28 deletions(-) diff --git a/Sources/PackageGraph/PinsStore.swift b/Sources/PackageGraph/PinsStore.swift index 6f66ad61f07..26a85b9f076 100644 --- a/Sources/PackageGraph/PinsStore.swift +++ b/Sources/PackageGraph/PinsStore.swift @@ -122,7 +122,7 @@ fileprivate struct PinsStorage { return try self.fileSystem.withLock(on: self.lockFilePath, type: .shared) { let version = try self.decoder.decode(path: self.path, fileSystem: self.fileSystem, as: Version.self) switch version.version { - case 1: + case V1.version: let v1 = try decoder.decode(path: self.path, fileSystem: self.fileSystem, as: V1.self) return try v1.object.pins.map{ try PinsStore.Pin($0, mirrors: mirrors) }.reduce(into: [PackageIdentity: PinsStore.Pin]()) { partial, iterator in if partial.keys.contains(iterator.packageRef.identity) { @@ -130,6 +130,14 @@ fileprivate struct PinsStorage { } partial[iterator.packageRef.identity] = iterator } + case V2.version: + let v2 = try decoder.decode(path: self.path, fileSystem: self.fileSystem, as: V2.self) + return try v2.pins.map{ try PinsStore.Pin($0, mirrors: mirrors) }.reduce(into: [PackageIdentity: PinsStore.Pin]()) { partial, iterator in + if partial.keys.contains(iterator.packageRef.identity) { + throw StringError("duplicated entry for package \"\(iterator.packageRef.identity)\"") + } + partial[iterator.packageRef.identity] = iterator + } default: throw InternalError("unknown RepositoryManager version: \(version)") } @@ -150,7 +158,7 @@ fileprivate struct PinsStorage { return } - let container = V1(pins: pins, mirrors: mirrors) + let container = V2(pins: pins, mirrors: mirrors) let data = try self.encoder.encode(container) try self.fileSystem.writeFileContents(self.path, data: data) } @@ -172,11 +180,13 @@ fileprivate struct PinsStorage { // v1 storage format struct V1: Codable { + static let version = 1 + let version: Int let object: Container init (pins: PinsStore.PinsMap, mirrors: DependencyMirrors) { - self.version = 1 + self.version = Self.version self.object = .init( pins: pins.values .sorted(by: { $0.packageRef.identity < $1.packageRef.identity }) @@ -200,27 +210,55 @@ fileprivate struct PinsStorage { self.state = .init(pin.state) } } + } - struct CheckoutInfo: Codable { - let revision: String - let branch: String? - let version: String? - - init(_ state: CheckoutState) { - switch state { - case .version(let version, let revision): - self.version = version.description - self.branch = nil - self.revision = revision.identifier - case .branch(let branch, let revision): - self.version = nil - self.branch = branch - self.revision = revision.identifier - case .revision(let revision): - self.version = nil - self.branch = nil - self.revision = revision.identifier - } + // v2 storage format + struct V2: Codable { + static let version = 2 + + let version: Int + let pins: [Pin] + + init (pins: PinsStore.PinsMap, mirrors: DependencyMirrors) { + self.version = Self.version + self.pins = pins.values + .sorted(by: { $0.packageRef.identity < $1.packageRef.identity }) + .map{ Pin($0, mirrors: mirrors) } + } + + struct Pin: Codable { + let identity: PackageIdentity + let location: String + let state: CheckoutInfo + + init(_ pin: PinsStore.Pin, mirrors: DependencyMirrors) { + self.identity = pin.packageRef.identity + // rdar://52529014, rdar://52529011: pin file should store the original location but remap when loading + self.location = mirrors.originalURL(for: pin.packageRef.location) ?? pin.packageRef.location + self.state = .init(pin.state) + } + } + } + + struct CheckoutInfo: Codable { + let revision: String + let branch: String? + let version: String? + + init(_ state: CheckoutState) { + switch state { + case .version(let version, let revision): + self.version = version.description + self.branch = nil + self.revision = revision.identifier + case .branch(let branch, let revision): + self.version = nil + self.branch = branch + self.revision = revision.identifier + case .revision(let revision): + self.version = nil + self.branch = nil + self.revision = revision.identifier } } } @@ -242,8 +280,21 @@ extension PinsStore.Pin { } } +extension PinsStore.Pin { + fileprivate init(_ pin: PinsStorage.V2.Pin, mirrors: DependencyMirrors) throws { + // rdar://52529014, rdar://52529011: pin file should store the original location but remap when loading + let url = mirrors.effectiveURL(for: pin.location) + let identity = pin.identity + let packageRef = PackageReference.remote(identity: identity, location: url) + self.init( + packageRef: packageRef, + state: try .init(pin.state) + ) + } +} + extension CheckoutState { - fileprivate init(_ state: PinsStorage.V1.CheckoutInfo) throws { + fileprivate init(_ state: PinsStorage.CheckoutInfo) throws { let revision: Revision = .init(identifier: state.revision) if let branch = state.branch { self = .branch(name: branch, revision: revision) diff --git a/Tests/WorkspaceTests/PinsStoreTests.swift b/Tests/WorkspaceTests/PinsStoreTests.swift index ab8ae7dd358..3fb44f07be6 100644 --- a/Tests/WorkspaceTests/PinsStoreTests.swift +++ b/Tests/WorkspaceTests/PinsStoreTests.swift @@ -188,11 +188,16 @@ final class PinsStoreTests: XCTestCase { store.pin(packageRef: .remote(identity: fooIdentity, location: fooMirroredURL), state: .version(v1, revision: .init(identifier: "foo-revision"))) - store.pin(packageRef: .remote(identity: barMirroredIdentity, location: barMirroredURL), + store.pin(packageRef: .remote(identity: barIdentity, location: barMirroredURL), state: .version(v1, revision: .init(identifier: "bar-revision"))) store.pin(packageRef: .remote(identity: bazIdentity, location: bazURL), state: .version(v1, revision: .init(identifier: "baz-revision"))) + XCTAssert(store.pinsMap.count == 3) + XCTAssertEqual(store.pinsMap[fooIdentity]!.packageRef.location, fooMirroredURL) + XCTAssertEqual(store.pinsMap[barIdentity]!.packageRef.location, barMirroredURL) + XCTAssertNil(store.pinsMap[barMirroredIdentity]) + XCTAssertEqual(store.pinsMap[bazIdentity]!.packageRef.location, bazURL) try store.saveState() XCTAssert(fileSystem.exists(pinsFile)) @@ -207,8 +212,6 @@ final class PinsStoreTests: XCTestCase { // Load the store again from disk, with mirrors let store3 = try PinsStore(pinsFile: pinsFile, workingDirectory: .root, fileSystem: fileSystem, mirrors: mirrors) XCTAssert(store3.pinsMap.count == 3) - XCTAssertEqual(store3.pinsMap[fooIdentity]!.packageRef.location, fooMirroredURL) - XCTAssertEqual(store3.pinsMap[barMirroredIdentity]!.packageRef.location, barMirroredURL) - XCTAssertEqual(store3.pinsMap[bazIdentity]!.packageRef.location, bazURL) + XCTAssertEqual(store3.pinsMap, store.pinsMap) } }