Revert "Revert "identity improvements (#3323)" (#3342)"

This reverts commit ec24ca3.
swiftlang · Mar 10, 2021 · 73d3fe9 · 73d3fe9
1 parent ec24ca3
commit 73d3fe9
Show file tree

Hide file tree

Showing 41 changed files with 662 additions and 400 deletions.
diff --git a/Examples/package-info/Sources/package-info/main.swift b/Examples/package-info/Sources/package-info/main.swift
@@ -32,9 +32,9 @@ let packagePath = localFileSystem.currentWorkingDirectory!
 // Each takes longer to load than the level above it, but provides more detail.
 let diagnostics = DiagnosticsEngine()
 let identityResolver = DefaultIdentityResolver()
-let manifest = try tsc_await { ManifestLoader.loadManifest(at: packagePath, kind: .local, swiftCompiler: swiftCompiler, swiftCompilerFlags: [], identityResolver: identityResolver, on: .global(), completion: $0) }
-let loadedPackage = try tsc_await { PackageBuilder.loadPackage(at: packagePath, swiftCompiler: swiftCompiler, swiftCompilerFlags: [], identityResolver: identityResolver, diagnostics: diagnostics, on: .global(), completion: $0) }
-let graph = try Workspace.loadGraph(packagePath: packagePath, swiftCompiler: swiftCompiler, swiftCompilerFlags: [], identityResolver: identityResolver, diagnostics: diagnostics)
+let manifest = try tsc_await { ManifestLoader.loadRootManifest(at: packagePath, swiftCompiler: swiftCompiler, swiftCompilerFlags: [], identityResolver: identityResolver, on: .global(), completion: $0) }
+let loadedPackage = try tsc_await { PackageBuilder.loadRootPackage(at: packagePath, swiftCompiler: swiftCompiler, swiftCompilerFlags: [], identityResolver: identityResolver, diagnostics: diagnostics, on: .global(), completion: $0) }
+let graph = try Workspace.loadRootGraph(at: packagePath, swiftCompiler: swiftCompiler, swiftCompilerFlags: [], identityResolver: identityResolver, diagnostics: diagnostics)
 
 // EXAMPLES
 // ========

diff --git a/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift b/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift
@@ -215,13 +215,8 @@ public struct BuildDescription: Codable {
         self.testDiscoveryCommands = testDiscoveryCommands
         self.copyCommands = copyCommands
 
-        self.builtTestProducts = try plan.buildProducts.filter{ $0.product.type == .test }.map { desc in
-            // FIXME(perf): Provide faster lookups.
-            guard let package = (plan.graph.packages.first{ $0.products.contains(desc.product) }) else {
-                throw InternalError("package with product \(desc.product) not found")
-            }
+        self.builtTestProducts = plan.buildProducts.filter{ $0.product.type == .test }.map { desc in
             return BuiltTestProduct(
-                packageName: package.name,
                 productName: desc.product.name,
                 binaryPath: desc.binary
             )

diff --git a/Sources/Commands/Describe.swift b/Sources/Commands/Describe.swift
@@ -55,7 +55,7 @@ fileprivate struct DescribedPackage: Encodable {
     let swiftLanguagesVersions: [String]?
 
     init(from package: Package) {
-        self.name = package.name
+        self.name = package.manifestName // TODO: rename property to manifestName?
         self.path = package.path.pathString
         self.toolsVersion = "\(package.manifest.toolsVersion.major).\(package.manifest.toolsVersion.minor)"
             + (package.manifest.toolsVersion.patch == 0 ? "" : ".\(package.manifest.toolsVersion.patch)")

diff --git a/Sources/Commands/SwiftPackageTool.swift b/Sources/Commands/SwiftPackageTool.swift
@@ -170,13 +170,16 @@ extension SwiftPackageTool {
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
 
-            let manifests = try temp_await {
+            let rootManifests = try temp_await {
                 workspace.loadRootManifests(packages: root.packages, diagnostics: swiftTool.diagnostics, completion: $0)
             }
-            guard let manifest = manifests.first else { return }
+            guard let rootManifest = rootManifests.first else {
+                throw StringError("invalid manifests at \(root.packages)")
+            }
 
             let builder = PackageBuilder(
-                manifest: manifest,
+                identity: .root(name: rootManifest.name),
+                manifest: rootManifest,
                 productFilter: .everything,
                 path: try swiftTool.getPackageRoot(),
                 xcTestMinimumDeploymentTargets: MinimumDeploymentTarget.default.xcTestMinimumDeploymentTargets,
@@ -237,12 +240,16 @@ extension SwiftPackageTool {
             // Get the root package.
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
-            let manifest = try temp_await {
+            let rootManifests = try temp_await {
                 workspace.loadRootManifests(packages: root.packages, diagnostics: swiftTool.diagnostics, completion: $0)
-            }[0]
+            }
+            guard let rootManifest = rootManifests.first else {
+                throw StringError("invalid manifests at \(root.packages)")
+            }
 
             let builder = PackageBuilder(
-                manifest: manifest,
+                identity: .root(name: rootManifest.name),
+                manifest: rootManifest,
                 productFilter: .everything,
                 path: try swiftTool.getPackageRoot(),
                 xcTestMinimumDeploymentTargets: [:], // Minimum deployment target does not matter for this operation.
@@ -366,15 +373,17 @@ extension SwiftPackageTool {
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
 
-            let manifests = try temp_await {
+            let rootManifests = try temp_await {
                 workspace.loadRootManifests(packages: root.packages, diagnostics: swiftTool.diagnostics, completion: $0)
             }
-            guard let manifest = manifests.first else { return }
+            guard let rootManifest = rootManifests.first else {
+                throw StringError("invalid manifests at \(root.packages)")
+            }
 
             let encoder = JSONEncoder.makeWithDefaults()
             encoder.userInfo[Manifest.dumpPackageKey] = true
 
-            let jsonData = try encoder.encode(manifest)
+            let jsonData = try encoder.encode(rootManifest)
             let jsonString = String(data: jsonData, encoding: .utf8)!
             print(jsonString)
         }
@@ -582,7 +591,7 @@ extension SwiftPackageTool {
                 destination = output
             } else {
                 let graph = try swiftTool.loadPackageGraph()
-                let packageName = graph.rootPackages[0].name
+                let packageName = graph.rootPackages[0].manifestName // TODO: use identity instead?
                 destination = packageRoot.appending(component: "\(packageName).zip")
             }
 
@@ -657,10 +666,10 @@ extension SwiftPackageTool {
                 dstdir = outpath.parentDirectory
             case let outpath?:
                 dstdir = outpath
-                projectName = graph.rootPackages[0].name
+                projectName = graph.rootPackages[0].manifestName // TODO: use identity instead?
             case _:
                 dstdir = try swiftTool.getPackageRoot()
-                projectName = graph.rootPackages[0].name
+                projectName = graph.rootPackages[0].manifestName // TODO: use identity instead?
             }
             let xcodeprojPath = Xcodeproj.buildXcodeprojPath(outputDir: dstdir, projectName: projectName)
 

diff --git a/Sources/Commands/SwiftTestTool.swift b/Sources/Commands/SwiftTestTool.swift
@@ -224,9 +224,12 @@ public struct SwiftTestTool: SwiftCommand {
         case .codeCovPath:
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
-            let rootManifest = try temp_await {
+            let rootManifests = try temp_await {
                 workspace.loadRootManifests(packages: root.packages, diagnostics: swiftTool.diagnostics, completion: $0)                
-            }[0]
+            }
+            guard let rootManifest = rootManifests.first else {
+                throw StringError("invalid manifests at \(root.packages)")
+            }
             let buildParameters = try swiftTool.buildParametersForTest()
             print(codeCovAsJSONPath(buildParameters: buildParameters, packageName: rootManifest.name))
 
@@ -352,6 +355,15 @@ public struct SwiftTestTool: SwiftCommand {
 
     /// Processes the code coverage data and emits a json.
     private func processCodeCoverage(_ testProducts: [BuiltTestProduct], swiftTool: SwiftTool) throws {
+        let workspace = try swiftTool.getActiveWorkspace()
+        let root = try swiftTool.getWorkspaceRoot()
+        let rootManifests = try temp_await {
+            workspace.loadRootManifests(packages: root.packages, diagnostics: swiftTool.diagnostics, completion: $0)
+        }
+        guard let rootManifest = rootManifests.first else {
+            throw StringError("invalid manifests at \(root.packages)")
+        }
+
         // Merge all the profraw files to produce a single profdata file.
         try mergeCodeCovRawDataFiles(swiftTool: swiftTool)
 
@@ -360,7 +372,7 @@ public struct SwiftTestTool: SwiftCommand {
             // Export the codecov data as JSON.
             let jsonPath = codeCovAsJSONPath(
                 buildParameters: buildParameters,
-                packageName: product.packageName)
+                packageName: rootManifest.name)
             try exportCodeCovAsJSON(to: jsonPath, testBinary: product.binaryPath, swiftTool: swiftTool)
         }
     }

diff --git a/Sources/Commands/show-dependencies.swift b/Sources/Commands/show-dependencies.swift
@@ -44,7 +44,7 @@ private final class PlainTextDumper: DependenciesDumper {
 
                 let pkgVersion = package.manifest.version?.description ?? "unspecified"
 
-                stream <<< "\(hanger)\(package.name)<\(package.manifest.packageLocation)@\(pkgVersion)>\n"
+                stream <<< "\(hanger)\(package.identity.description)<\(package.manifest.packageLocation)@\(pkgVersion)>\n"
 
                 if !package.dependencies.isEmpty {
                     let replacement = (index == packages.count - 1) ?  "    " : "│   "
@@ -69,7 +69,7 @@ private final class FlatListDumper: DependenciesDumper {
     func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
         func recursiveWalk(packages: [ResolvedPackage]) {
             for package in packages {
-                stream <<< package.name <<< "\n"
+                stream <<< package.identity.description <<< "\n"
                 if !package.dependencies.isEmpty {
                     recursiveWalk(packages: package.dependencies)
                 }
@@ -88,7 +88,7 @@ private final class DotDumper: DependenciesDumper {
             let url = package.manifest.packageLocation
             if nodesAlreadyPrinted.contains(url) { return }
             let pkgVersion = package.manifest.version?.description ?? "unspecified"
-            stream <<< #""\#(url)" [label="\#(package.name)\n\#(url)\n\#(pkgVersion)"]"# <<< "\n"
+            stream <<< #""\#(url)" [label="\#(package.identity.description)\n\#(url)\n\#(pkgVersion)"]"# <<< "\n"
             nodesAlreadyPrinted.insert(url)
         }
 
@@ -130,7 +130,7 @@ private final class JSONDumper: DependenciesDumper {
     func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
         func convert(_ package: ResolvedPackage) -> JSON {
             return .orderedDictionary([
-                "name": .string(package.name),
+                "name": .string(package.manifestName), // TODO: remove? add identity?
                 "url": .string(package.manifest.packageLocation),
                 "version": .string(package.manifest.version?.description ?? "unspecified"),
                 "path": .string(package.path.pathString),

diff --git a/Sources/PackageGraph/GraphLoadingNode.swift b/Sources/PackageGraph/GraphLoadingNode.swift
@@ -20,13 +20,17 @@ import TSCUtility
 /// - SeeAlso: DependencyResolutionNode
 public struct GraphLoadingNode: Equatable, Hashable {
 
+    /// The package identity.
+    public let identity: PackageIdentity
+
     /// The package manifest.
     public let manifest: Manifest
 
     /// The product filter applied to the package.
     public let productFilter: ProductFilter
 
-    public init(manifest: Manifest, productFilter: ProductFilter) {
+    public init(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter) {
+        self.identity = identity
         self.manifest = manifest
         self.productFilter = productFilter
     }
@@ -41,9 +45,9 @@ extension GraphLoadingNode: CustomStringConvertible {
     public var description: String {
         switch productFilter {
         case .everything:
-            return manifest.name
+            return self.manifest.name
         case .specific(let set):
-            return "\(manifest.name)[\(set.sorted().joined(separator: ", "))]"
+            return "\(self.manifest.name)[\(set.sorted().joined(separator: ", "))]"
         }
     }
 }
diff --git a/Sources/PackageGraph/LocalPackageContainer.swift b/Sources/PackageGraph/LocalPackageContainer.swift
@@ -39,22 +39,23 @@ public final class LocalPackageContainer: PackageContainer {
     private func loadManifest() throws -> Manifest {
         try manifest.memoize() {
             // Load the tools version.
-            let toolsVersion = try toolsVersionLoader.load(at: AbsolutePath(package.location), fileSystem: fileSystem)
+            let toolsVersion = try self.toolsVersionLoader.load(at: AbsolutePath(self.package.location), fileSystem: self.fileSystem)
 
             // Validate the tools version.
-            try toolsVersion.validateToolsVersion(self.currentToolsVersion, packagePath: package.location)
+            try toolsVersion.validateToolsVersion(self.currentToolsVersion, packagePath: self.package.location)
 
             // Load the manifest.
             // FIXME: this should not block
             return try temp_await {
-                manifestLoader.load(at: AbsolutePath(package.location),
-                                    packageKind: package.kind,
-                                    packageLocation: package.location,
+                manifestLoader.load(at: AbsolutePath(self.package.location),
+                                    packageIdentity: self.package.identity,
+                                    packageKind: self.package.kind,
+                                    packageLocation: self.package.location,
                                     version: nil,
                                     revision: nil,
                                     toolsVersion: toolsVersion,
-                                    identityResolver: identityResolver,
-                                    fileSystem: fileSystem,
+                                    identityResolver: self.identityResolver,
+                                    fileSystem: self.fileSystem,
                                     diagnostics: nil,
                                     on: .global(),
                                     completion: $0)