Skip to content

Commit 18c097d

Browse files
tomerdtomerd
authored
refactor location abstractions (#3764)
motivation: reduce dependency on string location information, in support of registry based dependencies changes: * add location information to PackageReference::Kind, using AbsolutePath and URL where appropriate * seperate out sourceControl dependencies to local and remote, where remote carries AbsolutePath and rmeote carries URL * add Location to RepositorySpecifier with local and remote, where remote carries AbsolutePath and rmeote carries URL * expand IdentityResolver to resolve identity from PackageReference::Kind, AbsolutePath and URL * adjust call sites (tons!) * adjust tests and test utilities (tons!) rdar://82954076
1 parent 1be68e8 commit 18c097d

File tree

75 files changed

+2030
-1623
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

75 files changed

+2030
-1623
lines changed

‎Sources/Commands/APIDigester.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ struct APIDigesterBaselineDumper {
8787

8888
// Clone the current package in a sandbox and checkout the baseline revision.
8989
let repositoryProvider = GitRepositoryProvider()
90-
let specifier = RepositorySpecifier(url: baselinePackageRoot.pathString)
90+
let specifier = RepositorySpecifier(path: baselinePackageRoot)
9191
let workingCopy = try repositoryProvider.createWorkingCopy(
9292
repository: specifier,
9393
sourcePath: packageRoot,

‎Sources/Commands/Describe.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@ fileprivate struct DescribedPackage: Encodable {
109109
case .fileSystem(let settings):
110110
self = .fileSystem(identity: settings.identity, path: settings.path)
111111
case .sourceControl(let settings):
112-
self = .sourceControl(identity: settings.identity, location: settings.location, requirement: settings.requirement)
112+
switch settings.location {
113+
case .local(let path):
114+
self = .sourceControl(identity: settings.identity, location: path.pathString, requirement: settings.requirement)
115+
case .remote(let url):
116+
self = .sourceControl(identity: settings.identity, location: url.absoluteString, requirement: settings.requirement)
117+
}
113118
case .registry(let settings):
114119
self = .registry(identity: settings.identity, requirement: settings.requirement)
115120
}

‎Sources/Commands/SwiftPackageCollectionsTool.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,10 +264,10 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
264264

265265
mutating func run() throws {
266266
try with { collections in
267-
let identity = PackageIdentity(url: packageURL)
267+
let identity = PackageIdentity(urlString: self.packageURL)
268268

269269
do { // assume URL is for a package in an imported collection
270-
let result = try tsc_await { collections.getPackageMetadata(identity: identity, location: packageURL, callback: $0) }
270+
let result = try tsc_await { collections.getPackageMetadata(identity: identity, location: self.packageURL, callback: $0) }
271271

272272
if let versionString = version {
273273
guard let version = TSCUtility.Version(versionString), let result = result.package.versions.first(where: { $0.version == version }), let printedResult = printVersion(result) else {

‎Sources/PackageCollections/API.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ public protocol PackageCollectionsProtocol {
111111
/// - Parameters:
112112
/// - reference: The package reference
113113
/// - callback: The closure to invoke when result becomes available
114+
// deprecated 9/21
114115
@available(*, deprecated, message: "user getPackageMetadata(identity:) instead")
115116
func getPackageMetadata(
116117
_ reference: PackageReference,
@@ -127,6 +128,7 @@ public protocol PackageCollectionsProtocol {
127128
/// - collections: Optional. If specified, only look for package in these collections. Data from the most recently
128129
/// processed collection will be used.
129130
/// - callback: The closure to invoke when result becomes available
131+
// deprecated 9/21
130132
@available(*, deprecated, message: "user getPackageMetadata(identity:) instead")
131133
func getPackageMetadata(
132134
_ reference: PackageReference,

‎Sources/PackageCollections/Model/Package.swift

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,22 @@ extension PackageCollectionsModel {
8383
/// The package's programming languages
8484
public let languages: Set<String>?
8585

86+
// deprecated 9/21
8687
@available(*, deprecated, message: "use identity and location instead")
8788
public var reference: PackageReference {
88-
return .init(identity: self.identity, kind: .remote, location: self.location, name: nil)
89+
guard let url = URL(string: self.location) else {
90+
fatalError("invalid url \(self.location)")
91+
}
92+
return .init(identity: self.identity, kind: .remoteSourceControl(url), name: nil)
8993
}
9094

95+
// deprecated 9/21
9196
@available(*, deprecated, message: "use identity and location instead")
9297
public var repository: RepositorySpecifier {
93-
return .init(url: self.location)
98+
guard let url = URL(string: self.location) else {
99+
fatalError("invalid url \(self.location)")
100+
}
101+
return .init(url: url)
94102
}
95103

96104
/// Initializes a `Package`

‎Sources/PackageCollections/Model/TargetListResult.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ extension PackageCollectionsModel.TargetListResult {
5050

5151
@available(*, deprecated, message: "use identity and location instead")
5252
public var repository: RepositorySpecifier {
53-
return .init(url: self.location)
53+
guard let url = URL(string: self.location) else {
54+
fatalError("invalid url \(self.location)")
55+
}
56+
return .init(url: url)
5457
}
5558
}
5659
}

‎Sources/PackageCollections/PackageCollections+Validation.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ extension PackageCollectionModel.V1 {
8080

8181
// TODO: validate package url?
8282
private func validate(package: Collection.Package, messages: inout [ValidationMessage]) {
83-
let packageID = PackageIdentity(url: package.url.absoluteString).description
83+
let packageID = PackageIdentity(url: package.url).description
8484

8585
guard !package.versions.isEmpty else {
8686
messages.append(.error("Package \(packageID) does not have any versions.", property: "package.versions"))

‎Sources/PackageCollections/PackageCollections.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,14 @@ public struct PackageCollections: PackageCollectionsProtocol {
341341

342342
// MARK: - Package Metadata
343343

344+
// deprecated 9/21
344345
@available(*, deprecated, message: "user identity based API instead")
345346
public func getPackageMetadata(_ reference: PackageReference,
346347
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void) {
347348
self.getPackageMetadata(reference, collections: nil, callback: callback)
348349
}
349350

351+
// deprecated 9/21
350352
@available(*, deprecated, message: "user identity based API instead")
351353
public func getPackageMetadata(_ reference: PackageReference,
352354
collections: Set<PackageCollectionsModel.CollectionIdentifier>?,

‎Sources/PackageCollections/Providers/GitHubPackageMetadataProvider.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
175175
}
176176
}
177177

178+
// FIXME: use Foundation.URL instead of string
178179
internal static func apiURL(_ url: String) -> Foundation.URL? {
179180
do {
180181
let regex = try NSRegularExpression(pattern: #"([^/@]+)[:/]([^:/]+)/([^/.]+)(\.git)?$"#, options: .caseInsensitive)

‎Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
255255
serializationOkay = false
256256
}
257257

258-
return .init(identity: .init(url: package.url.absoluteString),
258+
return .init(identity: .init(url: package.url),
259259
location: package.url.absoluteString,
260260
summary: package.summary,
261261
keywords: package.keywords,

‎Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
308308
while let row = try statement.step() {
309309
if let collectionData = Data(base64Encoded: row.string(at: 0)),
310310
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
311-
matches.append((collection: collection, package: PackageIdentity(url: row.string(at: 1))))
311+
matches.append((collection: collection, package: PackageIdentity(urlString: row.string(at: 1))))
312312
matchingCollections.insert(collection)
313313
}
314314
}
@@ -425,7 +425,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
425425
while let row = try statement.step() {
426426
if let collectionData = Data(base64Encoded: row.string(at: 0)),
427427
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
428-
matches.append((collection: collection, package: PackageIdentity(url: row.string(at: 1))))
428+
matches.append((collection: collection, package: PackageIdentity(urlString: row.string(at: 1))))
429429
matchingCollections.insert(collection)
430430
}
431431
}
@@ -575,7 +575,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
575575
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
576576
matches.append((
577577
collection: collection,
578-
package: PackageIdentity(url: row.string(at: 1)),
578+
package: PackageIdentity(urlString: row.string(at: 1)),
579579
targetName: row.string(at: 2)
580580
))
581581
matchingCollections.insert(collection)
@@ -859,7 +859,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
859859

860860
if let collectionData = Data(base64Encoded: row.string(at: 0)),
861861
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
862-
let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(url: row.string(at: 1)))
862+
let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(urlString: row.string(at: 1)))
863863
self.targetTrie.insert(word: targetName.lowercased(), foundIn: collectionPackage)
864864
}
865865
}

‎Sources/PackageGraph/DependencyResolver.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,15 @@ public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {
5555
}
5656

5757
public func willResolve(term: Term) {
58-
self.log("resolving: \(term.node.package.location)")
58+
self.log("resolving: \(term.node.package.identity)")
5959
}
6060

6161
public func didResolve(term: Term, version: Version, duration: DispatchTimeInterval) {
62-
self.log("resolved: \(term.node.package.location) @ \(version)")
62+
self.log("resolved: \(term.node.package.identity) @ \(version)")
6363
}
6464

6565
public func derived(term: Term) {
66-
self.log("derived: \(term.node.package.location)")
66+
self.log("derived: \(term.node.package.identity)")
6767
}
6868

6969
public func conflict(conflict: Incompatibility) {

‎Sources/PackageGraph/LocalPackageContainer.swift

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import TSCUtility
2121
///
2222
/// This class represent packages that are referenced locally in the file system.
2323
/// There is no need to perform any git operations on such packages and they
24-
/// should be used as-is. Infact, they might not even have a git repository.
24+
/// should be used as-is. In fact, they might not even have a git repository.
2525
/// Examples: Root packages, local dependencies, edited packages.
2626
public final class LocalPackageContainer: PackageContainer {
2727
public let package: PackageReference
@@ -30,27 +30,35 @@ public final class LocalPackageContainer: PackageContainer {
3030
private let toolsVersionLoader: ToolsVersionLoaderProtocol
3131
private let currentToolsVersion: ToolsVersion
3232

33-
/// The file system that shoud be used to load this package.
33+
/// The file system that should be used to load this package.
3434
private let fileSystem: FileSystem
3535

3636
/// cached version of the manifest
3737
private let manifest = ThreadSafeBox<Manifest>()
3838

3939
private func loadManifest() throws -> Manifest {
4040
try manifest.memoize() {
41+
let path: AbsolutePath
42+
switch self.package.kind {
43+
case .root(let _path), .fileSystem(let _path):
44+
path = _path
45+
default:
46+
throw InternalError("invalid package type \(package.kind)")
47+
}
48+
4149
// Load the tools version.
42-
let toolsVersion = try self.toolsVersionLoader.load(at: AbsolutePath(self.package.location), fileSystem: self.fileSystem)
50+
let toolsVersion = try self.toolsVersionLoader.load(at: path, fileSystem: self.fileSystem)
4351

4452
// Validate the tools version.
4553
try toolsVersion.validateToolsVersion(self.currentToolsVersion, packageIdentity: self.package.identity)
4654

4755
// Load the manifest.
4856
// FIXME: this should not block
4957
return try temp_await {
50-
manifestLoader.load(at: AbsolutePath(self.package.location),
58+
manifestLoader.load(at: path,
5159
packageIdentity: self.package.identity,
5260
packageKind: self.package.kind,
53-
packageLocation: self.package.location,
61+
packageLocation: path.pathString,
5462
version: nil,
5563
revision: nil,
5664
toolsVersion: toolsVersion,
@@ -80,8 +88,14 @@ public final class LocalPackageContainer: PackageContainer {
8088
toolsVersionLoader: ToolsVersionLoaderProtocol,
8189
currentToolsVersion: ToolsVersion,
8290
fileSystem: FileSystem = localFileSystem
83-
) {
84-
assert(URL.scheme(package.location) == nil, "unexpected scheme \(URL.scheme(package.location)!) in \(package.location)")
91+
) throws {
92+
//assert(URL.scheme(package.location) == nil, "unexpected scheme \(URL.scheme(package.location)!) in \(package.location)")
93+
switch package.kind {
94+
case .root, .fileSystem:
95+
break
96+
default:
97+
throw InternalError("invalid package type \(package.kind)")
98+
}
8599
self.package = package
86100
self.identityResolver = identityResolver
87101
self.manifestLoader = manifestLoader
@@ -117,6 +131,6 @@ public final class LocalPackageContainer: PackageContainer {
117131

118132
extension LocalPackageContainer: CustomStringConvertible {
119133
public var description: String {
120-
return "LocalPackageContainer(\(package.location))"
134+
return "LocalPackageContainer(\(self.package.identity))"
121135
}
122136
}

‎Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ extension PackageGraph {
3636

3737
// Create a map of the manifests, keyed by their identity.
3838
let rootManifestsMap = root.manifests
39-
let externalManifestsMap = externalManifests.map{ (identityResolver.resolveIdentity(for: $0.packageLocation), $0) }
39+
let externalManifestsMap = try externalManifests.map{ (try identityResolver.resolveIdentity(for: $0.packageKind), $0) }
4040
let manifestMap = rootManifestsMap.merging(externalManifestsMap, uniquingKeysWith: { lhs, rhs in
4141
return lhs
4242
})
@@ -116,7 +116,7 @@ extension PackageGraph {
116116
binaryArtifacts: binaryArtifacts,
117117
xcTestMinimumDeploymentTargets: xcTestMinimumDeploymentTargets,
118118
shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
119-
createREPLProduct: manifest.packageKind == .root ? createREPLProduct : false,
119+
createREPLProduct: manifest.packageKind.isRoot ? createREPLProduct : false,
120120
fileSystem: fileSystem
121121
)
122122
let package = try builder.construct()
@@ -125,7 +125,7 @@ extension PackageGraph {
125125
// Throw if any of the non-root package is empty.
126126
if package.targets.isEmpty // System packages have targets in the package but not the manifest.
127127
&& package.manifest.targets.isEmpty // An unneeded dependency will not have loaded anything from the manifest.
128-
&& manifest.packageKind != .root {
128+
&& !manifest.packageKind.isRoot {
129129
throw PackageGraphError.noModules(package)
130130
}
131131
}
@@ -204,7 +204,7 @@ private func createResolvedPackages(
204204
guard let package = manifestToPackage[node.manifest] else {
205205
return nil
206206
}
207-
let isAllowedToVendUnsafeProducts = unsafeAllowedPackages.contains{ $0.location == package.manifest.packageLocation }
207+
let isAllowedToVendUnsafeProducts = unsafeAllowedPackages.contains{ $0.identity == package.identity }
208208
let allowedToOverride = rootManifestSet.contains(node.manifest)
209209
return ResolvedPackageBuilder(
210210
package,
@@ -240,7 +240,12 @@ private func createResolvedPackages(
240240
case .fileSystem(let settings):
241241
dependencyLocation = settings.path.pathString
242242
case .sourceControl(let settings):
243-
dependencyLocation = settings.location
243+
switch settings.location {
244+
case .local(let path):
245+
dependencyLocation = path.pathString
246+
case .remote(let url):
247+
dependencyLocation = url.absoluteString
248+
}
244249
case .registry:
245250
// FIXME
246251
fatalError("registry based dependencies not implemented yet")

‎Sources/PackageGraph/PackageModel+Extensions.swift

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,21 @@ extension PackageDependency {
1616
public func createPackageRef() -> PackageReference {
1717
// TODO (next steps): move the location into PackageKind to preserve path vs. location
1818
let packageKind: PackageReference.Kind
19-
let location: String
2019
switch self {
2120
case .fileSystem(let settings):
22-
packageKind = .local
23-
location = settings.path.pathString
21+
packageKind = .fileSystem(settings.path)
2422
case .sourceControl(let settings):
25-
packageKind = .remote
26-
location = settings.location
23+
switch settings.location {
24+
case .local(let path):
25+
packageKind = .localSourceControl(path)
26+
case .remote(let url):
27+
packageKind = .remoteSourceControl(url)
28+
}
2729
case .registry:
2830
// FIXME
2931
fatalError("registry based dependencies not implemented yet")
3032
}
31-
return PackageReference(
32-
identity: self.identity,
33-
kind: packageKind,
34-
location: location
35-
)
33+
return PackageReference(identity: self.identity, kind: packageKind)
3634
}
3735
}
3836

0 commit comments

Comments
 (0)