refactor location abstractions

Sources/Commands/APIDigester.swift

@@ -87,7 +87,7 @@ struct APIDigesterBaselineDumper {
 
         // Clone the current package in a sandbox and checkout the baseline revision.
         let repositoryProvider = GitRepositoryProvider()
-        let specifier = RepositorySpecifier(url: baselinePackageRoot.pathString)
+        let specifier = RepositorySpecifier(path: baselinePackageRoot)
         let workingCopy = try repositoryProvider.createWorkingCopy(
             repository: specifier,
             sourcePath: packageRoot,

Sources/Commands/Describe.swift

@@ -109,7 +109,12 @@ fileprivate struct DescribedPackage: Encodable {
             case .fileSystem(let settings):
                 self = .fileSystem(identity: settings.identity, path: settings.path)
             case .sourceControl(let settings):
-                self = .sourceControl(identity: settings.identity, location: settings.location, requirement: settings.requirement)
+                switch settings.location {
+                case .local(let path):
+                    self = .sourceControl(identity: settings.identity, location: path.pathString, requirement: settings.requirement)
+                case .remote(let url):
+                    self = .sourceControl(identity: settings.identity, location: url.absoluteString, requirement: settings.requirement)
+                }
             case .registry(let settings):
                 self = .registry(identity: settings.identity, requirement: settings.requirement)
             }

Sources/Commands/SwiftPackageCollectionsTool.swift

@@ -264,10 +264,10 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
 
         mutating func run() throws {
             try with { collections in
-                let identity = PackageIdentity(url: packageURL)
+                let identity = PackageIdentity(urlString: self.packageURL)
 
                 do { // assume URL is for a package in an imported collection
-                    let result = try tsc_await { collections.getPackageMetadata(identity: identity, location: packageURL, callback: $0) }
+                    let result = try tsc_await { collections.getPackageMetadata(identity: identity, location: self.packageURL, callback: $0) }
 
                     if let versionString = version {
                         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

@@ -111,6 +111,7 @@ public protocol PackageCollectionsProtocol {
     /// - Parameters:
     ///   - reference: The package reference
     ///   - callback: The closure to invoke when result becomes available
+    // deprecated 9/21
     @available(*, deprecated, message: "user getPackageMetadata(identity:) instead")
     func getPackageMetadata(
         _ reference: PackageReference,
@@ -127,6 +128,7 @@ public protocol PackageCollectionsProtocol {
     ///   - collections: Optional. If specified, only look for package in these collections. Data from the most recently
     ///                  processed collection will be used.
     ///   - callback: The closure to invoke when result becomes available
+    // deprecated 9/21
     @available(*, deprecated, message: "user getPackageMetadata(identity:) instead")
     func getPackageMetadata(
         _ reference: PackageReference,

Sources/PackageCollections/Model/Package.swift

@@ -83,14 +83,22 @@ extension PackageCollectionsModel {
         /// The package's programming languages
         public let languages: Set<String>?
 
+        // deprecated 9/21
         @available(*, deprecated, message: "use identity and location instead")
         public var reference: PackageReference {
-            return .init(identity: self.identity, kind: .remote, location: self.location, name: nil)
+            guard let url = URL(string: self.location) else {
+                fatalError("invalid url \(self.location)")
+            }
+            return .init(identity: self.identity, kind: .remoteSourceControl(url), name: nil)
         }
 
+        // deprecated 9/21
         @available(*, deprecated, message: "use identity and location instead")
         public var repository: RepositorySpecifier {
-            return .init(url: self.location)
+            guard let url = URL(string: self.location) else {
+                fatalError("invalid url \(self.location)")
+            }
+            return .init(url: url)
         }
 
         /// Initializes a `Package`

Sources/PackageCollections/Model/TargetListResult.swift

@@ -50,7 +50,10 @@ extension PackageCollectionsModel.TargetListResult {
 
         @available(*, deprecated, message: "use identity and location instead")
         public var repository: RepositorySpecifier {
-            return .init(url: self.location)
+            guard let url = URL(string: self.location) else {
+                fatalError("invalid url \(self.location)")
+            }
+            return .init(url: url)
         }
     }
 }

Sources/PackageCollections/PackageCollections+Validation.swift

@@ -80,7 +80,7 @@ extension PackageCollectionModel.V1 {
 
         // TODO: validate package url?
         private func validate(package: Collection.Package, messages: inout [ValidationMessage]) {
-            let packageID = PackageIdentity(url: package.url.absoluteString).description
+            let packageID = PackageIdentity(url: package.url).description
 
             guard !package.versions.isEmpty else {
                 messages.append(.error("Package \(packageID) does not have any versions.", property: "package.versions"))

Sources/PackageCollections/PackageCollections.swift

@@ -341,12 +341,14 @@ public struct PackageCollections: PackageCollectionsProtocol {
 
     // MARK: - Package Metadata
 
+    // deprecated 9/21
     @available(*, deprecated, message: "user identity based API instead")
     public func getPackageMetadata(_ reference: PackageReference,
                                    callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void) {
         self.getPackageMetadata(reference, collections: nil, callback: callback)
     }
 
+    // deprecated 9/21
     @available(*, deprecated, message: "user identity based API instead")
     public func getPackageMetadata(_ reference: PackageReference,
                                    collections: Set<PackageCollectionsModel.CollectionIdentifier>?,

Sources/PackageCollections/Providers/GitHubPackageMetadataProvider.swift

@@ -175,6 +175,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
         }
     }
 
+    // FIXME: use Foundation.URL instead of string
     internal static func apiURL(_ url: String) -> Foundation.URL? {
         do {
             let regex = try NSRegularExpression(pattern: #"([^/@]+)[:/]([^:/]+)/([^/.]+)(\.git)?$"#, options: .caseInsensitive)

Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

@@ -255,7 +255,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                 serializationOkay = false
             }
 
-            return .init(identity: .init(url: package.url.absoluteString),
+            return .init(identity: .init(url: package.url),
                          location: package.url.absoluteString,
                          summary: package.summary,
                          keywords: package.keywords,

Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

@@ -308,7 +308,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                     while let row = try statement.step() {
                         if let collectionData = Data(base64Encoded: row.string(at: 0)),
                             let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
-                            matches.append((collection: collection, package: PackageIdentity(url: row.string(at: 1))))
+                            matches.append((collection: collection, package: PackageIdentity(urlString: row.string(at: 1))))
                             matchingCollections.insert(collection)
                         }
                     }
@@ -425,7 +425,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                     while let row = try statement.step() {
                         if let collectionData = Data(base64Encoded: row.string(at: 0)),
                             let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
-                            matches.append((collection: collection, package: PackageIdentity(url: row.string(at: 1))))
+                            matches.append((collection: collection, package: PackageIdentity(urlString: row.string(at: 1))))
                             matchingCollections.insert(collection)
                         }
                     }
@@ -575,7 +575,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                                 let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
                                 matches.append((
                                     collection: collection,
-                                    package: PackageIdentity(url: row.string(at: 1)),
+                                    package: PackageIdentity(urlString: row.string(at: 1)),
                                     targetName: row.string(at: 2)
                                 ))
                                 matchingCollections.insert(collection)
@@ -859,7 +859,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
 
                             if let collectionData = Data(base64Encoded: row.string(at: 0)),
                                 let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
-                                let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(url: row.string(at: 1)))
+                                let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(urlString: row.string(at: 1)))
                                 self.targetTrie.insert(word: targetName.lowercased(), foundIn: collectionPackage)
                             }
                         }

Sources/PackageGraph/DependencyResolver.swift

@@ -55,15 +55,15 @@ public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {
     }
 
     public func willResolve(term: Term) {
-        self.log("resolving: \(term.node.package.location)")
+        self.log("resolving: \(term.node.package.identity)")
     }
 
     public func didResolve(term: Term, version: Version, duration: DispatchTimeInterval) {
-        self.log("resolved: \(term.node.package.location) @ \(version)")
+        self.log("resolved: \(term.node.package.identity) @ \(version)")
     }
 
     public func derived(term: Term) {
-        self.log("derived: \(term.node.package.location)")
+        self.log("derived: \(term.node.package.identity)")
     }
 
     public func conflict(conflict: Incompatibility) {

Sources/PackageGraph/LocalPackageContainer.swift

@@ -21,7 +21,7 @@ import TSCUtility
 ///
 /// This class represent packages that are referenced locally in the file system.
 /// There is no need to perform any git operations on such packages and they
-/// should be used as-is. Infact, they might not even have a git repository.
+/// should be used as-is. In fact, they might not even have a git repository.
 /// Examples: Root packages, local dependencies, edited packages.
 public final class LocalPackageContainer: PackageContainer {
     public let package: PackageReference
@@ -30,27 +30,35 @@ public final class LocalPackageContainer: PackageContainer {
     private let toolsVersionLoader: ToolsVersionLoaderProtocol
     private let currentToolsVersion: ToolsVersion
 
-    /// The file system that shoud be used to load this package.
+    /// The file system that should be used to load this package.
     private let fileSystem: FileSystem
 
     /// cached version of the manifest
     private let manifest = ThreadSafeBox<Manifest>()
 
     private func loadManifest() throws -> Manifest {
         try manifest.memoize() {
+            let path: AbsolutePath
+            switch self.package.kind {
+            case .root(let _path), .fileSystem(let _path):
+                path = _path
+            default:
+                throw InternalError("invalid package type \(package.kind)")
+            }
+
             // Load the tools version.
-            let toolsVersion = try self.toolsVersionLoader.load(at: AbsolutePath(self.package.location), fileSystem: self.fileSystem)
+            let toolsVersion = try self.toolsVersionLoader.load(at: path, fileSystem: self.fileSystem)
 
             // Validate the tools version.
             try toolsVersion.validateToolsVersion(self.currentToolsVersion, packageIdentity: self.package.identity)
 
             // Load the manifest.
             // FIXME: this should not block
             return try temp_await {
-                manifestLoader.load(at: AbsolutePath(self.package.location),
+                manifestLoader.load(at: path,
                                     packageIdentity: self.package.identity,
                                     packageKind: self.package.kind,
-                                    packageLocation: self.package.location,
+                                    packageLocation: path.pathString,
                                     version: nil,
                                     revision: nil,
                                     toolsVersion: toolsVersion,
@@ -80,8 +88,14 @@ public final class LocalPackageContainer: PackageContainer {
         toolsVersionLoader: ToolsVersionLoaderProtocol,
         currentToolsVersion: ToolsVersion,
         fileSystem: FileSystem = localFileSystem
-    ) {
-        assert(URL.scheme(package.location) == nil, "unexpected scheme \(URL.scheme(package.location)!) in \(package.location)")
+    ) throws {
+        //assert(URL.scheme(package.location) == nil, "unexpected scheme \(URL.scheme(package.location)!) in \(package.location)")
+        switch package.kind {
+        case .root, .fileSystem:
+            break
+        default:
+            throw InternalError("invalid package type \(package.kind)")
+        }
         self.package = package
         self.identityResolver = identityResolver
         self.manifestLoader = manifestLoader
@@ -117,6 +131,6 @@ public final class LocalPackageContainer: PackageContainer {
 
 extension LocalPackageContainer: CustomStringConvertible  {
     public var description: String {
-        return "LocalPackageContainer(\(package.location))"
+        return "LocalPackageContainer(\(self.package.identity))"
     }
 }

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -36,7 +36,7 @@ extension PackageGraph {
 
         // Create a map of the manifests, keyed by their identity.
         let rootManifestsMap = root.manifests
-        let externalManifestsMap = externalManifests.map{ (identityResolver.resolveIdentity(for: $0.packageLocation), $0) }
+        let externalManifestsMap = try externalManifests.map{ (try identityResolver.resolveIdentity(for: $0.packageKind), $0) }
         let manifestMap = rootManifestsMap.merging(externalManifestsMap, uniquingKeysWith: { lhs, rhs in
             return lhs
         })
@@ -116,7 +116,7 @@ extension PackageGraph {
                     binaryArtifacts: binaryArtifacts,
                     xcTestMinimumDeploymentTargets: xcTestMinimumDeploymentTargets,
                     shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
-                    createREPLProduct: manifest.packageKind == .root ? createREPLProduct : false,
+                    createREPLProduct: manifest.packageKind.isRoot ? createREPLProduct : false,
                     fileSystem: fileSystem
                 )
                 let package = try builder.construct()
@@ -125,7 +125,7 @@ extension PackageGraph {
                 // Throw if any of the non-root package is empty.
                 if package.targets.isEmpty // System packages have targets in the package but not the manifest.
                     && package.manifest.targets.isEmpty // An unneeded dependency will not have loaded anything from the manifest.
-                    && manifest.packageKind != .root {
+                    && !manifest.packageKind.isRoot {
                     throw PackageGraphError.noModules(package)
                 }
             }
@@ -204,7 +204,7 @@ private func createResolvedPackages(
         guard let package = manifestToPackage[node.manifest] else {
             return nil
         }
-        let isAllowedToVendUnsafeProducts = unsafeAllowedPackages.contains{ $0.location == package.manifest.packageLocation }
+        let isAllowedToVendUnsafeProducts = unsafeAllowedPackages.contains{ $0.identity == package.identity }
         let allowedToOverride = rootManifestSet.contains(node.manifest)
         return ResolvedPackageBuilder(
             package,
@@ -240,7 +240,12 @@ private func createResolvedPackages(
             case .fileSystem(let settings):
                 dependencyLocation = settings.path.pathString
             case .sourceControl(let settings):
-                dependencyLocation = settings.location
+                switch settings.location {
+                case .local(let path):
+                    dependencyLocation = path.pathString
+                case .remote(let url):
+                    dependencyLocation = url.absoluteString
+                }
             case .registry:
                 // FIXME
                 fatalError("registry based dependencies not implemented yet")

Sources/PackageGraph/PackageModel+Extensions.swift

@@ -16,23 +16,21 @@ extension PackageDependency {
     public func createPackageRef() -> PackageReference {
         // TODO (next steps): move the location into PackageKind to preserve path vs. location
         let packageKind: PackageReference.Kind
-        let location: String
         switch self {
         case .fileSystem(let settings):
-            packageKind = .local
-            location = settings.path.pathString
+            packageKind = .fileSystem(settings.path)
         case .sourceControl(let settings):
-            packageKind = .remote
-            location = settings.location
+            switch settings.location {
+            case .local(let path):
+                packageKind = .localSourceControl(path)
+            case .remote(let url):
+                packageKind = .remoteSourceControl(url)
+            }
         case .registry:
             // FIXME
             fatalError("registry based dependencies not implemented yet")
         }
-        return PackageReference(
-            identity: self.identity,
-            kind: packageKind,
-            location: location
-        )
+        return PackageReference(identity: self.identity, kind: packageKind)
     }
 }