reduce the use of manifest name (#3851)

motivation: manifest name is for display purposes only

changes:
* rename Manifest::name to Manifest::displayName to more clearly articulate the purpose
* remove name attribute from Package and ResolvedPackage to make it harder to use it by mistake
* rename PackageRef::name attribute to PackageRef::deprecatedName to discourage the use of it (needed for backwards compatibility)
* update other call sites where its possible to use identity of name
* adjust tests

Author: tomerd

Sources/Build/BuildPlan.swift

@@ -2002,7 +2002,7 @@ public class BuildPlan {
         var arguments = ["-I" + buildPath, "-L" + buildPath]
 
         // Link the special REPL product that contains all of the library targets.
-        let replProductName = graph.rootPackages[0].manifest.name + Product.replProductSuffix
+        let replProductName = graph.rootPackages[0].identity.description + Product.replProductSuffix
         arguments.append("-l" + replProductName)
 
         // The graph should have the REPL product.

Sources/Commands/Describe.swift

@@ -14,7 +14,8 @@ import Foundation
 
 /// Represents a package for the sole purpose of generating a description.
 struct DescribedPackage: Encodable {
-    let name: String
+    let name: String // for backwards compatibility
+    let manifestDisplayName: String
     let path: String
     let toolsVersion: String
     let dependencies: [DescribedPackageDependency]
@@ -27,7 +28,8 @@ struct DescribedPackage: Encodable {
     let swiftLanguagesVersions: [String]?
 
     init(from package: Package) {
-        self.name = package.manifestName // TODO: rename property to manifestName?
+        self.manifestDisplayName = package.manifest.displayName
+        self.name = self.manifestDisplayName // TODO: deprecate, backwards compatibility 11/2021
         self.path = package.path.pathString
         self.toolsVersion = "\(package.manifest.toolsVersion.major).\(package.manifest.toolsVersion.minor)"
         + (package.manifest.toolsVersion.patch == 0 ? "" : ".\(package.manifest.toolsVersion.patch)")

Sources/Commands/SwiftPackageTool.swift

@@ -847,7 +847,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].manifest.displayName // TODO: use identity instead?
                 destination = packageRoot.appending(component: "\(packageName).zip")
             }
 
@@ -922,10 +922,10 @@ extension SwiftPackageTool {
                 dstdir = outpath.parentDirectory
             case let outpath?:
                 dstdir = outpath
-                projectName = graph.rootPackages[0].manifestName // TODO: use identity instead?
+                projectName = graph.rootPackages[0].manifest.displayName // TODO: use identity instead?
             case _:
                 dstdir = try swiftTool.getPackageRoot()
-                projectName = graph.rootPackages[0].manifestName // TODO: use identity instead?
+                projectName = graph.rootPackages[0].manifest.displayName // TODO: use identity instead?
             }
             let xcodeprojPath = Xcodeproj.buildXcodeprojPath(outputDir: dstdir, projectName: projectName)
 
@@ -1321,11 +1321,11 @@ fileprivate func logPackageChanges(changes: [(PackageReference, Workspace.Packag
         let currentVersion = pins.pinsMap[package.identity]?.state.description ?? ""
         switch change {
         case let .added(state):
-            stream <<< "+ \(package.name) \(state.requirement.prettyPrinted)"
+            stream <<< "+ \(package.identity) \(state.requirement.prettyPrinted)"
         case let .updated(state):
-            stream <<< "~ \(package.name) \(currentVersion) -> \(package.name) \(state.requirement.prettyPrinted)"
+            stream <<< "~ \(package.identity) \(currentVersion) -> \(package.identity) \(state.requirement.prettyPrinted)"
         case .removed:
-            stream <<< "- \(package.name) \(currentVersion)"
+            stream <<< "- \(package.identity) \(currentVersion)"
         case .unchanged:
             continue
         }

Sources/Commands/SwiftTestTool.swift

@@ -382,7 +382,7 @@ public struct SwiftTestTool: SwiftCommand {
             // Export the codecov data as JSON.
             let jsonPath = codeCovAsJSONPath(
                 buildParameters: buildParameters,
-                packageName: rootManifest.name)
+                packageName: rootManifest.displayName)
             try exportCodeCovAsJSON(to: jsonPath, testBinary: product.binaryPath, swiftTool: swiftTool)
         }
     }

Sources/Commands/show-dependencies.swift

@@ -114,7 +114,8 @@ 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?
+                "identity": .string(package.identity.description),
+                "name": .string(package.manifest.displayName), // TODO: remove?
                 "url": .string(package.manifest.packageLocation),
                 "version": .string(package.manifest.version?.description ?? "unspecified"),
                 "path": .string(package.path.pathString),

Sources/PackageGraph/DependencyResolutionNode.swift

@@ -95,7 +95,7 @@ public enum DependencyResolutionNode {
         // Don’t create a version lock for anything but a product.
         guard specificProduct != nil else { return nil }
         return PackageContainerConstraint(
-            package: package,
+            package: self.package,
             versionRequirement: .exact(version),
             products: .specific([])
         )
@@ -108,7 +108,7 @@ public enum DependencyResolutionNode {
         // Don’t create a revision lock for anything but a product.
         guard specificProduct != nil else { return nil }
         return PackageContainerConstraint(
-            package: package,
+            package: self.package,
             requirement: .revision(revision),
             products: .specific([])
         )
@@ -123,13 +123,13 @@ extension DependencyResolutionNode: Equatable {
 
 extension DependencyResolutionNode: Hashable {
     public func hash(into hasher: inout Hasher) {
-        hasher.combine(package)
-        hasher.combine(specificProduct)
+        hasher.combine(self.package)
+        hasher.combine(self.specificProduct)
     }
 }
 
 extension DependencyResolutionNode: CustomStringConvertible {
     public var description: String {
-        return "\(package.name)\(productFilter)"
+        return "\(self.package.identity)\(self.productFilter)"
     }
 }

Sources/PackageGraph/GraphLoadingNode.swift

@@ -44,9 +44,9 @@ extension GraphLoadingNode: CustomStringConvertible {
     public var description: String {
         switch productFilter {
         case .everything:
-            return self.manifest.name
+            return self.identity.description
         case .specific(let set):
-            return "\(self.manifest.name)[\(set.sorted().joined(separator: ", "))]"
+            return "\(self.identity.description)[\(set.sorted().joined(separator: ", "))]"
         }
     }
 }

Sources/PackageGraph/PackageContainer.swift

@@ -90,7 +90,7 @@ public protocol PackageContainer {
     /// This can be used by the containers to fill in the missing information that is obtained
     /// after the container is available. The updated identifier is returned in result of the
     /// dependency resolution.
-    func getUpdatedIdentifier(at boundVersion: BoundVersion) throws -> PackageReference
+    func loadPackageReference(at boundVersion: BoundVersion) throws -> PackageReference
 }
 
 extension PackageContainer {

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -279,7 +279,7 @@ private func createResolvedPackages(
                     // backwards compatibility with older versions of SwiftPM that had too weak of a validation
                     // we will upgrade this to an error in a few versions to tighten up the validation
                     if dependency.explicitNameForTargetDependencyResolutionOnly == .none ||
-                        resolvedPackage.package.manifestName == dependency.explicitNameForTargetDependencyResolutionOnly {
+                        resolvedPackage.package.manifest.displayName == dependency.explicitNameForTargetDependencyResolutionOnly {
                         packageObservabilityScope.emit(warning: error.description + ". this will be escalated to an error in future versions of SwiftPM.")
                     } else {
                         return packageObservabilityScope.emit(error)

Sources/PackageGraph/PackageGraph.swift

@@ -187,8 +187,8 @@ extension PackageGraphError: CustomStringConvertible {
 
         case .cycleDetected(let cycle):
             return "cyclic dependency declaration found: " +
-            (cycle.path + cycle.cycle).map({ $0.name }).joined(separator: " -> ") +
-            " -> " + cycle.cycle[0].name
+            (cycle.path + cycle.cycle).map({ $0.displayName }).joined(separator: " -> ") +
+            " -> " + cycle.cycle[0].displayName
 
         case .productDependencyNotFound(let package, let targetName, let dependencyProductName, let dependencyPackageName, let dependencyProductInDecl):
             if dependencyProductInDecl {

Sources/PackageGraph/PinsStore.swift

@@ -129,7 +129,7 @@ fileprivate struct PinsStorage {
                 let v1 = try decoder.decode(path: self.path, fileSystem: self.fileSystem, as: V1.self)
                 return try v1.object.pins.map{ try PinsStore.Pin($0, mirrors: mirrors) }.reduce(into: [PackageIdentity: PinsStore.Pin]()) { partial, iterator in
                     if partial.keys.contains(iterator.packageRef.identity) {
-                        throw StringError("duplicated entry for package \"\(iterator.packageRef.name)\"")
+                        throw StringError("duplicated entry for package \"\(iterator.packageRef.identity)\"")
                     }
                     partial[iterator.packageRef.identity] = iterator
                 }
@@ -234,7 +234,7 @@ fileprivate struct PinsStorage {
                     throw StringError("invalid package type \(pin.packageRef.kind)")
                 }
 
-                self.package = pin.packageRef.name
+                self.package = pin.packageRef.deprecatedName
                 // rdar://52529014, rdar://52529011: pin file should store the original location but remap when loading
                 self.repositoryURL = mirrors.originalURL(for: location) ?? location
                 self.state = .init(pin.state)
@@ -328,7 +328,7 @@ extension PinsStore.Pin {
             throw StringError("invalid package location \(location)")
         }
         if let newName = pin.package {
-            packageRef = packageRef.with(newName: newName)
+            packageRef = packageRef.withName(newName)
         }
         self.init(
             packageRef: packageRef,

Sources/PackageGraph/Pubgrub/PubgrubDependencyResolver.swift

@@ -215,18 +215,18 @@ public struct PubgrubDependencyResolver {
 
             // TODO: replace with async/await when available
             let container = try temp_await { provider.getContainer(for: assignment.term.node.package, completion: $0) }
-            let identifier = try container.underlying.getUpdatedIdentifier(at: boundVersion)
+            let updatePackage = try container.underlying.loadPackageReference(at: boundVersion)
 
-            if var existing = flattenedAssignments[identifier] {
+            if var existing = flattenedAssignments[updatePackage] {
                 assert(existing.binding == boundVersion, "Two products in one package resolved to different versions: \(existing.products)@\(existing.binding) vs \(products)@\(boundVersion)")
                 existing.products.formUnion(products)
-                flattenedAssignments[identifier] = existing
+                flattenedAssignments[updatePackage] = existing
             } else {
-                flattenedAssignments[identifier] = (binding: boundVersion, products: products)
+                flattenedAssignments[updatePackage] = (binding: boundVersion, products: products)
             }
         }
         var finalAssignments: [DependencyResolver.Binding]
-            = flattenedAssignments.keys.sorted(by: { $0.name < $1.name }).map { package in
+            = flattenedAssignments.keys.sorted(by: { $0.deprecatedName < $1.deprecatedName }).map { package in
                 let details = flattenedAssignments[package]!
                 return (package: package, binding: details.binding, products: details.products)
             }
@@ -235,8 +235,8 @@ public struct PubgrubDependencyResolver {
         for (package, override) in state.overriddenPackages {
             // TODO: replace with async/await when available
             let container = try temp_await { provider.getContainer(for: package, completion: $0) }
-            let identifier = try container.underlying.getUpdatedIdentifier(at: override.version)
-            finalAssignments.append((identifier, override.version, override.products))
+            let updatePackage = try container.underlying.loadPackageReference(at: override.version)
+            finalAssignments.append((updatePackage, override.version, override.products))
         }
 
         self.delegate?.solved(result: finalAssignments)
@@ -376,8 +376,8 @@ public struct PubgrubDependencyResolver {
                         constraints.append(dependency)
                     case .unversioned:
                         throw DependencyResolverError.revisionDependencyContainsLocalPackage(
-                            dependency: package.name,
-                            localPackage: dependency.package.name
+                            dependency: package.identity.description,
+                            localPackage: dependency.package.identity.description
                         )
                     }
                 }
@@ -1329,7 +1329,7 @@ private final class PubGrubPackageContainer {
         let versions: [Version] = try self.underlying.versionsAscending()
 
         guard let idx = versions.firstIndex(of: firstVersion) else {
-            throw InternalError("from version \(firstVersion) not found in \(node.package.name)")
+            throw InternalError("from version \(firstVersion) not found in \(node.package.identity)")
         }
 
         let sync = DispatchGroup()
@@ -1347,7 +1347,7 @@ private final class PubGrubPackageContainer {
         // timeout is a function of # of versions since we need to make several git operations per tag/version
         let timeout = DispatchTimeInterval.seconds(60 + versions.count)
         guard case .success = sync.wait(timeout: .now() + timeout) else {
-            throw StringError("timeout computing '\(node.package.name)' bounds")
+            throw StringError("timeout computing '\(node.package.identity)' bounds")
         }
 
         return (lowerBounds, upperBounds)
@@ -1386,7 +1386,7 @@ private final class ContainerProvider {
     /// Get a cached container for the given identifier, asserting / throwing if not found.
     func getCachedContainer(for package: PackageReference) throws -> PubGrubPackageContainer {
         guard let container = self.containersCache[package] else {
-            throw InternalError("container for \(package.name) expected to be cached")
+            throw InternalError("container for \(package.identity) expected to be cached")
         }
         return container
     }
@@ -1531,6 +1531,6 @@ private extension PackageRequirement {
 
 private extension DependencyResolutionNode {
     var nameForDiagnostics: String {
-        return "'\(package.name)'"
+        return "'\(package.identity)'"
     }
 }

Sources/PackageGraph/ResolvedPackage.swift

@@ -26,19 +26,6 @@ public final class ResolvedPackage {
         return self.underlyingPackage.manifest
     }
 
-    /// The name of the package.
-    @available(*, deprecated, message: "use identity (or manifestName, but only if you must) instead")
-    public var name: String {
-        return self.underlyingPackage.name
-    }
-
-    /// The name of the package as entered in the manifest.
-    /// This should rarely be used beyond presentation purposes
-    //@available(*, deprecated)
-    public var manifestName: String {
-        return self.underlyingPackage.manifestName
-    }
-
     /// The local path of the package.
     public var path: AbsolutePath {
         return self.underlyingPackage.path

Sources/PackageLoading/ManifestLoader.swift

@@ -274,7 +274,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
                 }
 
                 let manifest = Manifest(
-                    name: parsedManifest.name,
+                    displayName: parsedManifest.name,
                     path: path,
                     packageKind: packageKind,
                     packageLocation: packageLocation,