Skip to content

Commit 766ea50

Browse files
matttmattt
committed
Add argument label to public PackageIdentity initializers
Distinguish between path and URL initializers at call sites
1 parent 336730b commit 766ea50

18 files changed

+96
-84
lines changed

‎Sources/PackageCollections/Utility.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ extension PackageReference {
5353
/// Initializes a `PackageReference` from `RepositorySpecifier`
5454
init(repository: RepositorySpecifier, kind: PackageReference.Kind = .remote) {
5555
self.init(
56-
identity: PackageIdentity(repository.url),
56+
identity: PackageIdentity(url: repository.url),
5757
path: repository.url,
5858
kind: kind
5959
)

‎Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ extension PackageGraph {
3838
// the URL but that shouldn't be needed after <rdar://problem/33693433>
3939
// Ensure that identity and package name are the same once we have an
4040
// API to specify identity in the manifest file
41-
let manifestMapSequence = (root.manifests + externalManifests).map({ (PackageIdentity($0.url), $0) })
41+
let manifestMapSequence = (root.manifests + externalManifests).map({ (PackageIdentity(url: $0.url), $0) })
4242
let manifestMap = Dictionary(uniqueKeysWithValues: manifestMapSequence)
4343
let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
4444
node.requiredDependencies().compactMap({ dependency in
4545
let url = mirrors.effectiveURL(forURL: dependency.url)
46-
return manifestMap[PackageIdentity(url)].map { manifest in
46+
return manifestMap[PackageIdentity(url: url)].map { manifest in
4747
GraphLoadingNode(manifest: manifest, productFilter: dependency.productFilter)
4848
}
4949
})
@@ -52,11 +52,11 @@ extension PackageGraph {
5252
// Construct the root manifest and root dependencies set.
5353
let rootManifestSet = Set(root.manifests)
5454
let rootDependencies = Set(root.dependencies.compactMap({
55-
manifestMap[PackageIdentity($0.url)]
55+
manifestMap[PackageIdentity(url: $0.url)]
5656
}))
5757
let rootManifestNodes = root.manifests.map { GraphLoadingNode(manifest: $0, productFilter: .everything) }
5858
let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageDependencyDescription) -> GraphLoadingNode? in
59-
guard let manifest = manifestMap[PackageIdentity(dependency.url)] else { return nil }
59+
guard let manifest = manifestMap[PackageIdentity(url: dependency.url)] else { return nil }
6060
return GraphLoadingNode(manifest: manifest, productFilter: dependency.productFilter)
6161
}
6262
let inputManifests = rootManifestNodes + rootDependencyNodes
@@ -210,7 +210,7 @@ private func createResolvedPackages(
210210

211211
// Create a map of package builders keyed by the package identity.
212212
let packageMapByIdentity: [PackageIdentity: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{
213-
let identity = PackageIdentity($0.package.manifest.url)
213+
let identity = PackageIdentity(url: $0.package.manifest.url)
214214
return (identity, $0)
215215
}
216216
let packageMapByName: [String: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{ ($0.package.name, $0) }
@@ -229,7 +229,7 @@ private func createResolvedPackages(
229229

230230
// Otherwise, look it up by its identity.
231231
let url = mirrors.effectiveURL(forURL: dependency.url)
232-
let resolvedPackage = packageMapByIdentity[PackageIdentity(url)]
232+
let resolvedPackage = packageMapByIdentity[PackageIdentity(url: url)]
233233

234234
// We check that the explicit package dependency name matches the package name.
235235
if let resolvedPackage = resolvedPackage,
@@ -360,10 +360,10 @@ private func createResolvedPackages(
360360
// dependency to share the same name. We don't check this in manifest loading for root-packages so
361361
// we can provide a more detailed diagnostic here.
362362
let referencedPackageURL = mirrors.effectiveURL(forURL: product.packageBuilder.package.manifest.url)
363-
let referencedPackageIdentity = PackageIdentity(referencedPackageURL)
363+
let referencedPackageIdentity = PackageIdentity(url: referencedPackageURL)
364364
let packageDependency = packageBuilder.package.manifest.dependencies.first { package in
365365
let packageURL = mirrors.effectiveURL(forURL: package.url)
366-
let packageIdentity = PackageIdentity(packageURL)
366+
let packageIdentity = PackageIdentity(url: packageURL)
367367
return packageIdentity == referencedPackageIdentity
368368
}!
369369

‎Sources/PackageGraph/PackageGraphRoot.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public struct PackageGraphRoot {
4848
/// Create a package graph root.
4949
public init(input: PackageGraphRootInput, manifests: [Manifest], explicitProduct: String? = nil) {
5050
self.packageRefs = zip(input.packages, manifests).map { (path, manifest) in
51-
let identity = PackageIdentity(manifest.url)
51+
let identity = PackageIdentity(url: manifest.url)
5252
return PackageReference(identity: identity, path: path.pathString, kind: .root)
5353
}
5454
self.manifests = manifests

‎Sources/PackageGraph/PackageModel+Extensions.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ extension PackageDependencyDescription {
2323
// We should instead use the declared URL of a package dependency
2424
// as its identity, as it will be needed for supporting package
2525
// registries.
26-
let identity = PackageIdentity(effectiveURL)
26+
let identity = PackageIdentity(url: effectiveURL)
2727

2828
return PackageReference(
2929
identity: identity,

‎Sources/PackageGraph/PinsStore.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable {
135135
public init(json: JSON) throws {
136136
let name: String? = json.get("package")
137137
let url: String = try json.get("repositoryURL")
138-
let identity = PackageIdentity(url)
138+
let identity = PackageIdentity(url: url)
139139
let ref = PackageReference(identity: identity, path: url)
140140
self.packageRef = name.flatMap(ref.with(newName:)) ?? ref
141141
self.state = try json.get("state")

‎Sources/PackageGraph/Pubgrub/PubgrubDependencyResolver.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ public final class PubgrubDependencyResolver {
396396
pinsMap: PinsStore.PinsMap = [:]
397397
) throws -> [(container: PackageReference, binding: BoundVersion, products: ProductFilter)] {
398398
let root = DependencyResolutionNode.root(package: PackageReference(
399-
identity: PackageIdentity("<synthesized-root>"),
399+
identity: PackageIdentity(url: "<synthesized-root>"),
400400
path: "<synthesized-root-path>",
401401
name: nil,
402402
kind: .root
@@ -979,7 +979,7 @@ private final class DiagnosticReportBuilder {
979979

980980
// FIXME: This is duplicated and wrong.
981981
private func isFailure(_ incompatibility: Incompatibility) -> Bool {
982-
return incompatibility.terms.count == 1 && incompatibility.terms.first?.node.package.identity == PackageIdentity("<synthesized-root>")
982+
return incompatibility.terms.count == 1 && incompatibility.terms.first?.node.package.identity == PackageIdentity(url: "<synthesized-root>")
983983
}
984984

985985
private func description(for term: Term, normalizeRange: Bool = false) -> String {

‎Sources/PackageLoading/ManifestLoader.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
268268
}
269269

270270
// Get the JSON string for the manifest.
271-
let identity = PackageIdentity(baseURL)
271+
let identity = PackageIdentity(url: baseURL)
272272
let jsonString = try loadJSONString(
273273
path: inputPath,
274274
toolsVersion: toolsVersion,
@@ -388,7 +388,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
388388
diagnostics: DiagnosticsEngine?
389389
) throws {
390390
let dependenciesByIdentity = Dictionary(grouping: manifest.dependencies, by: { dependency in
391-
PackageIdentity(dependency.url)
391+
PackageIdentity(url: dependency.url)
392392
})
393393

394394
let duplicateDependencyIdentities = dependenciesByIdentity

‎Sources/PackageModel/PackageIdentity.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,21 @@ public struct PackageIdentity: Hashable, CustomStringConvertible {
3434
/// A textual representation of this instance.
3535
public let description: String
3636

37+
/// Creates a package identity from a string.
38+
/// - Parameter string: A string used to identify a package.
39+
init(_ string: String) {
40+
self.description = Self.provider.init(string).description
41+
}
3742

3843
/// Creates a package identity from a URL.
3944
/// - Parameter url: The package's URL.
40-
public init(_ url: String) {
41-
self.description = Self.provider.init(url).description
45+
public init(url: String) { // TODO: Migrate to Foundation.URL
46+
self.init(url)
4247
}
4348

4449
/// Creates a package identity from a file path.
4550
/// - Parameter path: An absolute path to the package.
46-
public init(_ path: AbsolutePath) {
51+
public init(path: AbsolutePath) {
4752
self.init(path.pathString)
4853
}
4954
}

‎Sources/SPMTestSupport/MockDependencyGraph.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public extension MockDependencyGraph {
4949
self.result = Dictionary(uniqueKeysWithValues: result.map { value in
5050
let (container, version) = value
5151
guard case .string(let str) = version else { fatalError() }
52-
let package = PackageReference(identity: PackageIdentity(container.lowercased()), path: "/\(container)")
52+
let package = PackageReference(identity: PackageIdentity(url: container.lowercased()), path: "/\(container)")
5353
return (package, Version(string: str)!)
5454
})
5555
self.name = name
@@ -91,7 +91,7 @@ private extension MockPackageContainer.Constraint {
9191
guard case .string(let identifier)? = dict["identifier"] else { fatalError() }
9292
guard let requirement = dict["requirement"] else { fatalError() }
9393
let products: ProductFilter = try! JSON(dict).get("products")
94-
let id = PackageReference(identity: PackageIdentity(identifier), path: "", kind: .remote)
94+
let id = PackageReference(identity: PackageIdentity(url: identifier), path: "", kind: .remote)
9595
self.init(container: id, versionRequirement: VersionSetSpecifier(requirement), products: products)
9696
}
9797
}

‎Sources/SPMTestSupport/MockPackageContainer.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ public class MockPackageContainer: PackageContainer {
8181
var dependencies: [String: [Dependency]] = [:]
8282
for (version, deps) in dependenciesByVersion {
8383
dependencies[version.description] = deps.map {
84-
let ref = PackageReference(identity: PackageIdentity($0.container), path: "/\($0.container)")
84+
let ref = PackageReference(identity: PackageIdentity(url: $0.container), path: "/\($0.container)")
8585
return (ref, .versionSet($0.versionRequirement))
8686
}
8787
}
88-
let ref = PackageReference(identity: PackageIdentity(name), path: "/\(name)")
88+
let ref = PackageReference(identity: PackageIdentity(url: name), path: "/\(name)")
8989
self.init(name: ref, dependencies: dependencies)
9090
}
9191

@@ -124,12 +124,12 @@ public struct MockPackageContainerProvider: PackageContainerProvider {
124124

125125
public extension MockPackageContainer.Constraint {
126126
init(container identifier: String, requirement: PackageRequirement, products: ProductFilter) {
127-
let ref = PackageReference(identity: PackageIdentity(identifier), path: "")
127+
let ref = PackageReference(identity: PackageIdentity(url: identifier), path: "")
128128
self.init(container: ref, requirement: requirement, products: products)
129129
}
130130

131131
init(container identifier: String, versionRequirement: VersionSetSpecifier, products: ProductFilter) {
132-
let ref = PackageReference(identity: PackageIdentity(identifier), path: "")
132+
let ref = PackageReference(identity: PackageIdentity(url: identifier), path: "")
133133
self.init(container: ref, versionRequirement: versionRequirement, products: products)
134134
}
135135
}

‎Sources/Workspace/ResolverPrecomputationProvider.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ private struct LocalPackageContainer: PackageContainer {
105105
if let identifier = dependency?.packageRef {
106106
return identifier
107107
} else {
108-
let identity = PackageIdentity(manifest.url)
108+
let identity = PackageIdentity(url: manifest.url)
109109
return PackageReference(
110110
identity: identity,
111111
path: manifest.path.pathString,

‎Sources/Workspace/Workspace.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,19 +1036,19 @@ extension Workspace {
10361036

10371037
func computePackageURLs() -> (required: Set<PackageReference>, missing: Set<PackageReference>) {
10381038
let manifestsMap: [PackageIdentity: Manifest] = Dictionary(uniqueKeysWithValues:
1039-
root.manifests.map({ (PackageIdentity($0.url), $0) }) +
1040-
dependencies.map({ (PackageIdentity($0.manifest.url), $0.manifest) }))
1039+
root.manifests.map({ (PackageIdentity(url: $0.url), $0) }) +
1040+
dependencies.map({ (PackageIdentity(url: $0.manifest.url), $0.manifest) }))
10411041

10421042
var inputIdentities: Set<PackageReference> = []
10431043
let inputNodes: [GraphLoadingNode] = root.manifests.map({ manifest in
1044-
let identity = PackageIdentity(manifest.url)
1044+
let identity = PackageIdentity(url: manifest.url)
10451045
let package = PackageReference(identity: identity, path: manifest.url, kind: manifest.packageKind)
10461046
inputIdentities.insert(package)
10471047
let node = GraphLoadingNode(manifest: manifest, productFilter: .everything)
10481048
return node
10491049
}) + root.dependencies.compactMap({ dependency in
10501050
let url = workspace.config.mirrors.effectiveURL(forURL: dependency.url)
1051-
let identity = PackageIdentity(url)
1051+
let identity = PackageIdentity(url: url)
10521052
let package = PackageReference(identity: identity, path: url)
10531053
inputIdentities.insert(package)
10541054
guard let manifest = manifestsMap[identity] else { return nil }
@@ -1060,7 +1060,7 @@ extension Workspace {
10601060
_ = transitiveClosure(inputNodes) { node in
10611061
return node.manifest.dependenciesRequired(for: node.productFilter).compactMap({ dependency in
10621062
let url = workspace.config.mirrors.effectiveURL(forURL: dependency.url)
1063-
let identity = PackageIdentity(url)
1063+
let identity = PackageIdentity(url: url)
10641064
let package = PackageReference(identity: identity, path: url)
10651065
requiredIdentities.insert(package)
10661066
guard let manifest = manifestsMap[identity] else { return nil }
@@ -1799,8 +1799,8 @@ extension Workspace {
17991799
for missingURLs in dependencyManifests.computePackageURLs().missing {
18001800
guard let manifest = loadManifest(forURL: missingURLs.path, diagnostics: diagnostics) else { continue }
18011801
if let override = rootManifests[manifest.name] {
1802-
let overrideIdentity = PackageIdentity(override.url)
1803-
let manifestIdentity = PackageIdentity(manifest.url)
1802+
let overrideIdentity = PackageIdentity(url: override.url)
1803+
let manifestIdentity = PackageIdentity(url: manifest.url)
18041804

18051805
diagnostics.emit(error: "unable to override package '\(manifest.name)' because its basename '\(manifestIdentity)' doesn't match directory name '\(overrideIdentity)'")
18061806

‎Tests/CommandsTests/PackageToolTests.swift

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ final class PackageToolTests: XCTestCase {
503503
try execute("resolve", "bar", "--branch", "YOLO")
504504
let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
505505
let state = CheckoutState(revision: yoloRevision, branch: "YOLO")
506-
let identity = PackageIdentity("bar")
506+
let identity = PackageIdentity(path: barPath)
507507
XCTAssertEqual(pinsStore.pinsMap[identity]?.state, state)
508508
}
509509

@@ -512,7 +512,7 @@ final class PackageToolTests: XCTestCase {
512512
try execute("resolve", "bar", "--revision", yoloRevision.identifier)
513513
let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
514514
let state = CheckoutState(revision: yoloRevision)
515-
let identity = PackageIdentity("bar")
515+
let identity = PackageIdentity(path: barPath)
516516
XCTAssertEqual(pinsStore.pinsMap[identity]?.state, state)
517517
}
518518

@@ -554,8 +554,9 @@ final class PackageToolTests: XCTestCase {
554554
let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
555555
XCTAssertEqual(pinsStore.pins.map{$0}.count, 2)
556556
for pkg in ["bar", "baz"] {
557-
let pin = pinsStore.pinsMap[PackageIdentity(pkg)]!
558-
XCTAssertEqual(pin.packageRef.identity, PackageIdentity(pkg))
557+
let path = try SwiftPMProduct.packagePath(for: pkg, packageRoot: fooPath)
558+
let pin = pinsStore.pinsMap[PackageIdentity(path: path)]!
559+
XCTAssertEqual(pin.packageRef.identity, PackageIdentity(path: path))
559560
XCTAssert(pin.packageRef.repository.url.hasSuffix(pkg))
560561
XCTAssertEqual(pin.state.version, "1.2.3")
561562
}
@@ -570,7 +571,7 @@ final class PackageToolTests: XCTestCase {
570571
do {
571572
try execute("resolve", "bar")
572573
let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
573-
let identity = PackageIdentity("bar")
574+
let identity = PackageIdentity(path: barPath)
574575
XCTAssertEqual(pinsStore.pinsMap[identity]?.state.version, "1.2.3")
575576
}
576577

@@ -594,7 +595,7 @@ final class PackageToolTests: XCTestCase {
594595
do {
595596
try execute("resolve", "bar", "--version", "1.2.3")
596597
let pinsStore = try PinsStore(pinsFile: pinsFile, fileSystem: localFileSystem)
597-
let identity = PackageIdentity("bar")
598+
let identity = PackageIdentity(path: barPath)
598599
XCTAssertEqual(pinsStore.pinsMap[identity]?.state.version, "1.2.3")
599600
try checkBar(5)
600601
}

‎Tests/PackageGraphTests/PubgrubTests.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import XCTest
1212

1313
import TSCBasic
1414
import PackageLoading
15-
import PackageModel
15+
@testable import PackageModel
1616
import SourceControl
1717

1818
import PackageGraph
@@ -299,7 +299,8 @@ final class PubgrubTests: XCTestCase {
299299
}
300300

301301
func testUpdatePackageIdentifierAfterResolution() {
302-
let fooRef = PackageReference(identity: PackageIdentity("foo"), path: "https://some.url/FooBar")
302+
let fooURL = "https://example.com/foo"
303+
let fooRef = PackageReference(identity: PackageIdentity(url: fooURL), path: fooURL)
303304
let foo = MockContainer(name: fooRef, dependenciesByVersion: [v1: [:]])
304305
foo.manifestName = "bar"
305306

0 commit comments

Comments
 (0)