[SE-0226]: Remove test target dependencies from dependency resolution

Package.swift

@@ -236,7 +236,7 @@ let package = Package(
             dependencies: ["PackageLoading", "SPMTestSupport"]),
         .testTarget(
             name: "PackageModelTests",
-            dependencies: ["PackageModel"]),
+            dependencies: ["PackageModel", "SPMTestSupport"]),
         .testTarget(
             name: "PackageGraphTests",
             dependencies: ["PackageGraph", "SPMTestSupport"]),

Sources/Commands/SwiftPackageTool.swift

@@ -112,8 +112,7 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
             let builder = PackageBuilder(
                 manifest: manifest,
                 path: try getPackageRoot(),
-                diagnostics: diagnostics,
-                isRootPackage: true
+                diagnostics: diagnostics
             )
             let package = try builder.construct()
 
@@ -337,8 +336,7 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
             let builder = PackageBuilder(
                 manifest: manifest,
                 path: try getPackageRoot(),
-                diagnostics: diagnostics,
-                isRootPackage: true
+                diagnostics: diagnostics
             )
             let package = try builder.construct()
             describe(package, in: options.describeMode, on: stdoutStream)
@@ -823,15 +821,14 @@ fileprivate extension SwiftPackageTool {
         stream <<< "\n"
 
         for (package, change) in changes {
-            guard let packageName = package.name else { continue }
             let currentVersion = pins.pinsMap[package.identity]?.state.description ?? ""
             switch change {
             case let .added(requirement):
-                stream <<< "+ \(packageName) \(requirement.prettyPrinted)"
+                stream <<< "+ \(package.name) \(requirement.prettyPrinted)"
             case let .updated(requirement):
-                stream <<< "~ \(packageName) \(currentVersion) -> \(packageName) \(requirement.prettyPrinted)"
+                stream <<< "~ \(package.name) \(currentVersion) -> \(package.name) \(requirement.prettyPrinted)"
             case .removed:
-                stream <<< "- \(packageName) \(currentVersion)"
+                stream <<< "- \(package.name) \(currentVersion)"
             case .unchanged:
                 continue
             }

Sources/PackageGraph/DependencyResolver.swift

@@ -1009,8 +1009,8 @@ public class DependencyResolver {
             let incompatibleConstraints = constraints.filter{ $0.requirement == .unversioned }
             guard incompatibleConstraints.isEmpty else {
                 self.error = DependencyResolverError.revisionDependencyContainsLocalPackage(
-                    dependency: container.identifier.identity,
-                    localPackage: incompatibleConstraints[0].identifier.identity
+                    dependency: container.identifier.name,
+                    localPackage: incompatibleConstraints[0].identifier.name
                 )
                 return AnySequence([])
             }

Sources/PackageGraph/PackageGraphLoader.swift

@@ -35,7 +35,7 @@ extension PackageGraphError: CustomStringConvertible {
     public var description: String {
         switch self {
         case .noModules(let package):
-            return "package '\(package)' contains no targets"
+            return "package '\(package)' contains no products"
 
         case .cycleDetected(let cycle):
             return "cyclic dependency declaration found: " +
@@ -82,7 +82,7 @@ public struct PackageGraphLoader {
         let manifestMapSequence = (root.manifests + externalManifests).map({ (PackageReference.computeIdentity(packageURL: $0.url), $0) })
         let manifestMap = Dictionary(uniqueKeysWithValues: manifestMapSequence)
         let successors: (Manifest) -> [Manifest] = { manifest in
-            manifest.dependencies.compactMap({
+            manifest.allRequiredDependencies.compactMap({
                 let url = config.mirroredURL(forURL: $0.url)
                 return manifestMap[PackageReference.computeIdentity(packageURL: url)]
             })
@@ -111,8 +111,6 @@ public struct PackageGraphLoader {
         // Create the packages.
         var manifestToPackage: [Manifest: Package] = [:]
         for manifest in allManifests {
-            let isRootPackage = rootManifestSet.contains(manifest)
-
             // Derive the path to the package.
             //
             // FIXME: Lift this out of the manifest.
@@ -125,17 +123,16 @@ public struct PackageGraphLoader {
                 additionalFileRules: additionalFileRules,
                 fileSystem: fileSystem,
                 diagnostics: diagnostics,
-                isRootPackage: isRootPackage,
                 shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
-                createREPLProduct: isRootPackage ? createREPLProduct : false
+                createREPLProduct: manifest.packageKind == .root ? createREPLProduct : false
             )
 
             diagnostics.wrap(with: PackageLocation.Local(name: manifest.name, packagePath: packagePath), {
                 let package = try builder.construct()
                 manifestToPackage[manifest] = package
 
                 // Throw if any of the non-root package is empty.
-                if package.targets.isEmpty && !isRootPackage {
+                if package.targets.isEmpty && manifest.packageKind != .root {
                     throw PackageGraphError.noModules(package)
                 }
             })
@@ -231,7 +228,7 @@ private func createResolvedPackages(
         let package = packageBuilder.package
 
         // Establish the manifest-declared package dependencies.
-        packageBuilder.dependencies = package.manifest.dependencies.compactMap({
+        packageBuilder.dependencies = package.manifest.allRequiredDependencies.compactMap({
             let url = config.mirroredURL(forURL: $0.url)
             return packageMap[PackageReference.computeIdentity(packageURL: url)]
         })

Sources/PackageGraph/PackageGraphRoot.swift

@@ -61,7 +61,7 @@ public struct PackageGraphRoot {
             return PackageReference(
                 identity: PackageReference.computeIdentity(packageURL: effectiveURL),
                 path: effectiveURL,
-                isLocal: (requirement == .localPackage)
+                kind: requirement == .localPackage ? .local : .remote
             )
         }
 
@@ -96,7 +96,7 @@ public struct PackageGraphRoot {
     public init(input: PackageGraphRootInput, manifests: [Manifest]) {
         self.packageRefs = zip(input.packages, manifests).map { (path, manifest) in
             let identity = PackageReference.computeIdentity(packageURL: manifest.url)
-            return PackageReference(identity: identity, path: path.pathString, isLocal: true)
+            return PackageReference(identity: identity, path: path.pathString, kind: .root)
         }
         self.manifests = manifests
         self.dependencies = input.dependencies

Sources/PackageGraph/PinsStore.swift

@@ -148,7 +148,8 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable, Equatable {
     public init(json: JSON) throws {
         let name: String? = json.get("package")
         let url: String = try json.get("repositoryURL")
-        let ref = PackageReference(identity: PackageReference.computeIdentity(packageURL: url), path: url)
+        let identity = PackageReference.computeIdentity(packageURL: url)
+        let ref = PackageReference(identity: identity, path: url)
         self.packageRef = name.flatMap(ref.with(newName:)) ?? ref
         self.state = try json.get("state")
     }

Sources/PackageGraph/Pubgrub.swift

@@ -842,8 +842,8 @@ public final class PubgrubDependencyResolver {
                     constraints.append(dependency)
                 case .unversioned:
                     throw DependencyResolverError.revisionDependencyContainsLocalPackage(
-                        dependency: package.identity,
-                        localPackage: dependency.identifier.identity
+                        dependency: package.name,
+                        localPackage: dependency.identifier.name
                     )
                 }
             }
@@ -898,7 +898,7 @@ public final class PubgrubDependencyResolver {
             identity: "<synthesized-root>",
             path: "<synthesized-root-path>",
             name: nil,
-            isLocal: true
+            kind: .root
         )
 
         self.root = root
@@ -1459,7 +1459,7 @@ private final class DiagnosticReportBuilder {
     }
 
     private func description(for term: Term, normalizeRange: Bool = false) -> String {
-        let name = term.package.name ?? term.package.lastPathComponent
+        let name = term.package.name
 
         switch term.requirement {
         case .any: return "every version of \(name)"

Sources/PackageGraph/RawPackageConstraints.swift

@@ -18,7 +18,7 @@ extension PackageDependencyDescription {
         return PackageReference(
             identity: PackageReference.computeIdentity(packageURL: effectiveURL),
             path: effectiveURL,
-            isLocal: (requirement == .localPackage)
+            kind: requirement == .localPackage ? .local : .remote
         )
     }
 }
@@ -27,7 +27,7 @@ extension Manifest {
 
     /// Constructs constraints of the dependencies in the raw package.
     public func dependencyConstraints(config: SwiftPMConfig) -> [RepositoryPackageConstraint] {
-        return dependencies.map({
+        return allRequiredDependencies.map({
             return RepositoryPackageConstraint(
                 container: $0.createPackageRef(config: config),
                 requirement: $0.requirement.toConstraintRequirement())

Sources/PackageGraph/RepositoryPackageContainerProvider.swift

@@ -62,7 +62,7 @@ public class RepositoryPackageContainerProvider: PackageContainerProvider {
         completion: @escaping (Result<PackageContainer, AnyError>) -> Void
     ) {
         // If the container is local, just create and return a local package container.
-        if identifier.isLocal {
+        if identifier.kind != .remote {
             callbacksQueue.async {
                 let container = LocalPackageContainer(identifier,
                     config: self.config,
@@ -107,7 +107,7 @@ extension PackageReference {
     ///
     /// This should only be accessed when the reference is not local.
     public var repository: RepositorySpecifier {
-        precondition(!isLocal)
+        precondition(kind == .remote)
         return RepositorySpecifier(url: path)
     }
 }
@@ -210,6 +210,7 @@ public class LocalPackageContainer: BasePackageContainer, CustomStringConvertibl
             baseURL: identifier.path,
             version: nil,
             toolsVersion: toolsVersion,
+            packageKind: identifier.kind,
             fileSystem: fs)
         return _manifest!
     }
@@ -469,6 +470,7 @@ public class RepositoryPackageContainer: BasePackageContainer, CustomStringConve
             baseURL: packageURL,
             version: version,
             toolsVersion: toolsVersion,
+            packageKind: identifier.kind,
             fileSystem: fs)
     }
 }

Sources/PackageLoading/ManifestLoader.swift

@@ -68,13 +68,16 @@ public protocol ManifestLoaderProtocol {
     ///   - path: The root path of the package.
     ///   - baseURL: The URL the manifest was loaded from.
     ///   - version: The version the manifest is from, if known.
-    ///   - manifestVersion: The version of manifest to load.
+    ///   - toolsVersion: The version of the tools the manifest supports.
+    ///   - kind: The kind of package the manifest is from.
     ///   - fileSystem: If given, the file system to load from (otherwise load from the local file system).
+    ///   - diagnostics: The diagnostics engine.
     func load(
         packagePath path: AbsolutePath,
         baseURL: String,
         version: Version?,
         toolsVersion: ToolsVersion,
+        packageKind: PackageReference.Kind,
         fileSystem: FileSystem?,
         diagnostics: DiagnosticsEngine?
     ) throws -> Manifest
@@ -87,12 +90,16 @@ extension ManifestLoaderProtocol {
     ///   - path: The root path of the package.
     ///   - baseURL: The URL the manifest was loaded from.
     ///   - version: The version the manifest is from, if known.
-    ///   - fileSystem: The file system to load from.
+    ///   - toolsVersion: The version of the tools the manifest supports.
+    ///   - kind: The kind of package the manifest is from.
+    ///   - fileSystem: If given, the file system to load from (otherwise load from the local file system).
+    ///   - diagnostics: The diagnostics engine.
     public func load(
         package path: AbsolutePath,
         baseURL: String,
         version: Version? = nil,
         toolsVersion: ToolsVersion,
+        packageKind: PackageReference.Kind,
         fileSystem: FileSystem? = nil,
         diagnostics: DiagnosticsEngine? = nil
     ) throws -> Manifest {
@@ -101,6 +108,7 @@ extension ManifestLoaderProtocol {
             baseURL: baseURL,
             version: version,
             toolsVersion: toolsVersion,
+            packageKind: packageKind,
             fileSystem: fileSystem,
             diagnostics: diagnostics
         )
@@ -166,17 +174,20 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     ///     - packagePath: The absolute path of the package root.
     ///     - swiftCompiler: The absolute path of a `swiftc` executable.
     ///         Its associated resources will be used by the loader.
+    ///     - kind: The kind of package the manifest is from.
     public static func loadManifest(
         packagePath: AbsolutePath,
-        swiftCompiler: AbsolutePath
+        swiftCompiler: AbsolutePath,
+        packageKind: PackageReference.Kind
     ) throws -> Manifest {
         let resources = try UserManifestResources(swiftCompiler: swiftCompiler)
         let loader = ManifestLoader(manifestResources: resources)
         let toolsVersion = try ToolsVersionLoader().load(at: packagePath, fileSystem: localFileSystem)
         return try loader.load(
             package: packagePath,
             baseURL: packagePath.pathString,
-            toolsVersion: toolsVersion
+            toolsVersion: toolsVersion,
+            packageKind: packageKind
         )
     }
 
@@ -185,6 +196,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         baseURL: String,
         version: Version?,
         toolsVersion: ToolsVersion,
+        packageKind: PackageReference.Kind,
         fileSystem: FileSystem? = nil,
         diagnostics: DiagnosticsEngine? = nil
     ) throws -> Manifest {
@@ -193,6 +205,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
             baseURL: baseURL,
             version: version,
             toolsVersion: toolsVersion,
+            packageKind: packageKind,
             fileSystem: fileSystem,
             diagnostics: diagnostics
         )
@@ -204,12 +217,14 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     ///   - path: The path to the manifest file (or a package root).
     ///   - baseURL: The URL the manifest was loaded from.
     ///   - version: The version the manifest is from, if known.
+    ///   - kind: The kind of package the manifest is from.
     ///   - fileSystem: If given, the file system to load from (otherwise load from the local file system).
     func loadFile(
         path inputPath: AbsolutePath,
         baseURL: String,
         version: Version?,
         toolsVersion: ToolsVersion,
+        packageKind: PackageReference.Kind,
         fileSystem: FileSystem? = nil,
         diagnostics: DiagnosticsEngine? = nil
     ) throws -> Manifest {
@@ -260,6 +275,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
             url: baseURL,
             version: version,
             toolsVersion: toolsVersion,
+            packageKind: packageKind,
             pkgConfig: manifestBuilder.pkgConfig,
             providers: manifestBuilder.providers,
             cLanguageStandard: manifestBuilder.cLanguageStandard,