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

This reverts commit 953f368.
swiftlang · Mar 10, 2021 · ec24ca3 · ec24ca3
1 parent 85f81f3
commit ec24ca3
Show file tree

Hide file tree

Showing 41 changed files with 400 additions and 662 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.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)
+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)
 
 // EXAMPLES
 // ========

diff --git a/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift b/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift
@@ -215,8 +215,13 @@ public struct BuildDescription: Codable {
         self.testDiscoveryCommands = testDiscoveryCommands
         self.copyCommands = copyCommands
 
-        self.builtTestProducts = plan.buildProducts.filter{ $0.product.type == .test }.map { desc in
+        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")
+            }
             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.manifestName // TODO: rename property to manifestName?
+        self.name = package.name
         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,16 +170,13 @@ extension SwiftPackageTool {
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
 
-            let rootManifests = try temp_await {
+            let manifests = 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)")
-            }
+            guard let manifest = manifests.first else { return }
 
             let builder = PackageBuilder(
-                identity: .root(name: rootManifest.name),
-                manifest: rootManifest,
+                manifest: manifest,
                 productFilter: .everything,
                 path: try swiftTool.getPackageRoot(),
                 xcTestMinimumDeploymentTargets: MinimumDeploymentTarget.default.xcTestMinimumDeploymentTargets,
@@ -240,16 +237,12 @@ extension SwiftPackageTool {
             // Get the root package.
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
-            let rootManifests = try temp_await {
+            let manifest = 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)")
-            }
+            }[0]
 
             let builder = PackageBuilder(
-                identity: .root(name: rootManifest.name),
-                manifest: rootManifest,
+                manifest: manifest,
                 productFilter: .everything,
                 path: try swiftTool.getPackageRoot(),
                 xcTestMinimumDeploymentTargets: [:], // Minimum deployment target does not matter for this operation.
@@ -373,17 +366,15 @@ extension SwiftPackageTool {
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
 
-            let rootManifests = try temp_await {
+            let manifests = 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)")
-            }
+            guard let manifest = manifests.first else { return }
 
             let encoder = JSONEncoder.makeWithDefaults()
             encoder.userInfo[Manifest.dumpPackageKey] = true
 
-            let jsonData = try encoder.encode(rootManifest)
+            let jsonData = try encoder.encode(manifest)
             let jsonString = String(data: jsonData, encoding: .utf8)!
             print(jsonString)
         }
@@ -591,7 +582,7 @@ extension SwiftPackageTool {
                 destination = output
             } else {
                 let graph = try swiftTool.loadPackageGraph()
-                let packageName = graph.rootPackages[0].manifestName // TODO: use identity instead?
+                let packageName = graph.rootPackages[0].name
                 destination = packageRoot.appending(component: "\(packageName).zip")
             }
 
@@ -666,10 +657,10 @@ extension SwiftPackageTool {
                 dstdir = outpath.parentDirectory
             case let outpath?:
                 dstdir = outpath
-                projectName = graph.rootPackages[0].manifestName // TODO: use identity instead?
+                projectName = graph.rootPackages[0].name
             case _:
                 dstdir = try swiftTool.getPackageRoot()
-                projectName = graph.rootPackages[0].manifestName // TODO: use identity instead?
+                projectName = graph.rootPackages[0].name
             }
             let xcodeprojPath = Xcodeproj.buildXcodeprojPath(outputDir: dstdir, projectName: projectName)
 

diff --git a/Sources/Commands/SwiftTestTool.swift b/Sources/Commands/SwiftTestTool.swift
@@ -224,12 +224,9 @@ public struct SwiftTestTool: SwiftCommand {
         case .codeCovPath:
             let workspace = try swiftTool.getActiveWorkspace()
             let root = try swiftTool.getWorkspaceRoot()
-            let rootManifests = try temp_await {
+            let rootManifest = 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)")
-            }
+            }[0]
             let buildParameters = try swiftTool.buildParametersForTest()
             print(codeCovAsJSONPath(buildParameters: buildParameters, packageName: rootManifest.name))
 
@@ -355,15 +352,6 @@ 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)
 
@@ -372,7 +360,7 @@ public struct SwiftTestTool: SwiftCommand {
             // Export the codecov data as JSON.
             let jsonPath = codeCovAsJSONPath(
                 buildParameters: buildParameters,
-                packageName: rootManifest.name)
+                packageName: product.packageName)
             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.identity.description)<\(package.manifest.packageLocation)@\(pkgVersion)>\n"
+                stream <<< "\(hanger)\(package.name)<\(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.identity.description <<< "\n"
+                stream <<< package.name <<< "\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.identity.description)\n\#(url)\n\#(pkgVersion)"]"# <<< "\n"
+            stream <<< #""\#(url)" [label="\#(package.name)\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.manifestName), // TODO: remove? add identity?
+                "name": .string(package.name),
                 "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,17 +20,13 @@ 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(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter) {
-        self.identity = identity
+    public init(manifest: Manifest, productFilter: ProductFilter) {
         self.manifest = manifest
         self.productFilter = productFilter
     }
@@ -45,9 +41,9 @@ extension GraphLoadingNode: CustomStringConvertible {
     public var description: String {
         switch productFilter {
         case .everything:
-            return self.manifest.name
+            return manifest.name
         case .specific(let set):
-            return "\(self.manifest.name)[\(set.sorted().joined(separator: ", "))]"
+            return "\(manifest.name)[\(set.sorted().joined(separator: ", "))]"
         }
     }
 }
diff --git a/Sources/PackageGraph/LocalPackageContainer.swift b/Sources/PackageGraph/LocalPackageContainer.swift
@@ -39,23 +39,22 @@ public final class LocalPackageContainer: PackageContainer {
     private func loadManifest() throws -> Manifest {
         try manifest.memoize() {
             // Load the tools version.
-            let toolsVersion = try self.toolsVersionLoader.load(at: AbsolutePath(self.package.location), fileSystem: self.fileSystem)
+            let toolsVersion = try toolsVersionLoader.load(at: AbsolutePath(package.location), fileSystem: fileSystem)
 
             // Validate the tools version.
-            try toolsVersion.validateToolsVersion(self.currentToolsVersion, packagePath: self.package.location)
+            try toolsVersion.validateToolsVersion(self.currentToolsVersion, packagePath: package.location)
 
             // Load the manifest.
             // FIXME: this should not block
             return try temp_await {
-                manifestLoader.load(at: AbsolutePath(self.package.location),
-                                    packageIdentity: self.package.identity,
-                                    packageKind: self.package.kind,
-                                    packageLocation: self.package.location,
+                manifestLoader.load(at: AbsolutePath(package.location),
+                                    packageKind: package.kind,
+                                    packageLocation: package.location,
                                     version: nil,
                                     revision: nil,
                                     toolsVersion: toolsVersion,
-                                    identityResolver: self.identityResolver,
-                                    fileSystem: self.fileSystem,
+                                    identityResolver: identityResolver,
+                                    fileSystem: fileSystem,
                                     diagnostics: nil,
                                     on: .global(),
                                     completion: $0)