From 4f4ad9710ca7850f27e11bfd4c511a564f66cf43 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 15 Apr 2024 12:54:30 -0700 Subject: [PATCH 01/16] [PackageGraph] Remove uses of `availableLibraries` from ModulesGraph.load and related methods --- .../PackageGraph/ModulesGraph+Loading.swift | 45 ++++++++----------- Sources/Workspace/Workspace.swift | 1 - Sources/swift-bootstrap/main.swift | 1 - .../PackageGraphPerfTests.swift | 1 - 4 files changed, 18 insertions(+), 30 deletions(-) diff --git a/Sources/PackageGraph/ModulesGraph+Loading.swift b/Sources/PackageGraph/ModulesGraph+Loading.swift index 59687da73f3..b72e3fb265e 100644 --- a/Sources/PackageGraph/ModulesGraph+Loading.swift +++ b/Sources/PackageGraph/ModulesGraph+Loading.swift @@ -32,7 +32,6 @@ extension ModulesGraph { customPlatformsRegistry: PlatformRegistry? = .none, customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none, testEntryPointPath: AbsolutePath? = nil, - availableLibraries: [LibraryMetadata], fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws -> ModulesGraph { @@ -160,7 +159,6 @@ extension ModulesGraph { unsafeAllowedPackages: unsafeAllowedPackages, platformRegistry: customPlatformsRegistry ?? .default, platformVersionProvider: platformVersionProvider, - availableLibraries: availableLibraries, fileSystem: fileSystem, observabilityScope: observabilityScope ) @@ -250,7 +248,6 @@ private func createResolvedPackages( unsafeAllowedPackages: Set, platformRegistry: PlatformRegistry, platformVersionProvider: PlatformVersionProvider, - availableLibraries: [LibraryMetadata], fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws -> [ResolvedPackage] { @@ -529,32 +526,26 @@ private func createResolvedPackages( t.name != productRef.name } - let identitiesAvailableInSDK = availableLibraries.flatMap { $0.identities.map { $0.identity } } - // TODO: Do we have to care about "name" vs. identity here? - if let name = productRef.package, identitiesAvailableInSDK.contains(PackageIdentity.plain(name)) { - // Do not emit any diagnostic. - } else { - // Find a product name from the available product dependencies that is most similar to the required product name. - let bestMatchedProductName = bestMatch(for: productRef.name, from: Array(allTargetNames)) - var packageContainingBestMatchedProduct: String? - if let bestMatchedProductName, productRef.name == bestMatchedProductName { - let dependentPackages = packageBuilder.dependencies.map(\.package) - for p in dependentPackages where p.targets.contains(where: { $0.name == bestMatchedProductName }) { - packageContainingBestMatchedProduct = p.identity.description - break - } + // Find a product name from the available product dependencies that is most similar to the required product name. + let bestMatchedProductName = bestMatch(for: productRef.name, from: Array(allTargetNames)) + var packageContainingBestMatchedProduct: String? + if let bestMatchedProductName, productRef.name == bestMatchedProductName { + let dependentPackages = packageBuilder.dependencies.map(\.package) + for p in dependentPackages where p.targets.contains(where: { $0.name == bestMatchedProductName }) { + packageContainingBestMatchedProduct = p.identity.description + break } - let error = PackageGraphError.productDependencyNotFound( - package: package.identity.description, - targetName: targetBuilder.target.name, - dependencyProductName: productRef.name, - dependencyPackageName: productRef.package, - dependencyProductInDecl: !declProductsAsDependency.isEmpty, - similarProductName: bestMatchedProductName, - packageContainingSimilarProduct: packageContainingBestMatchedProduct - ) - packageObservabilityScope.emit(error) } + let error = PackageGraphError.productDependencyNotFound( + package: package.identity.description, + targetName: targetBuilder.target.name, + dependencyProductName: productRef.name, + dependencyPackageName: productRef.package, + dependencyProductInDecl: !declProductsAsDependency.isEmpty, + similarProductName: bestMatchedProductName, + packageContainingSimilarProduct: packageContainingBestMatchedProduct + ) + packageObservabilityScope.emit(error) } continue } diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index e6016e3d554..802bc1065c9 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -924,7 +924,6 @@ extension Workspace { createREPLProduct: self.configuration.createREPLProduct, customXCTestMinimumDeploymentTargets: customXCTestMinimumDeploymentTargets, testEntryPointPath: testEntryPointPath, - availableLibraries: self.providedLibraries, fileSystem: self.fileSystem, observabilityScope: observabilityScope ) diff --git a/Sources/swift-bootstrap/main.swift b/Sources/swift-bootstrap/main.swift index 6cf38354370..5b7ca511708 100644 --- a/Sources/swift-bootstrap/main.swift +++ b/Sources/swift-bootstrap/main.swift @@ -390,7 +390,6 @@ struct SwiftBootstrapBuildTool: ParsableCommand { partial[item.key] = (manifest: item.value, fs: self.fileSystem) }, binaryArtifacts: [:], - availableLibraries: [], // assume no provided libraries during bootstrap fileSystem: fileSystem, observabilityScope: observabilityScope ) diff --git a/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift b/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift index 78b0298697e..c469caa9e88 100644 --- a/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift +++ b/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift @@ -97,7 +97,6 @@ final class PackageGraphPerfTests: XCTestCasePerf { identityResolver: identityResolver, externalManifests: externalManifests, binaryArtifacts: [:], - availableLibraries: [], // assume no provided libraries for testing. fileSystem: fs, observabilityScope: observability.topScope ) From c01182b270e3fbc546aa868c6eb4a818e1c4624e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 16 Apr 2024 09:11:07 -0700 Subject: [PATCH 02/16] Add new `providedLibrary` package reference kind This kind of reference is going to be returned by package resolution if it detects that a provided library version matches all of the constraints. This makes it easier to handle provided libraries post-resolution instead of eliding them completely. --- Sources/Build/BuildPlan/BuildPlan.swift | 2 +- Sources/PackageGraph/ModulesGraph.swift | 3 +- .../PubGrub/PubGrubDependencyResolver.swift | 2 +- .../PackageLoading/ManifestJSONParser.swift | 2 +- Sources/PackageModel/IdentityResolver.swift | 2 + Sources/PackageModel/PackageReference.swift | 18 ++++- .../Plugins/PluginContextSerializer.swift | 2 + .../SPMTestSupport/MockManifestLoader.swift | 6 ++ Sources/SPMTestSupport/MockWorkspace.swift | 2 + Sources/Workspace/CMakeLists.txt | 1 + Sources/Workspace/Diagnostics.swift | 2 +- .../ProvidedLibraryPackageContainer.swift | 74 +++++++++++++++++++ .../Workspace+PackageContainer.swift | 8 ++ Sources/Workspace/Workspace+State.swift | 12 +++ Sources/Workspace/Workspace.swift | 2 + .../PackageLoadingTests/PDLoadingTests.swift | 2 + 16 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 Sources/Workspace/PackageContainer/ProvidedLibraryPackageContainer.swift diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 8f03a297328..123d34e3e55 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -740,7 +740,7 @@ extension ResolvedPackage { switch self.underlying.manifest.packageKind { case .registry, .remoteSourceControl, .localSourceControl: return true - case .root, .fileSystem: + case .root, .fileSystem, .providedLibrary: return false } } diff --git a/Sources/PackageGraph/ModulesGraph.swift b/Sources/PackageGraph/ModulesGraph.swift index 8160fc5f84a..ba29c9ef906 100644 --- a/Sources/PackageGraph/ModulesGraph.swift +++ b/Sources/PackageGraph/ModulesGraph.swift @@ -358,7 +358,8 @@ extension PackageGraphError: CustomStringConvertible { switch $0.manifest.packageKind { case .root(let path), .fileSystem(let path), - .localSourceControl(let path): + .localSourceControl(let path), + .providedLibrary(let path): description += " (at '\(path)')" case .remoteSourceControl(let url): description += " (from '\(url)')" diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index ffee1492d1e..553b354790a 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -897,7 +897,7 @@ extension PackageRequirement { extension PackageReference { public func matchingPrebuiltLibrary(in availableLibraries: [LibraryMetadata]) -> LibraryMetadata? { switch self.kind { - case .fileSystem, .localSourceControl, .root: + case .fileSystem, .localSourceControl, .root, .providedLibrary: return nil // can never match a prebuilt library case .registry(let identity): if let registryIdentity = identity.registry { diff --git a/Sources/PackageLoading/ManifestJSONParser.swift b/Sources/PackageLoading/ManifestJSONParser.swift index 564e5991982..7b95ae7ab93 100644 --- a/Sources/PackageLoading/ManifestJSONParser.swift +++ b/Sources/PackageLoading/ManifestJSONParser.swift @@ -83,7 +83,7 @@ enum ManifestJSONParser { case .localSourceControl(let _packagePath): // we have a more accurate path than the virtual one packagePath = _packagePath - case .root(let _packagePath), .fileSystem(let _packagePath): + case .root(let _packagePath), .fileSystem(let _packagePath), .providedLibrary(let _packagePath): // we dont have a more accurate path, and they should be the same // asserting (debug only) to make sure refactoring is correct 11/2023 assert(packagePath == _packagePath, "expecting package path '\(packagePath)' to be the same as '\(_packagePath)'") diff --git a/Sources/PackageModel/IdentityResolver.swift b/Sources/PackageModel/IdentityResolver.swift index 847836ecf2d..63bc0b560c9 100644 --- a/Sources/PackageModel/IdentityResolver.swift +++ b/Sources/PackageModel/IdentityResolver.swift @@ -46,6 +46,8 @@ public struct DefaultIdentityResolver: IdentityResolver { return try self.resolveIdentity(for: url) case .registry(let identity): return identity + case .providedLibrary(let path): + return try self.resolveIdentity(for: path) } } diff --git a/Sources/PackageModel/PackageReference.swift b/Sources/PackageModel/PackageReference.swift index ff6b448101c..448909fe03b 100644 --- a/Sources/PackageModel/PackageReference.swift +++ b/Sources/PackageModel/PackageReference.swift @@ -34,6 +34,9 @@ public struct PackageReference { /// A package from a registry. case registry(PackageIdentity) + /// A prebuilt library provided by a toolchain + case providedLibrary(AbsolutePath) + // FIXME: we should not need this once we migrate off URLs //@available(*, deprecated) public var locationString: String { @@ -49,6 +52,8 @@ public struct PackageReference { case .registry(let identity): // FIXME: this is a placeholder return identity.description + case .providedLibrary(let path): + return path.pathString } } @@ -70,6 +75,8 @@ public struct PackageReference { return "remoteSourceControl \(url)" case .registry(let identity): return "registry \(identity)" + case .providedLibrary(let path): + return "library \(path)" } } @@ -124,6 +131,8 @@ public struct PackageReference { case .registry(let identity): // FIXME: this is a placeholder self.deprecatedName = name ?? identity.description + case .providedLibrary(let path): + self.deprecatedName = name ?? PackageIdentityParser.computeDefaultName(fromPath: path) } } @@ -151,6 +160,10 @@ public struct PackageReference { public static func registry(identity: PackageIdentity) -> PackageReference { PackageReference(identity: identity, kind: .registry(identity)) } + + public static func library(identity: PackageIdentity, path: AbsolutePath) -> PackageReference { + PackageReference(identity: identity, kind: .providedLibrary(path)) + } } extension PackageReference: Equatable { @@ -203,7 +216,7 @@ extension PackageReference: CustomStringConvertible { extension PackageReference.Kind: Encodable { private enum CodingKeys: String, CodingKey { - case root, fileSystem, localSourceControl, remoteSourceControl, registry + case root, fileSystem, localSourceControl, remoteSourceControl, registry, providedLibrary } public func encode(to encoder: Encoder) throws { @@ -224,6 +237,9 @@ extension PackageReference.Kind: Encodable { case .registry: var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .registry) try unkeyedContainer.encode(self.isRoot) + case .providedLibrary(let path): + var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .providedLibrary) + try unkeyedContainer.encode(path) } } } diff --git a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift index 9edfb0a7bb4..3816f7791b5 100644 --- a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift +++ b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift @@ -240,6 +240,8 @@ internal struct PluginContextSerializer { return .repository(url: url.absoluteString, displayVersion: String(describing: package.manifest.version), scmRevision: String(describing: package.manifest.revision)) case .registry(let identity): return .registry(identity: identity.description, displayVersion: String(describing: package.manifest.version)) + case .providedLibrary(_): + throw InternalError("provided libraries are not supported in plugin context") } } diff --git a/Sources/SPMTestSupport/MockManifestLoader.swift b/Sources/SPMTestSupport/MockManifestLoader.swift index fc1f345ed64..08b1bf32540 100644 --- a/Sources/SPMTestSupport/MockManifestLoader.swift +++ b/Sources/SPMTestSupport/MockManifestLoader.swift @@ -109,6 +109,9 @@ extension ManifestLoader { packageIdentity = identity // FIXME: placeholder packageLocation = identity.description + case .providedLibrary(let path): + packageIdentity = try identityResolver.resolveIdentity(for: path) + packageLocation = path.pathString } return try await self.load( manifestPath: manifestPath, @@ -156,6 +159,9 @@ extension ManifestLoader { packageIdentity = identity // FIXME: placeholder packageLocation = identity.description + case .providedLibrary(let path): + packageIdentity = try identityResolver.resolveIdentity(for: path) + packageLocation = path.pathString } return try await self.load( packagePath: packagePath, diff --git a/Sources/SPMTestSupport/MockWorkspace.swift b/Sources/SPMTestSupport/MockWorkspace.swift index 717fc4f50ee..81056281f22 100644 --- a/Sources/SPMTestSupport/MockWorkspace.swift +++ b/Sources/SPMTestSupport/MockWorkspace.swift @@ -1041,6 +1041,8 @@ extension PackageReference.Kind { return "remoteSourceControl" case .registry: return "registry" + case .providedLibrary: + return "library" } } } diff --git a/Sources/Workspace/CMakeLists.txt b/Sources/Workspace/CMakeLists.txt index 7817b6b4a43..447c3353684 100644 --- a/Sources/Workspace/CMakeLists.txt +++ b/Sources/Workspace/CMakeLists.txt @@ -15,6 +15,7 @@ add_library(Workspace ManagedArtifact.swift ManagedDependency.swift PackageContainer/FileSystemPackageContainer.swift + PackageContainer/ProvidedLibraryPackageContainer.swift PackageContainer/RegistryPackageContainer.swift PackageContainer/SourceControlPackageContainer.swift ResolvedFileWatcher.swift diff --git a/Sources/Workspace/Diagnostics.swift b/Sources/Workspace/Diagnostics.swift index 8bf918875bc..007214647d1 100644 --- a/Sources/Workspace/Diagnostics.swift +++ b/Sources/Workspace/Diagnostics.swift @@ -185,7 +185,7 @@ extension Basics.Diagnostic { return "'\(identity.description)'" case .remoteSourceControl(let url): return "'\($0.identity)' from \(url)" - case .localSourceControl(let path), .fileSystem(let path), .root(let path): + case .localSourceControl(let path), .fileSystem(let path), .root(let path), .providedLibrary(let path): return "'\($0.identity)' at \(path)" } } diff --git a/Sources/Workspace/PackageContainer/ProvidedLibraryPackageContainer.swift b/Sources/Workspace/PackageContainer/ProvidedLibraryPackageContainer.swift new file mode 100644 index 00000000000..f423dcf454a --- /dev/null +++ b/Sources/Workspace/PackageContainer/ProvidedLibraryPackageContainer.swift @@ -0,0 +1,74 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +import Dispatch +import PackageGraph +import PackageLoading +import PackageModel + +import struct TSCUtility.Version + +public struct ProvidedLibraryPackageContainer: PackageContainer { + public let package: PackageReference + + /// Observability scope to emit diagnostics + private let observabilityScope: ObservabilityScope + + public init( + package: PackageReference, + observabilityScope: ObservabilityScope + ) throws { + switch package.kind { + case .providedLibrary: + break + default: + throw InternalError("invalid package type \(package.kind)") + } + self.package = package + self.observabilityScope = observabilityScope.makeChildScope( + description: "ProvidedLibraryPackageContainer", + metadata: package.diagnosticsMetadata) + } + + public func isToolsVersionCompatible(at version: Version) -> Bool { + true + } + + public func toolsVersion(for version: Version) throws -> ToolsVersion { + .v6_0 + } + + public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] { + return try versionsDescending() + } + + public func versionsAscending() throws -> [Version] { + [] // TODO + } + + public func getDependencies(at version: Version, productFilter: ProductFilter) throws -> [PackageContainerConstraint] { + [] + } + + public func getDependencies(at revision: String, productFilter: ProductFilter) throws -> [PackageContainerConstraint] { + [] + } + + public func getUnversionedDependencies(productFilter: ProductFilter) throws -> [PackageContainerConstraint] { + [] + } + + public func loadPackageReference(at boundVersion: BoundVersion) throws -> PackageReference { + package + } +} diff --git a/Sources/Workspace/Workspace+PackageContainer.swift b/Sources/Workspace/Workspace+PackageContainer.swift index 4787e3adef4..4ba1016756a 100644 --- a/Sources/Workspace/Workspace+PackageContainer.swift +++ b/Sources/Workspace/Workspace+PackageContainer.swift @@ -93,6 +93,14 @@ extension Workspace: PackageContainerProvider { queue.async { completion(.success(container)) } + + case .providedLibrary: + let container = try ProvidedLibraryPackageContainer( + package: package, + observabilityScope: observabilityScope) + queue.async { + completion(.success(container)) + } } } catch { queue.async { diff --git a/Sources/Workspace/Workspace+State.swift b/Sources/Workspace/Workspace+State.swift index 0d70ee2af0c..784ae7f3aff 100644 --- a/Sources/Workspace/Workspace+State.swift +++ b/Sources/Workspace/Workspace+State.swift @@ -440,6 +440,9 @@ extension WorkspaceStateStorage { self.kind = .registry // FIXME: placeholder self.location = self.identity.description + case .providedLibrary(let path): + self.kind = .providedLibrary + self.location = path.pathString } self.name = reference.deprecatedName } @@ -450,6 +453,7 @@ extension WorkspaceStateStorage { case localSourceControl case remoteSourceControl case registry + case providedLibrary } } } @@ -492,6 +496,8 @@ extension PackageModel.PackageReference { kind = .remoteSourceControl(SourceControlURL(reference.location)) case .registry: kind = .registry(identity) + case .providedLibrary: + kind = try .providedLibrary(.init(validating: reference.location)) } self.init( @@ -766,6 +772,9 @@ extension WorkspaceStateStorage { self.kind = .registry // FIXME: placeholder self.location = self.identity.description + case .providedLibrary(let path): + self.kind = .providedLibrary + self.location = path.pathString } self.name = reference.deprecatedName } @@ -776,6 +785,7 @@ extension WorkspaceStateStorage { case localSourceControl case remoteSourceControl case registry + case providedLibrary } } } @@ -819,6 +829,8 @@ extension PackageModel.PackageReference { kind = .remoteSourceControl(SourceControlURL(reference.location)) case .registry: kind = .registry(identity) + case .providedLibrary: + kind = try .providedLibrary(.init(validating: reference.location)) } self.init( diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 802bc1065c9..0f81611f1fe 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -1269,6 +1269,8 @@ extension Workspace { try self.removeRepository(dependency: dependencyToRemove) case .registry: try self.removeRegistryArchive(for: dependencyToRemove) + case .providedLibrary: + break // NOOP } // Save the state. diff --git a/Tests/PackageLoadingTests/PDLoadingTests.swift b/Tests/PackageLoadingTests/PDLoadingTests.swift index aa2bfd7af44..08c82442d63 100644 --- a/Tests/PackageLoadingTests/PDLoadingTests.swift +++ b/Tests/PackageLoadingTests/PDLoadingTests.swift @@ -97,6 +97,8 @@ class PackageDescriptionLoadingTests: XCTestCase, ManifestLoaderDelegate { packagePath = path case .remoteSourceControl, .registry: packagePath = .root + case .providedLibrary(let path): + packagePath = path } let toolsVersion = toolsVersion From 0536bb72d14abcc49a2e34e083ba6697cb59568b Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 16 Apr 2024 11:30:48 -0700 Subject: [PATCH 03/16] Expanded `providedLibrary` PackageReference to include origin This is useful for identity comparisons to enable replacement library to match against original remote package. --- Sources/PackageGraph/ModulesGraph.swift | 5 +-- .../PackageLoading/ManifestJSONParser.swift | 2 +- Sources/PackageModel/IdentityResolver.swift | 4 +-- Sources/PackageModel/PackageReference.swift | 32 +++++++++++------ .../SPMTestSupport/MockManifestLoader.swift | 8 ++--- Sources/Workspace/Diagnostics.swift | 4 +-- Sources/Workspace/Workspace+State.swift | 34 ++++++++++++++++--- .../PackageLoadingTests/PDLoadingTests.swift | 2 +- 8 files changed, 64 insertions(+), 27 deletions(-) diff --git a/Sources/PackageGraph/ModulesGraph.swift b/Sources/PackageGraph/ModulesGraph.swift index ba29c9ef906..49b2ea1f979 100644 --- a/Sources/PackageGraph/ModulesGraph.swift +++ b/Sources/PackageGraph/ModulesGraph.swift @@ -358,11 +358,12 @@ extension PackageGraphError: CustomStringConvertible { switch $0.manifest.packageKind { case .root(let path), .fileSystem(let path), - .localSourceControl(let path), - .providedLibrary(let path): + .localSourceControl(let path): description += " (at '\(path)')" case .remoteSourceControl(let url): description += " (from '\(url)')" + case .providedLibrary(let url, let path): + description += " (from \(url) at \(path))" case .registry: break } diff --git a/Sources/PackageLoading/ManifestJSONParser.swift b/Sources/PackageLoading/ManifestJSONParser.swift index 7b95ae7ab93..3818c565828 100644 --- a/Sources/PackageLoading/ManifestJSONParser.swift +++ b/Sources/PackageLoading/ManifestJSONParser.swift @@ -83,7 +83,7 @@ enum ManifestJSONParser { case .localSourceControl(let _packagePath): // we have a more accurate path than the virtual one packagePath = _packagePath - case .root(let _packagePath), .fileSystem(let _packagePath), .providedLibrary(let _packagePath): + case .root(let _packagePath), .fileSystem(let _packagePath), .providedLibrary(_, let _packagePath): // we dont have a more accurate path, and they should be the same // asserting (debug only) to make sure refactoring is correct 11/2023 assert(packagePath == _packagePath, "expecting package path '\(packagePath)' to be the same as '\(_packagePath)'") diff --git a/Sources/PackageModel/IdentityResolver.swift b/Sources/PackageModel/IdentityResolver.swift index 63bc0b560c9..9755d7e26f3 100644 --- a/Sources/PackageModel/IdentityResolver.swift +++ b/Sources/PackageModel/IdentityResolver.swift @@ -46,8 +46,8 @@ public struct DefaultIdentityResolver: IdentityResolver { return try self.resolveIdentity(for: url) case .registry(let identity): return identity - case .providedLibrary(let path): - return try self.resolveIdentity(for: path) + case .providedLibrary(let url, _): + return try self.resolveIdentity(for: url) } } diff --git a/Sources/PackageModel/PackageReference.swift b/Sources/PackageModel/PackageReference.swift index 448909fe03b..545a0f6ade8 100644 --- a/Sources/PackageModel/PackageReference.swift +++ b/Sources/PackageModel/PackageReference.swift @@ -34,8 +34,8 @@ public struct PackageReference { /// A package from a registry. case registry(PackageIdentity) - /// A prebuilt library provided by a toolchain - case providedLibrary(AbsolutePath) + /// A prebuilt library provided by a toolchain for a package identified by the given "origin" URL. + case providedLibrary(SourceControlURL, AbsolutePath) // FIXME: we should not need this once we migrate off URLs //@available(*, deprecated) @@ -52,8 +52,8 @@ public struct PackageReference { case .registry(let identity): // FIXME: this is a placeholder return identity.description - case .providedLibrary(let path): - return path.pathString + case .providedLibrary(let url, _): + return url.absoluteString } } @@ -75,8 +75,8 @@ public struct PackageReference { return "remoteSourceControl \(url)" case .registry(let identity): return "registry \(identity)" - case .providedLibrary(let path): - return "library \(path)" + case .providedLibrary(let url, let path): + return "provided library for \(url) @ \(path)" } } @@ -131,8 +131,8 @@ public struct PackageReference { case .registry(let identity): // FIXME: this is a placeholder self.deprecatedName = name ?? identity.description - case .providedLibrary(let path): - self.deprecatedName = name ?? PackageIdentityParser.computeDefaultName(fromPath: path) + case .providedLibrary(let url, _): + self.deprecatedName = name ?? PackageIdentityParser.computeDefaultName(fromURL: url) } } @@ -161,8 +161,12 @@ public struct PackageReference { PackageReference(identity: identity, kind: .registry(identity)) } - public static func library(identity: PackageIdentity, path: AbsolutePath) -> PackageReference { - PackageReference(identity: identity, kind: .providedLibrary(path)) + public static func providedLibrary( + identity: PackageIdentity, + origin: SourceControlURL, + path: AbsolutePath + ) -> PackageReference { + PackageReference(identity: identity, kind: .providedLibrary(origin, path)) } } @@ -183,6 +187,11 @@ extension PackageReference: Equatable { switch (self.kind, other.kind) { case (.remoteSourceControl(let lurl), .remoteSourceControl(let rurl)): return lurl.canonicalURL == rurl.canonicalURL + case (.remoteSourceControl(let lurl), .providedLibrary(let rurl, _)), + (.providedLibrary(let lurl, _), .remoteSourceControl(let rurl)): + return lurl.canonicalURL == rurl.canonicalURL + case (.providedLibrary(_, let lpath), .providedLibrary(_, let rpath)): + return lpath == rpath default: return true } @@ -237,8 +246,9 @@ extension PackageReference.Kind: Encodable { case .registry: var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .registry) try unkeyedContainer.encode(self.isRoot) - case .providedLibrary(let path): + case .providedLibrary(let url, let path): var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .providedLibrary) + try unkeyedContainer.encode(url) try unkeyedContainer.encode(path) } } diff --git a/Sources/SPMTestSupport/MockManifestLoader.swift b/Sources/SPMTestSupport/MockManifestLoader.swift index 08b1bf32540..70da34b5cc6 100644 --- a/Sources/SPMTestSupport/MockManifestLoader.swift +++ b/Sources/SPMTestSupport/MockManifestLoader.swift @@ -109,8 +109,8 @@ extension ManifestLoader { packageIdentity = identity // FIXME: placeholder packageLocation = identity.description - case .providedLibrary(let path): - packageIdentity = try identityResolver.resolveIdentity(for: path) + case .providedLibrary(let url, let path): + packageIdentity = try identityResolver.resolveIdentity(for: url) packageLocation = path.pathString } return try await self.load( @@ -159,8 +159,8 @@ extension ManifestLoader { packageIdentity = identity // FIXME: placeholder packageLocation = identity.description - case .providedLibrary(let path): - packageIdentity = try identityResolver.resolveIdentity(for: path) + case .providedLibrary(let url, let path): + packageIdentity = try identityResolver.resolveIdentity(for: url) packageLocation = path.pathString } return try await self.load( diff --git a/Sources/Workspace/Diagnostics.swift b/Sources/Workspace/Diagnostics.swift index 007214647d1..aef1442693b 100644 --- a/Sources/Workspace/Diagnostics.swift +++ b/Sources/Workspace/Diagnostics.swift @@ -183,9 +183,9 @@ extension Basics.Diagnostic { switch $0.kind { case .registry(let identity): return "'\(identity.description)'" - case .remoteSourceControl(let url): + case .remoteSourceControl(let url), .providedLibrary(let url, _): return "'\($0.identity)' from \(url)" - case .localSourceControl(let path), .fileSystem(let path), .root(let path), .providedLibrary(let path): + case .localSourceControl(let path), .fileSystem(let path), .root(let path): return "'\($0.identity)' at \(path)" } } diff --git a/Sources/Workspace/Workspace+State.swift b/Sources/Workspace/Workspace+State.swift index 784ae7f3aff..bd4db386e2e 100644 --- a/Sources/Workspace/Workspace+State.swift +++ b/Sources/Workspace/Workspace+State.swift @@ -420,6 +420,7 @@ extension WorkspaceStateStorage { let kind: Kind let location: String let name: String + let originURL: String? init(_ reference: PackageModel.PackageReference) { self.identity = reference.identity.description @@ -427,21 +428,27 @@ extension WorkspaceStateStorage { case .root(let path): self.kind = .root self.location = path.pathString + self.originURL = nil case .fileSystem(let path): self.kind = .fileSystem self.location = path.pathString + self.originURL = nil case .localSourceControl(let path): self.kind = .localSourceControl self.location = path.pathString + self.originURL = nil case .remoteSourceControl(let url): self.kind = .remoteSourceControl self.location = url.absoluteString + self.originURL = nil case .registry: self.kind = .registry // FIXME: placeholder self.location = self.identity.description - case .providedLibrary(let path): + self.originURL = nil + case .providedLibrary(let url, let path): self.kind = .providedLibrary + self.originURL = url.absoluteString self.location = path.pathString } self.name = reference.deprecatedName @@ -497,7 +504,13 @@ extension PackageModel.PackageReference { case .registry: kind = .registry(identity) case .providedLibrary: - kind = try .providedLibrary(.init(validating: reference.location)) + guard let url = reference.originURL else { + throw InternalError("Cannot form provided library reference without origin: \(reference)") + } + kind = try .providedLibrary( + SourceControlURL(url), + .init(validating: reference.location) + ) } self.init( @@ -752,6 +765,7 @@ extension WorkspaceStateStorage { let kind: Kind let location: String let name: String + let originURL: String? init(_ reference: PackageModel.PackageReference) { self.identity = reference.identity.description @@ -759,21 +773,27 @@ extension WorkspaceStateStorage { case .root(let path): self.kind = .root self.location = path.pathString + self.originURL = nil case .fileSystem(let path): self.kind = .fileSystem self.location = path.pathString + self.originURL = nil case .localSourceControl(let path): self.kind = .localSourceControl self.location = path.pathString + self.originURL = nil case .remoteSourceControl(let url): self.kind = .remoteSourceControl self.location = url.absoluteString + self.originURL = nil case .registry: self.kind = .registry // FIXME: placeholder self.location = self.identity.description - case .providedLibrary(let path): + self.originURL = nil + case .providedLibrary(let url, let path): self.kind = .providedLibrary + self.originURL = url.absoluteString self.location = path.pathString } self.name = reference.deprecatedName @@ -830,7 +850,13 @@ extension PackageModel.PackageReference { case .registry: kind = .registry(identity) case .providedLibrary: - kind = try .providedLibrary(.init(validating: reference.location)) + guard let url = reference.originURL else { + throw InternalError("Cannot form a provided library reference without origin: \(reference)") + } + kind = try .providedLibrary( + SourceControlURL(url), + .init(validating: reference.location) + ) } self.init( diff --git a/Tests/PackageLoadingTests/PDLoadingTests.swift b/Tests/PackageLoadingTests/PDLoadingTests.swift index 08c82442d63..b8b04ef0e91 100644 --- a/Tests/PackageLoadingTests/PDLoadingTests.swift +++ b/Tests/PackageLoadingTests/PDLoadingTests.swift @@ -97,7 +97,7 @@ class PackageDescriptionLoadingTests: XCTestCase, ManifestLoaderDelegate { packagePath = path case .remoteSourceControl, .registry: packagePath = .root - case .providedLibrary(let path): + case .providedLibrary(_, let path): packagePath = path } From b3a3d59888de38fdbef499b33b2431909c379bda Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 16 Apr 2024 13:28:26 -0700 Subject: [PATCH 04/16] Augment ManagedDependency to handle provided libraries --- .../Plugins/PluginContextSerializer.swift | 2 +- Sources/Workspace/ManagedDependency.swift | 20 ++++++++++++++ .../Workspace/Workspace+Dependencies.swift | 24 ++++++++++++++--- Sources/Workspace/Workspace+Editing.swift | 3 +++ Sources/Workspace/Workspace+Manifests.swift | 15 ++++++++--- Sources/Workspace/Workspace+Pinning.swift | 2 +- Sources/Workspace/Workspace+State.swift | 26 +++++++++++++++++++ Sources/Workspace/Workspace.swift | 4 +++ 8 files changed, 88 insertions(+), 8 deletions(-) diff --git a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift index 3816f7791b5..e246f2300ef 100644 --- a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift +++ b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift @@ -240,7 +240,7 @@ internal struct PluginContextSerializer { return .repository(url: url.absoluteString, displayVersion: String(describing: package.manifest.version), scmRevision: String(describing: package.manifest.revision)) case .registry(let identity): return .registry(identity: identity.description, displayVersion: String(describing: package.manifest.version)) - case .providedLibrary(_): + case .providedLibrary(_, _): throw InternalError("provided libraries are not supported in plugin context") } } diff --git a/Sources/Workspace/ManagedDependency.swift b/Sources/Workspace/ManagedDependency.swift index b771f37221c..4f476c99965 100644 --- a/Sources/Workspace/ManagedDependency.swift +++ b/Sources/Workspace/ManagedDependency.swift @@ -34,6 +34,9 @@ extension Workspace { /// The dependency is downloaded from a registry. case registryDownload(version: Version) + /// The dependency is part of the toolchain in a binary form. + case providedLibrary(at: AbsolutePath, version: Version) + /// The dependency is in edited state. /// /// If the path is non-nil, the dependency is managed by a user and is @@ -51,6 +54,8 @@ extension Workspace { return "sourceControlCheckout (\(checkoutState))" case .registryDownload(let version): return "registryDownload (\(version))" + case .providedLibrary(let path, let version): + return "library (\(path) @ \(version)" case .edited: return "edited" case .custom: @@ -146,6 +151,21 @@ extension Workspace { ) } + public static func providedLibrary( + packageRef: PackageReference, + version: Version + ) throws -> ManagedDependency { + guard case .providedLibrary(_, let path) = packageRef.kind else { + throw InternalError("invalid package type: \(packageRef.kind)") + } + + return ManagedDependency( + packageRef: packageRef, + state: .providedLibrary(at: path, version: version), + subpath: try RelativePath(validating: packageRef.identity.description) + ) + } + /// Create an edited dependency public static func edited( packageRef: PackageReference, diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 5ece297dc4a..9609e14ecc1 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -430,7 +430,7 @@ extension Workspace { return !pin.state.equals(checkoutState) case .registryDownload(let version): return !pin.state.equals(version) - case .edited, .fileSystem, .custom: + case .edited, .fileSystem, .providedLibrary, .custom: return true } } @@ -781,6 +781,19 @@ extension Workspace { state: .custom(version: version, path: path), subpath: RelativePath(validating: "") ) + self.state.dependencies.add(dependency) + try self.state.save() + return path + } else if let libraryContainer = container as? ProvidedLibraryPackageContainer { + guard case .providedLibrary(_, let path) = libraryContainer.package.kind else { + throw InternalError("invalid container for \(package.identity) of type \(package.kind)") + } + + let dependency: ManagedDependency = try .providedLibrary( + packageRef: libraryContainer.package, + version: version + ) + self.state.dependencies.add(dependency) try self.state.save() return path @@ -1033,6 +1046,8 @@ extension Workspace { packageStateChanges[binding.package.identity] = (binding.package, .updated(newState)) case .registryDownload: throw InternalError("Unexpected unversioned binding for downloaded dependency") + case .providedLibrary: + throw InternalError("Unexpected unversioned binding for library dependency") case .custom: throw InternalError("Unexpected unversioned binding for custom dependency") } @@ -1103,9 +1118,12 @@ extension Workspace { case .version(let version): let stateChange: PackageStateChange switch currentDependency?.state { - case .sourceControlCheckout(.version(version, _)), .registryDownload(version), .custom(version, _): + case .sourceControlCheckout(.version(version, _)), + .registryDownload(version), + .providedLibrary(_, version: version), + .custom(version, _): stateChange = .unchanged - case .edited, .fileSystem, .sourceControlCheckout, .registryDownload, .custom: + case .edited, .fileSystem, .sourceControlCheckout, .registryDownload, .providedLibrary, .custom: stateChange = .updated(.init(requirement: .version(version), products: binding.products)) case nil: stateChange = .added(.init(requirement: .version(version), products: binding.products)) diff --git a/Sources/Workspace/Workspace+Editing.swift b/Sources/Workspace/Workspace+Editing.swift index c27afe324a6..86f7d085586 100644 --- a/Sources/Workspace/Workspace+Editing.swift +++ b/Sources/Workspace/Workspace+Editing.swift @@ -52,6 +52,9 @@ extension Workspace { case .registryDownload: observabilityScope.emit(error: "registry dependency '\(dependency.packageRef.identity)' can't be edited") return + case .providedLibrary: + observabilityScope.emit(error: "library dependency '\(dependency.packageRef.identity)' can't be edited") + return case .custom: observabilityScope.emit(error: "custom dependency '\(dependency.packageRef.identity)' can't be edited") return diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index e202a624e51..48a229a9c85 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -151,7 +151,7 @@ extension Workspace { result.insert(packageRef) } - case .registryDownload, .edited, .custom: + case .registryDownload, .edited, .providedLibrary, .custom: continue case .fileSystem: result.insert(dependency.packageRef) @@ -343,7 +343,7 @@ extension Workspace { products: productFilter ) allConstraints.append(constraint) - case .sourceControlCheckout, .registryDownload, .fileSystem, .custom: + case .sourceControlCheckout, .registryDownload, .fileSystem, .providedLibrary, .custom: break } allConstraints += try externalManifest.dependencyConstraints(productFilter: productFilter) @@ -358,7 +358,7 @@ extension Workspace { for (_, managedDependency, productFilter, _) in dependencies { switch managedDependency.state { - case .sourceControlCheckout, .registryDownload, .fileSystem, .custom: continue + case .sourceControlCheckout, .registryDownload, .fileSystem, .providedLibrary, .custom: continue case .edited: break } // FIXME: We shouldn't need to construct a new package reference object here. @@ -394,6 +394,8 @@ extension Workspace { return path ?? self.location.editSubdirectory(for: dependency) case .fileSystem(let path): return path + case .providedLibrary(let path, _): + return path case .custom(_, let path): return path } @@ -684,6 +686,9 @@ extension Workspace { case .registryDownload(let downloadedVersion): packageKind = managedDependency.packageRef.kind packageVersion = downloadedVersion + case .providedLibrary(_, let version): + packageKind = managedDependency.packageRef.kind + packageVersion = version case .custom(let availableVersion, _): packageKind = managedDependency.packageRef.kind packageVersion = availableVersion @@ -897,6 +902,10 @@ extension Workspace { observabilityScope .emit(.editedDependencyMissing(packageName: dependency.packageRef.identity.description)) + case .providedLibrary(_, version: _): + // TODO: If the dependency is not available we can turn it into a source control dependency + break + case .fileSystem: self.state.dependencies.remove(dependency.packageRef.identity) try self.state.save() diff --git a/Sources/Workspace/Workspace+Pinning.swift b/Sources/Workspace/Workspace+Pinning.swift index b51e69fb04f..6a652a937ad 100644 --- a/Sources/Workspace/Workspace+Pinning.swift +++ b/Sources/Workspace/Workspace+Pinning.swift @@ -145,7 +145,7 @@ extension PinsStore.Pin { packageRef: dependency.packageRef, state: .version(version, revision: .none) ) - case .edited, .fileSystem, .custom: + case .edited, .fileSystem, .providedLibrary, .custom: // NOOP return nil } diff --git a/Sources/Workspace/Workspace+State.swift b/Sources/Workspace/Workspace+State.swift index bd4db386e2e..8d827cb562f 100644 --- a/Sources/Workspace/Workspace+State.swift +++ b/Sources/Workspace/Workspace+State.swift @@ -256,6 +256,15 @@ extension WorkspaceStateStorage { let version = try container.decode(String.self, forKey: .version) return try self .init(underlying: .registryDownload(version: TSCUtility.Version(versionString: version))) + case "providedLibrary": + let path = try container.decode(AbsolutePath.self, forKey: .path) + let version = try container.decode(String.self, forKey: .version) + return try self.init( + underlying: .providedLibrary( + at: path, + version: TSCUtility.Version(versionString: version) + ) + ) case "edited": let path = try container.decode(AbsolutePath?.self, forKey: .path) return try self.init(underlying: .edited( @@ -286,6 +295,10 @@ extension WorkspaceStateStorage { case .registryDownload(let version): try container.encode("registryDownload", forKey: .name) try container.encode(version, forKey: .version) + case .providedLibrary(let path, let version): + try container.encode("providedLibrary", forKey: .name) + try container.encode(path, forKey: .path) + try container.encode(version, forKey: .version) case .edited(_, let path): try container.encode("edited", forKey: .name) try container.encode(path, forKey: .path) @@ -631,6 +644,15 @@ extension WorkspaceStateStorage { let version = try container.decode(String.self, forKey: .version) return try self .init(underlying: .registryDownload(version: TSCUtility.Version(versionString: version))) + case "providedLibrary": + let path = try container.decode(AbsolutePath.self, forKey: .path) + let version = try container.decode(String.self, forKey: .version) + return try self.init( + underlying: .providedLibrary( + at: path, + version: TSCUtility.Version(versionString: version) + ) + ) case "edited": let path = try container.decode(AbsolutePath?.self, forKey: .path) return try self.init(underlying: .edited( @@ -661,6 +683,10 @@ extension WorkspaceStateStorage { case .registryDownload(let version): try container.encode("registryDownload", forKey: .name) try container.encode(version, forKey: .version) + case .providedLibrary(let path, let version): + try container.encode("providedLibrary", forKey: .name) + try container.encode(path, forKey: .path) + try container.encode(version, forKey: .version) case .edited(_, let path): try container.encode("edited", forKey: .name) try container.encode(path, forKey: .path) diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 0f81611f1fe..5574916885c 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -708,6 +708,8 @@ extension Workspace { defaultRequirement = checkoutState.requirement case .registryDownload(let version), .custom(let version, _): defaultRequirement = .versionSet(.exact(version)) + case .providedLibrary(_, version: let version): + defaultRequirement = .versionSet(.exact(version)) case .fileSystem: throw StringError("local dependency '\(dependency.packageRef.identity)' can't be resolved") case .edited: @@ -1351,6 +1353,8 @@ extension Workspace { } case .registryDownload(let version)?, .custom(let version, _): result.append("resolved to '\(version)'") + case .providedLibrary(_, version: let version): + result.append("resolved to '\(version)'") case .edited?: result.append("edited") case .fileSystem?: From 2f93ea5cefd80bfb5d7b4bb494258f35e0215d27 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 17 Apr 2024 15:13:49 -0700 Subject: [PATCH 05/16] [PackageModel] Add a new target kind - provided library This target points to a prebuilt library that comes from a certain location in a toolchain. It's going to be injected by the package manager into library manifests and shouldn't be accessible via user-facing APIs. --- .../ProductBuildDescription.swift | 5 +++ .../LLBuildManifestBuilder+Swift.swift | 2 + .../Build/BuildPlan/BuildPlan+Product.swift | 8 +++- Sources/Build/BuildPlan/BuildPlan+Swift.swift | 5 +++ Sources/Build/BuildPlan/BuildPlan.swift | 2 +- Sources/Commands/Snippets/Cards/TopCard.swift | 2 + Sources/PackageLoading/PackageBuilder.swift | 15 +++++++ Sources/PackageModel/CMakeLists.txt | 1 + .../Manifest/TargetDescription.swift | 19 +++++++- .../ManifestSourceGeneration.swift | 2 + .../Target/ProvidedLibraryTarget.swift | 44 +++++++++++++++++++ Sources/PackageModel/Target/Target.swift | 2 + Sources/PackageModelSyntax/AddTarget.swift | 4 +- .../TargetDescription+Syntax.swift | 1 + .../Plugins/PluginContextSerializer.swift | 2 +- Sources/XCBuildSupport/PIFBuilder.swift | 3 ++ 16 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 Sources/PackageModel/Target/ProvidedLibraryTarget.swift diff --git a/Sources/Build/BuildDescription/ProductBuildDescription.swift b/Sources/Build/BuildDescription/ProductBuildDescription.swift index 2277a6f638f..289c48dec48 100644 --- a/Sources/Build/BuildDescription/ProductBuildDescription.swift +++ b/Sources/Build/BuildDescription/ProductBuildDescription.swift @@ -45,6 +45,9 @@ package final class ProductBuildDescription: SPMBuildCore.ProductBuildDescriptio // Computed during build planning. var dylibs: [ProductBuildDescription] = [] + /// The list of provided libraries that are going to be used by this product. + var providedLibraries: [String: AbsolutePath] = [:] + /// Any additional flags to be added. These flags are expected to be computed during build planning. var additionalFlags: [String] = [] @@ -156,6 +159,8 @@ package final class ProductBuildDescription: SPMBuildCore.ProductBuildDescriptio args += ["-F", self.buildParameters.buildPath.pathString] } + self.providedLibraries.forEach { args += ["-L", $1.pathString, "-l", $0] } + args += ["-L", self.buildParameters.buildPath.pathString] args += try ["-o", binaryPath.pathString] args += ["-module-name", self.product.name.spm_mangledToC99ExtendedIdentifier()] diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift index 10fa290f50d..e2ea4abc021 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift @@ -424,6 +424,8 @@ extension LLBuildManifestBuilder { if target.underlying is BinaryTarget { return } // Ignore Plugin Targets. if target.underlying is PluginTarget { return } + // Ignore Provided Libraries. + if target.underlying is ProvidedLibraryTarget { return } // Depend on the binary for executable targets. if target.type == .executable { diff --git a/Sources/Build/BuildPlan/BuildPlan+Product.swift b/Sources/Build/BuildPlan/BuildPlan+Product.swift index c3932f0d626..c1731786e1d 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Product.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Product.swift @@ -115,6 +115,8 @@ extension BuildPlan { } buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths + buildProduct.providedLibraries = dependencies.providedLibraries + buildProduct.availableTools = dependencies.availableTools } @@ -127,6 +129,7 @@ extension BuildPlan { staticTargets: [ResolvedModule], systemModules: [ResolvedModule], libraryBinaryPaths: Set, + providedLibraries: [String: AbsolutePath], availableTools: [String: AbsolutePath] ) { /* Prior to tools-version 5.9, we used to erroneously recursively traverse executable/plugin dependencies and statically include their @@ -204,6 +207,7 @@ extension BuildPlan { var staticTargets = [ResolvedModule]() var systemModules = [ResolvedModule]() var libraryBinaryPaths: Set = [] + var providedLibraries = [String: AbsolutePath]() var availableTools = [String: AbsolutePath]() for dependency in allTargets { @@ -257,6 +261,8 @@ extension BuildPlan { } case .plugin: continue + case .providedLibrary: + providedLibraries[target.name] = target.underlying.path } case .product(let product, _): @@ -274,7 +280,7 @@ extension BuildPlan { } } - return (linkLibraries, staticTargets, systemModules, libraryBinaryPaths, availableTools) + return (linkLibraries, staticTargets, systemModules, libraryBinaryPaths, providedLibraries, availableTools) } /// Extracts the artifacts from an artifactsArchive diff --git a/Sources/Build/BuildPlan/BuildPlan+Swift.swift b/Sources/Build/BuildPlan/BuildPlan+Swift.swift index 058631598f2..dadb3c776e8 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Swift.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Swift.swift @@ -14,6 +14,7 @@ import struct Basics.InternalError import class PackageModel.BinaryTarget import class PackageModel.ClangTarget import class PackageModel.SystemLibraryTarget +import class PackageModel.ProvidedLibraryTarget extension BuildPlan { func plan(swiftTarget: SwiftTargetBuildDescription) throws { @@ -48,6 +49,10 @@ extension BuildPlan { swiftTarget.libraryBinaryPaths.insert(library.libraryPath) } } + case let target as ProvidedLibraryTarget: + swiftTarget.additionalFlags += [ + "-I", target.path.pathString + ] default: break } diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 123d34e3e55..7a74eb5cf3f 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -433,7 +433,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { toolsVersion: toolsVersion, fileSystem: fileSystem )) - case is SystemLibraryTarget, is BinaryTarget: + case is SystemLibraryTarget, is BinaryTarget, is ProvidedLibraryTarget: break default: throw InternalError("unhandled \(target.underlying)") diff --git a/Sources/Commands/Snippets/Cards/TopCard.swift b/Sources/Commands/Snippets/Cards/TopCard.swift index 7b915234db0..bb0398fda1e 100644 --- a/Sources/Commands/Snippets/Cards/TopCard.swift +++ b/Sources/Commands/Snippets/Cards/TopCard.swift @@ -153,6 +153,8 @@ fileprivate extension Target.Kind { return "snippets" case .macro: return "macros" + case .providedLibrary: + return "provided libraries" } } } diff --git a/Sources/PackageLoading/PackageBuilder.swift b/Sources/PackageLoading/PackageBuilder.swift index c81ccb1fadd..76d96c1d98e 100644 --- a/Sources/PackageLoading/PackageBuilder.swift +++ b/Sources/PackageLoading/PackageBuilder.swift @@ -536,6 +536,16 @@ public final class PackageBuilder { throw ModuleError.artifactNotFound(targetName: target.name, expectedArtifactName: target.name) } return artifact.path + } else if let targetPath = target.path, target.type == .providedLibrary { + guard let path = try? AbsolutePath(validating: targetPath) else { + throw ModuleError.invalidCustomPath(target: target.name, path: targetPath) + } + + if !self.fileSystem.isDirectory(path) { + throw ModuleError.unsupportedTargetPath(targetPath) + } + + return path } else if let subpath = target.path { // If there is a custom path defined, use that. if subpath == "" || subpath == "." { return self.packagePath @@ -844,6 +854,11 @@ public final class PackageBuilder { path: potentialModule.path, origin: artifactOrigin ) + } else if potentialModule.type == .providedLibrary { + return ProvidedLibraryTarget( + name: potentialModule.name, + path: potentialModule.path + ) } // Check for duplicate target dependencies diff --git a/Sources/PackageModel/CMakeLists.txt b/Sources/PackageModel/CMakeLists.txt index 26a19962746..a7b13798bb4 100644 --- a/Sources/PackageModel/CMakeLists.txt +++ b/Sources/PackageModel/CMakeLists.txt @@ -51,6 +51,7 @@ add_library(PackageModel Target/BinaryTarget.swift Target/ClangTarget.swift Target/PluginTarget.swift + Target/ProvidedLibraryTarget.swift Target/SwiftTarget.swift Target/SystemLibraryTarget.swift Target/Target.swift diff --git a/Sources/PackageModel/Manifest/TargetDescription.swift b/Sources/PackageModel/Manifest/TargetDescription.swift index 57df7f6d37a..1c943a5313c 100644 --- a/Sources/PackageModel/Manifest/TargetDescription.swift +++ b/Sources/PackageModel/Manifest/TargetDescription.swift @@ -21,6 +21,7 @@ public struct TargetDescription: Hashable, Encodable, Sendable { case binary case plugin case `macro` + case providedLibrary } /// Represents a target's dependency on another entity. @@ -222,6 +223,19 @@ public struct TargetDescription: Hashable, Encodable, Sendable { if pkgConfig != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pkgConfig") } if providers != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "providers") } if pluginCapability != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginCapability") } + case .providedLibrary: + if path == nil { throw Error.providedLibraryTargetRequiresPath(targetName: name) } + if url != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "url") } + if !dependencies.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "dependencies") } + if !exclude.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "exclude") } + if sources != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "sources") } + if !resources.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "resources") } + if publicHeadersPath != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "publicHeadersPath") } + if pkgConfig != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pkgConfig") } + if providers != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "providers") } + if pluginCapability != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginCapability") } + if !settings.isEmpty { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "settings") } + if pluginUsages != nil { throw Error.disallowedPropertyInTarget(targetName: name, propertyName: "pluginUsages") } } self.name = name @@ -370,13 +384,16 @@ import protocol Foundation.LocalizedError private enum Error: LocalizedError, Equatable { case binaryTargetRequiresEitherPathOrURL(targetName: String) case disallowedPropertyInTarget(targetName: String, propertyName: String) - + case providedLibraryTargetRequiresPath(targetName: String) + var errorDescription: String? { switch self { case .binaryTargetRequiresEitherPathOrURL(let targetName): return "binary target '\(targetName)' neither defines neither path nor URL for its artifacts" case .disallowedPropertyInTarget(let targetName, let propertyName): return "target '\(targetName)' contains a value for disallowed property '\(propertyName)'" + case .providedLibraryTargetRequiresPath(let targetName): + return "provided library target '\(targetName)' does not define a path to the library" } } } diff --git a/Sources/PackageModel/ManifestSourceGeneration.swift b/Sources/PackageModel/ManifestSourceGeneration.swift index f2b0d65c45c..211c44032ee 100644 --- a/Sources/PackageModel/ManifestSourceGeneration.swift +++ b/Sources/PackageModel/ManifestSourceGeneration.swift @@ -317,6 +317,8 @@ fileprivate extension SourceCodeFragment { self.init(enum: "plugin", subnodes: params, multiline: true) case .macro: self.init(enum: "macro", subnodes: params, multiline: true) + case .providedLibrary: + self.init(enum: "providedLibrary", subnodes: params, multiline: true) } } diff --git a/Sources/PackageModel/Target/ProvidedLibraryTarget.swift b/Sources/PackageModel/Target/ProvidedLibraryTarget.swift new file mode 100644 index 00000000000..a8ed95b23ae --- /dev/null +++ b/Sources/PackageModel/Target/ProvidedLibraryTarget.swift @@ -0,0 +1,44 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import struct Basics.AbsolutePath + +/// Represents a target library that comes from a toolchain in prebuilt form. +public final class ProvidedLibraryTarget: Target { + public init( + name: String, + path: AbsolutePath + ) { + let sources = Sources(paths: [], root: path) + super.init( + name: name, + type: .providedLibrary, + path: sources.root, + sources: sources, + dependencies: [], + packageAccess: false, + buildSettings: .init(), + buildSettingsDescription: [], + pluginUsages: [], + usesUnsafeFlags: false + ) + } + + + public override func encode(to encoder: Encoder) throws { + try super.encode(to: encoder) + } + + required public init(from decoder: Decoder) throws { + try super.init(from: decoder) + } +} diff --git a/Sources/PackageModel/Target/Target.swift b/Sources/PackageModel/Target/Target.swift index 40bb1a575e9..1074e353c87 100644 --- a/Sources/PackageModel/Target/Target.swift +++ b/Sources/PackageModel/Target/Target.swift @@ -21,6 +21,7 @@ public class Target: PolymorphicCodableProtocol { SystemLibraryTarget.self, BinaryTarget.self, PluginTarget.self, + ProvidedLibraryTarget.self, ] /// The target kind. @@ -33,6 +34,7 @@ public class Target: PolymorphicCodableProtocol { case plugin case snippet case `macro` + case providedLibrary } /// A group a target belongs to that allows customizing access boundaries. A target is treated as diff --git a/Sources/PackageModelSyntax/AddTarget.swift b/Sources/PackageModelSyntax/AddTarget.swift index 0ae83b78e0c..7fda6f58229 100644 --- a/Sources/PackageModelSyntax/AddTarget.swift +++ b/Sources/PackageModelSyntax/AddTarget.swift @@ -62,7 +62,7 @@ public struct AddTarget { ) let outerDirectory: String? = switch target.type { - case .binary, .plugin, .system: nil + case .binary, .plugin, .system, .providedLibrary: nil case .executable, .regular, .macro: "Sources" case .test: "Tests" } @@ -167,7 +167,7 @@ public struct AddTarget { } let sourceFileText: SourceFileSyntax = switch target.type { - case .binary, .plugin, .system: + case .binary, .plugin, .system, .providedLibrary: fatalError("should have exited above") case .macro: diff --git a/Sources/PackageModelSyntax/TargetDescription+Syntax.swift b/Sources/PackageModelSyntax/TargetDescription+Syntax.swift index f47f6590f06..5081932bed8 100644 --- a/Sources/PackageModelSyntax/TargetDescription+Syntax.swift +++ b/Sources/PackageModelSyntax/TargetDescription+Syntax.swift @@ -27,6 +27,7 @@ extension TargetDescription: ManifestSyntaxRepresentable { case .regular: "target" case .system: "systemLibrary" case .test: "testTarget" + case .providedLibrary: "providedLibrary" } } diff --git a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift index e246f2300ef..9f84bbf9598 100644 --- a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift +++ b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift @@ -282,7 +282,7 @@ fileprivate extension WireInput.Target.TargetInfo.SourceModuleKind { self = .test case .macro: self = .macro - case .binary, .plugin, .systemModule: + case .binary, .plugin, .systemModule, .providedLibrary: throw StringError("unexpected target kind \(kind) for source module") } } diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index 3d77fd3b95d..aa2cfc994df 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -391,6 +391,9 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { case .macro: // Macros are not supported when using XCBuild, similar to package plugins. return + case .providedLibrary: + // Provided libraries don't need to be built. + return } } From 52b7c6e39e2729ed8de4f695d928546199429ab0 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 18 Apr 2024 11:34:55 -0700 Subject: [PATCH 06/16] Package LibraryMetadata together with its toolchain location Location is required down the line for build planning to inject proper header and library includes into the build. The location is currently assumed to be in `share/pm/` --- Sources/Build/BuildOperation.swift | 6 +-- .../PubGrub/ContainerProvider.swift | 2 +- .../PubGrub/PubGrubDependencyResolver.swift | 10 ++-- .../LibraryMetadata.swift | 11 ++++- Sources/PackageModel/Toolchain.swift | 2 +- Sources/PackageModel/UserToolchain.swift | 14 ++++-- .../SPMTestSupport/MockBuildTestHelper.swift | 2 +- .../Workspace/Workspace+Dependencies.swift | 16 +++---- Sources/Workspace/Workspace+Editing.swift | 4 +- Sources/Workspace/Workspace+Manifests.swift | 14 +++--- Sources/Workspace/Workspace.swift | 6 +-- Tests/BuildTests/BuildOperationTests.swift | 15 +++--- Tests/PackageGraphTests/PubgrubTests.swift | 47 +++++++++++-------- 13 files changed, 89 insertions(+), 60 deletions(-) diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index 9255948a749..6055c2c708d 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -269,7 +269,7 @@ package final class BuildOperation: PackageStructureDelegate, SPMBuildCore.Build // TODO: Currently this function will only match frameworks. func detectUnexpressedDependencies( - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], targetDependencyMap: [String: [String]]? ) { // Ensure we only emit these once, regardless of how many builds are being done. @@ -279,8 +279,8 @@ package final class BuildOperation: PackageStructureDelegate, SPMBuildCore.Build Self.didEmitUnexpressedDependencies = true let availableFrameworks = Dictionary(uniqueKeysWithValues: availableLibraries.compactMap { - if let identity = Set($0.identities.map(\.identity)).spm_only { - return ("\($0.productName!).framework", identity) + if let identity = Set($0.metadata.identities.map(\.identity)).spm_only { + return ("\($0.metadata.productName).framework", identity) } else { return nil } diff --git a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift index d73e41ffdf9..46cd49a9fbf 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift @@ -107,7 +107,7 @@ final class ContainerProvider { } /// Starts prefetching the given containers. - func prefetch(containers identifiers: [PackageReference], availableLibraries: [LibraryMetadata]) { + func prefetch(containers identifiers: [PackageReference], availableLibraries: [ProvidedLibrary]) { let filteredIdentifiers = identifiers.filter { $0.matchingPrebuiltLibrary(in: availableLibraries) == nil } diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index 553b354790a..d6f5ddf3983 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -105,7 +105,7 @@ public struct PubGrubDependencyResolver { private let pins: PinsStore.Pins /// The packages that are available in a prebuilt form in SDK or a toolchain - private let availableLibraries: [LibraryMetadata] + private let availableLibraries: [ProvidedLibrary] /// The container provider used to load package containers. private let provider: ContainerProvider @@ -125,7 +125,7 @@ public struct PubGrubDependencyResolver { public init( provider: PackageContainerProvider, pins: PinsStore.Pins = [:], - availableLibraries: [LibraryMetadata] = [], + availableLibraries: [ProvidedLibrary] = [], skipDependenciesUpdates: Bool = false, prefetchBasedOnResolvedFile: Bool = false, observabilityScope: ObservabilityScope, @@ -895,14 +895,14 @@ extension PackageRequirement { } extension PackageReference { - public func matchingPrebuiltLibrary(in availableLibraries: [LibraryMetadata]) -> LibraryMetadata? { + public func matchingPrebuiltLibrary(in availableLibraries: [ProvidedLibrary]) -> ProvidedLibrary? { switch self.kind { case .fileSystem, .localSourceControl, .root, .providedLibrary: return nil // can never match a prebuilt library case .registry(let identity): if let registryIdentity = identity.registry { return availableLibraries.first( - where: { $0.identities.contains( + where: { $0.metadata.identities.contains( where: { $0 == .packageIdentity( scope: registryIdentity.scope.description, name: registryIdentity.name.description @@ -916,7 +916,7 @@ extension PackageReference { } case .remoteSourceControl(let url): return availableLibraries.first(where: { - $0.identities.contains(where: { $0 == .sourceControl(url: url) }) + $0.metadata.identities.contains(where: { $0 == .sourceControl(url: url) }) }) } } diff --git a/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift b/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift index 7e12d31a6f3..362dae238cf 100644 --- a/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift +++ b/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift @@ -13,6 +13,15 @@ import Basics import Foundation +public struct ProvidedLibrary { + public let location: AbsolutePath + public let metadata: LibraryMetadata + + public var version: String { + metadata.version + } +} + public struct LibraryMetadata: Decodable { public enum Identity: Equatable, Decodable { case packageIdentity(scope: String, name: String) @@ -24,7 +33,7 @@ public struct LibraryMetadata: Decodable { /// The version that was built (e.g., 509.0.2) public let version: String /// The product name, if it differs from the module name (e.g., SwiftParser). - public let productName: String? + public let productName: String let schemaVersion: Int } diff --git a/Sources/PackageModel/Toolchain.swift b/Sources/PackageModel/Toolchain.swift index cd7db9c68ba..8009f62d326 100644 --- a/Sources/PackageModel/Toolchain.swift +++ b/Sources/PackageModel/Toolchain.swift @@ -44,7 +44,7 @@ public protocol Toolchain { var installedSwiftPMConfiguration: InstalledSwiftPMConfiguration { get } /// Metadata for libraries provided by the used toolchain. - var providedLibraries: [LibraryMetadata] { get } + var providedLibraries: [ProvidedLibrary] { get } /// The root path to the Swift SDK used by this toolchain. var sdkRootPath: AbsolutePath? { get } diff --git a/Sources/PackageModel/UserToolchain.swift b/Sources/PackageModel/UserToolchain.swift index 9b7042332bb..7d1f833e903 100644 --- a/Sources/PackageModel/UserToolchain.swift +++ b/Sources/PackageModel/UserToolchain.swift @@ -88,7 +88,7 @@ public final class UserToolchain: Toolchain { public let installedSwiftPMConfiguration: InstalledSwiftPMConfiguration - public let providedLibraries: [LibraryMetadata] + public let providedLibraries: [ProvidedLibrary] /// Returns the runtime library for the given sanitizer. public func runtimeLibrary(for sanitizer: Sanitizer) throws -> AbsolutePath { @@ -487,7 +487,7 @@ public final class UserToolchain: Toolchain { searchStrategy: SearchStrategy = .default, customLibrariesLocation: ToolchainConfiguration.SwiftPMLibrariesLocation? = nil, customInstalledSwiftPMConfiguration: InstalledSwiftPMConfiguration? = nil, - customProvidedLibraries: [LibraryMetadata]? = nil + customProvidedLibraries: [ProvidedLibrary]? = nil ) throws { self.swiftSDK = swiftSDK self.environment = environment @@ -551,7 +551,15 @@ public final class UserToolchain: Toolchain { self.providedLibraries = try Self.loadJSONResource( config: path, type: [LibraryMetadata].self, - default: []) + default: [] + ).map { + .init( + location: path.parentDirectory.appending(component: $0.productName), + metadata: $0 + ) + }.filter { + localFileSystem.isDirectory($0.location) + } } // Use the triple from Swift SDK or compute the host triple using swiftc. diff --git a/Sources/SPMTestSupport/MockBuildTestHelper.swift b/Sources/SPMTestSupport/MockBuildTestHelper.swift index 1b8824b928c..9ce3ac5fa67 100644 --- a/Sources/SPMTestSupport/MockBuildTestHelper.swift +++ b/Sources/SPMTestSupport/MockBuildTestHelper.swift @@ -39,7 +39,7 @@ package struct MockToolchain: PackageModel.Toolchain { package let swiftPluginServerPath: AbsolutePath? = nil package let extraFlags = PackageModel.BuildFlags() package let installedSwiftPMConfiguration = InstalledSwiftPMConfiguration.default - package let providedLibraries = [LibraryMetadata]() + package let providedLibraries = [ProvidedLibrary]() package func getClangCompiler() throws -> AbsolutePath { "/fake/path/to/clang" diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 9609e14ecc1..3f6a917cf07 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -37,7 +37,7 @@ import class PackageGraph.PinsStore import struct PackageGraph.PubGrubDependencyResolver import struct PackageGraph.Term import class PackageLoading.ManifestLoader -import struct PackageModel.LibraryMetadata +import struct PackageModel.ProvidedLibrary import enum PackageModel.PackageDependency import struct PackageModel.PackageIdentity import struct PackageModel.PackageReference @@ -57,7 +57,7 @@ extension Workspace { root: PackageGraphRootInput, packages: [String] = [], dryRun: Bool = false, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> [(PackageReference, Workspace.PackageStateChange)]? { let start = DispatchTime.now() @@ -200,7 +200,7 @@ extension Workspace { func _resolve( root: PackageGraphRootInput, explicitProduct: String?, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], resolvedFileStrategy: ResolvedFileStrategy, observabilityScope: ObservabilityScope ) throws -> DependencyManifests { @@ -304,7 +304,7 @@ extension Workspace { func _resolveBasedOnResolvedVersionsFile( root: PackageGraphRootInput, explicitProduct: String?, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> DependencyManifests { let (manifests, precomputationResult) = try self.tryResolveBasedOnResolvedVersionsFile( @@ -343,7 +343,7 @@ extension Workspace { fileprivate func tryResolveBasedOnResolvedVersionsFile( root: PackageGraphRootInput, explicitProduct: String?, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> (DependencyManifests, ResolutionPrecomputationResult) { // Ensure the cache path exists. @@ -495,7 +495,7 @@ extension Workspace { func resolveAndUpdateResolvedFile( root: PackageGraphRootInput, explicitProduct: String? = nil, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], forceResolution: Bool, constraints: [PackageContainerConstraint], observabilityScope: ObservabilityScope @@ -849,7 +849,7 @@ extension Workspace { dependencyManifests: DependencyManifests, pinsStore: PinsStore, constraints: [PackageContainerConstraint], - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> ResolutionPrecomputationResult { let computedConstraints = @@ -1144,7 +1144,7 @@ extension Workspace { /// Creates resolver for the workspace. fileprivate func createResolver( pins: PinsStore.Pins, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> PubGrubDependencyResolver { var delegate: DependencyResolverDelegate diff --git a/Sources/Workspace/Workspace+Editing.swift b/Sources/Workspace/Workspace+Editing.swift index 86f7d085586..03939149bed 100644 --- a/Sources/Workspace/Workspace+Editing.swift +++ b/Sources/Workspace/Workspace+Editing.swift @@ -15,7 +15,7 @@ import class Basics.ObservabilityScope import struct Basics.RelativePath import func Basics.temp_await import struct PackageGraph.PackageGraphRootInput -import struct PackageModel.LibraryMetadata +import struct PackageModel.ProvidedLibrary import struct SourceControl.Revision import class TSCBasic.InMemoryFileSystem @@ -173,7 +173,7 @@ extension Workspace { dependency: ManagedDependency, forceRemove: Bool, root: PackageGraphRootInput? = nil, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws { // Compute if we need to force remove. diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 48a229a9c85..e388d5d9feb 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -28,7 +28,7 @@ import struct PackageGraph.PackageGraphRoot import class PackageLoading.ManifestLoader import struct PackageLoading.ManifestValidator import struct PackageLoading.ToolsVersionParser -import struct PackageModel.LibraryMetadata +import struct PackageModel.ProvidedLibrary import class PackageModel.Manifest import struct PackageModel.PackageIdentity import struct PackageModel.PackageReference @@ -61,7 +61,7 @@ extension Workspace { private let workspace: Workspace - private let availableLibraries: [LibraryMetadata] + private let availableLibraries: [ProvidedLibrary] private let observabilityScope: ObservabilityScope @@ -81,7 +81,7 @@ extension Workspace { fileSystem: FileSystem )], workspace: Workspace, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) { self.root = root @@ -173,7 +173,7 @@ extension Workspace { fileSystem: FileSystem )], workspace: Workspace, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> ( @@ -205,7 +205,7 @@ extension Workspace { }) let identitiesAvailableInSDK = availableLibraries.flatMap { - $0.identities.map { + $0.metadata.identities.map { $0.ref }.filter { // We "trust the process" here, if an identity from the SDK is available, filter it. @@ -438,7 +438,7 @@ extension Workspace { public func loadDependencyManifests( root: PackageGraphRoot, automaticallyAddManagedDependencies: Bool = false, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> DependencyManifests { let prepopulateManagedDependencies: ([PackageReference]) throws -> Void = { refs in @@ -821,7 +821,7 @@ extension Workspace { /// If some edited dependency is removed from the file system, mark it as unedited and /// fallback on the original checkout. private func fixManagedDependencies( - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) { // Reset managed dependencies if the state file was removed during the lifetime of the Workspace object. diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 5574916885c..4e81b3d7e81 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -572,7 +572,7 @@ public class Workspace { ) } - fileprivate var providedLibraries: [LibraryMetadata] { + fileprivate var providedLibraries: [ProvidedLibrary] { // Note: Eventually, we should get these from the individual SDKs, but the first step is providing the metadata centrally in the toolchain. return self.hostToolchain.providedLibraries } @@ -626,7 +626,7 @@ extension Workspace { packageName: String, forceRemove: Bool, root: PackageGraphRootInput, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws { guard let dependency = self.state.dependencies[.plain(packageName)] else { @@ -879,7 +879,7 @@ extension Workspace { forceResolvedVersions: Bool = false, customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none, testEntryPointPath: AbsolutePath? = nil, - availableLibraries: [LibraryMetadata], + availableLibraries: [ProvidedLibrary], expectedSigningEntities: [PackageIdentity: RegistryReleaseMetadata.SigningEntity] = [:], observabilityScope: ObservabilityScope ) throws -> ModulesGraph { diff --git a/Tests/BuildTests/BuildOperationTests.swift b/Tests/BuildTests/BuildOperationTests.swift index 21c40cafb5d..35e72c63462 100644 --- a/Tests/BuildTests/BuildOperationTests.swift +++ b/Tests/BuildTests/BuildOperationTests.swift @@ -52,12 +52,15 @@ final class BuildOperationTests: XCTestCase { buildOp.detectUnexpressedDependencies( availableLibraries: [ .init( - identities: [ - .sourceControl(url: .init("https://example.com/org/foo")) - ], - version: "1.0.0", - productName: "Best", - schemaVersion: 1 + location: "/foo", + metadata: .init( + identities: [ + .sourceControl(url: .init("https://example.com/org/foo")) + ], + version: "1.0.0", + productName: "Best", + schemaVersion: 1 + ) ) ], targetDependencyMap: ["Lunch": []] diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift index 7684413fa48..782cb476664 100644 --- a/Tests/PackageGraphTests/PubgrubTests.swift +++ b/Tests/PackageGraphTests/PubgrubTests.swift @@ -2023,13 +2023,16 @@ final class PubGrubTestsBasicGraphs: XCTestCase { try builder.serve(fooRef, at: .version(.init(stringLiteral: "1.2.0"))) try builder.serve(fooRef, at: .version(.init(stringLiteral: "2.0.0"))) - let availableLibraries: [LibraryMetadata] = [ + let availableLibraries: [ProvidedLibrary] = [ .init( - identities: [.sourceControl(url: "https://example.com/org/foo")], - version: "1.0.0", - productName: nil, - schemaVersion: 1 - ), + location: .init("/foo"), + metadata: .init( + identities: [.sourceControl(url: "https://example.com/org/foo")], + version: "1.0.0", + productName: "foo", + schemaVersion: 1 + ) + ) ] let resolver = builder.create(availableLibraries: availableLibraries) @@ -2080,13 +2083,16 @@ final class PubGrubTestsBasicGraphs: XCTestCase { try builder.serve("target", at: "1.0.0") try builder.serve("target", at: "2.0.0") - let availableLibraries: [LibraryMetadata] = [ + let availableLibraries: [ProvidedLibrary] = [ .init( - identities: [.sourceControl(url: "https://example.com/org/foo")], - version: "1.1.0", - productName: nil, - schemaVersion: 1 - ), + location: .init("/foo"), + metadata: .init( + identities: [.sourceControl(url: "https://example.com/org/foo")], + version: "1.1.0", + productName: "foo", + schemaVersion: 1 + ) + ) ] let resolver = builder.create(availableLibraries: availableLibraries) @@ -2126,13 +2132,16 @@ final class PubGrubTestsBasicGraphs: XCTestCase { "bar": [fooRef: (.versionSet(.range(.upToNextMinor(from: "2.0.0"))), .everything)], ]) - let availableLibraries: [LibraryMetadata] = [ + let availableLibraries: [ProvidedLibrary] = [ .init( - identities: [.sourceControl(url: "https://example.com/org/foo")], - version: "1.0.0", - productName: nil, - schemaVersion: 1 - ), + location: .init("/foo"), + metadata: .init( + identities: [.sourceControl(url: "https://example.com/org/foo")], + version: "1.0.0", + productName: "foo", + schemaVersion: 1 + ) + ) ] let resolver = builder.create(availableLibraries: availableLibraries) @@ -3704,7 +3713,7 @@ class DependencyGraphBuilder { func create( pins: PinsStore.Pins = [:], - availableLibraries: [LibraryMetadata] = [], + availableLibraries: [ProvidedLibrary] = [], delegate: DependencyResolverDelegate? = .none ) -> PubGrubDependencyResolver { defer { From bff25274fc92537aca6d46b23973b6d014fdc665 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 18 Apr 2024 11:45:39 -0700 Subject: [PATCH 07/16] [Resolution/Loading] Handle packages backed by a provided library Instead of eliding provided library packages, start returning them from resolution as a special `.providedLibrary` reference and augment manifest loader to build manifests for such libraries based on contents of the library's directory (each `.swiftmodule` becomes a `.providedLibrary` target). --- .../PubGrub/PubGrubDependencyResolver.swift | 18 +++--- .../ManifestLoader+Validation.swift | 5 ++ Sources/PackageLoading/ManifestLoader.swift | 60 +++++++++++++++++++ 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index d6f5ddf3983..00eead9a16f 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -231,7 +231,7 @@ public struct PubGrubDependencyResolver { continue } - let package = assignment.term.node.package + var package = assignment.term.node.package let boundVersion: BoundVersion switch assignment.term.requirement { @@ -241,16 +241,18 @@ public struct PubGrubDependencyResolver { throw InternalError("unexpected requirement value for assignment \(assignment.term)") } - // Strip packages that have prebuilt libraries only if they match library version. - // - // FIXME: This is built on assumption that libraries are part of the SDK and are - // always available in include/library paths, but what happens if they are - // part of a toolchain instead? Builder needs an indicator that certain path - // has to be included when building packages that depend on prebuilt libraries. if let library = package.matchingPrebuiltLibrary(in: availableLibraries), boundVersion == .version(.init(stringLiteral: library.version)) { - continue + guard case .remoteSourceControl(let url) = package.kind else { + throw InternalError("Matched provided library against invalid package: \(package)") + } + + package = .providedLibrary( + identity: package.identity, + origin: url, + path: library.location + ) } let products = assignment.term.node.productFilter diff --git a/Sources/PackageLoading/ManifestLoader+Validation.swift b/Sources/PackageLoading/ManifestLoader+Validation.swift index af215dd5298..768286b05ef 100644 --- a/Sources/PackageLoading/ManifestLoader+Validation.swift +++ b/Sources/PackageLoading/ManifestLoader+Validation.swift @@ -34,6 +34,11 @@ public struct ManifestValidator { /// Validate the provided manifest. public func validate() -> [Basics.Diagnostic] { + // Provided library manifest is synthesized by the package manager. + if case .providedLibrary = self.manifest.packageKind { + return [] + } + var diagnostics = [Basics.Diagnostic]() diagnostics += self.validateTargets() diff --git a/Sources/PackageLoading/ManifestLoader.swift b/Sources/PackageLoading/ManifestLoader.swift index 9424af7e617..bf8f55fda45 100644 --- a/Sources/PackageLoading/ManifestLoader.swift +++ b/Sources/PackageLoading/ManifestLoader.swift @@ -206,6 +206,19 @@ extension ManifestLoaderProtocol { completion: @escaping (Result) -> Void ) { do { + if case .providedLibrary = packageKind { + let manifest = try self.loadLibrary( + fileSystem: fileSystem, + packagePath: packagePath, + packageKind: packageKind, + packageIdentity: packageIdentity, + packageLocation: packageLocation, + packageVersion: packageVersion?.version + ) + completion(.success(manifest)) + return + } + // find the manifest path and parse it's tools-version let manifestPath = try ManifestLoader.findManifest(packagePath: packagePath, fileSystem: fileSystem, currentToolsVersion: currentToolsVersion) let manifestToolsVersion = try ToolsVersionParser.parse(manifestPath: manifestPath, fileSystem: fileSystem) @@ -266,6 +279,53 @@ extension ManifestLoaderProtocol { ) } } + + private func loadLibrary( + fileSystem: FileSystem, + packagePath: AbsolutePath, + packageKind: PackageReference.Kind, + packageIdentity: PackageIdentity, + packageLocation: String, + packageVersion: Version? + ) throws -> Manifest { + let names = try fileSystem.getDirectoryContents(packagePath).filter { + $0.hasSuffix("swiftmodule") + }.map { + let components = $0.split(separator: ".") + return String(components[0]) + } + + let products: [ProductDescription] = try names.map { + try .init(name: $0, type: .library(.automatic), targets: [$0]) + } + + let targets: [TargetDescription] = try names.map { + try .init( + name: $0, + path: packagePath.pathString, + type: .providedLibrary + ) + } + + return .init( + displayName: packageIdentity.description, + path: packagePath.appending(component: "provided-libraries.json"), + packageKind: packageKind, + packageLocation: packageLocation, + defaultLocalization: nil, + platforms: [], + version: packageVersion, + revision: nil, + toolsVersion: .v6_0, + pkgConfig: nil, + providers: nil, + cLanguageStandard: nil, + cxxLanguageStandard: nil, + swiftLanguageVersions: nil, + products: products, + targets: targets + ) + } } // MARK: - ManifestLoader From f7fef71dc5b7b58173bb61184f4a22aa45d8113f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 19 Apr 2024 13:04:37 -0700 Subject: [PATCH 08/16] [PubGrub] Expand provided library resolution tests Since provided libraries are now returned from resultion we need to double-check if resultion returns proper package reference kinds for packages with provided libraries. --- Sources/PackageGraph/CMakeLists.txt | 1 + .../ProvidedLibraryPackageContainer.swift | 3 +- .../PubGrub/ContainerProvider.swift | 8 ++ Sources/Workspace/CMakeLists.txt | 1 - .../Workspace/Workspace+Dependencies.swift | 1 + .../Workspace+PackageContainer.swift | 1 + Tests/PackageGraphTests/PubgrubTests.swift | 91 ++++++++++++++++--- 7 files changed, 92 insertions(+), 14 deletions(-) rename Sources/{Workspace/PackageContainer => PackageGraph}/ProvidedLibraryPackageContainer.swift (94%) diff --git a/Sources/PackageGraph/CMakeLists.txt b/Sources/PackageGraph/CMakeLists.txt index b95b1eb9feb..13d010b11a7 100644 --- a/Sources/PackageGraph/CMakeLists.txt +++ b/Sources/PackageGraph/CMakeLists.txt @@ -20,6 +20,7 @@ add_library(PackageGraph PackageModel+Extensions.swift PackageRequirement.swift PinsStore.swift + ProvidedLibraryPackageContainer.swift Resolution/PubGrub/Assignment.swift Resolution/PubGrub/ContainerProvider.swift Resolution/PubGrub/DiagnosticReportBuilder.swift diff --git a/Sources/Workspace/PackageContainer/ProvidedLibraryPackageContainer.swift b/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift similarity index 94% rename from Sources/Workspace/PackageContainer/ProvidedLibraryPackageContainer.swift rename to Sources/PackageGraph/ProvidedLibraryPackageContainer.swift index f423dcf454a..8f95ca0f7ad 100644 --- a/Sources/Workspace/PackageContainer/ProvidedLibraryPackageContainer.swift +++ b/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift @@ -12,12 +12,13 @@ import Basics import Dispatch -import PackageGraph import PackageLoading import PackageModel import struct TSCUtility.Version +/// TODO: This could be removed once logic to handle provided libraries is integrated +/// into a \c PubGrubPackageContainer. public struct ProvidedLibraryPackageContainer: PackageContainer { public let package: PackageReference diff --git a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift index 46cd49a9fbf..95e43367f6e 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift @@ -97,6 +97,14 @@ final class ContainerProvider { ) { result in let result = result.tryMap { container -> PubGrubPackageContainer in let pubGrubContainer = PubGrubPackageContainer(underlying: container, pins: self.pins) + + // This container is not cached because it's intended to be transparent + // and requested only when forming final assignments. Caching it would + // mean that subsequent calls to `solve` would pick it up. + if case .providedLibrary = package.kind { + return pubGrubContainer + } + // only cache positive results self.containersCache[package] = pubGrubContainer return pubGrubContainer diff --git a/Sources/Workspace/CMakeLists.txt b/Sources/Workspace/CMakeLists.txt index 447c3353684..7817b6b4a43 100644 --- a/Sources/Workspace/CMakeLists.txt +++ b/Sources/Workspace/CMakeLists.txt @@ -15,7 +15,6 @@ add_library(Workspace ManagedArtifact.swift ManagedDependency.swift PackageContainer/FileSystemPackageContainer.swift - PackageContainer/ProvidedLibraryPackageContainer.swift PackageContainer/RegistryPackageContainer.swift PackageContainer/SourceControlPackageContainer.swift ResolvedFileWatcher.swift diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 3f6a917cf07..6b9bd9897a3 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -33,6 +33,7 @@ import struct PackageGraph.ObservabilityDependencyResolverDelegate import struct PackageGraph.PackageContainerConstraint import struct PackageGraph.PackageGraphRoot import struct PackageGraph.PackageGraphRootInput +import struct PackageGraph.ProvidedLibraryPackageContainer import class PackageGraph.PinsStore import struct PackageGraph.PubGrubDependencyResolver import struct PackageGraph.Term diff --git a/Sources/Workspace/Workspace+PackageContainer.swift b/Sources/Workspace/Workspace+PackageContainer.swift index 4ba1016756a..ca5e857b343 100644 --- a/Sources/Workspace/Workspace+PackageContainer.swift +++ b/Sources/Workspace/Workspace+PackageContainer.swift @@ -17,6 +17,7 @@ import enum PackageFingerprint.FingerprintCheckingMode import enum PackageGraph.ContainerUpdateStrategy import protocol PackageGraph.PackageContainer import protocol PackageGraph.PackageContainerProvider +import struct PackageGraph.ProvidedLibraryPackageContainer import struct PackageModel.PackageReference // MARK: - Package container provider diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift index 782cb476664..8920d14211e 100644 --- a/Tests/PackageGraphTests/PubgrubTests.swift +++ b/Tests/PackageGraphTests/PubgrubTests.swift @@ -2044,12 +2044,18 @@ final class PubGrubTestsBasicGraphs: XCTestCase { ]) let result = resolver.solve(constraints: dependencies1) - // Available libraries are filtered from the resolver results, so this is expected to be empty. - AssertResult(result, []) + print(try result.get()) + AssertResult(result, [ + ( + "foo", + .providedLibrary(.init("https://example.com/org/foo"), .init("/foo")), + .version(.init(stringLiteral: "1.0.0")) + ), + ]) let result2 = resolver.solve(constraints: dependencies2) AssertResult(result2, [ - ("foo", .version(.init(stringLiteral: "1.2.0"))), + ("foo", fooRef.kind, .version(.init(stringLiteral: "1.2.0"))), ]) } @@ -2101,14 +2107,23 @@ final class PubGrubTestsBasicGraphs: XCTestCase { "target": (.versionSet(.range(.upToNextMajor(from: "2.0.0"))), .everything), ]) - // This behavior requires an explanation - "foo" is elided because 1.1.0 is prebuilt. - // It matches "root" requirements but without prebuilt library the solver would pick - // "1.0.0" because "foo" 1.1.0 dependency version requirements are incompatible with - // "target" 2.0.0. + // This behavior requires an explanation - "foo" is selected to be 1.1.0 because its + // prebuilt matches "root" requirements but without prebuilt library the solver would + // pick "1.0.0" because "foo" 1.1.0 dependency version requirements are incompatible + // with "target" 2.0.0. let result = resolver.solve(constraints: dependencies) AssertResult(result, [ - ("target", .version(.init(stringLiteral: "2.0.0"))), + ( + "foo", + .providedLibrary(.init("https://example.com/org/foo"), .init("/foo")), + .version(.init(stringLiteral: "1.1.0")) + ), + ( + "target", + .localSourceControl("/target"), + .version(.init(stringLiteral: "2.0.0")) + ), ]) } @@ -2152,8 +2167,8 @@ final class PubGrubTestsBasicGraphs: XCTestCase { let result = resolver.solve(constraints: dependencies) AssertResult(result, [ - ("foo", .version(.init(stringLiteral: "1.1.0"))), - ("bar", .version(.init(stringLiteral: "1.0.0"))), + ("foo", fooRef.kind, .version(.init(stringLiteral: "1.1.0"))), + ("bar", .localSourceControl("/bar"), .version(.init(stringLiteral: "1.0.0"))), ]) } } @@ -3297,6 +3312,22 @@ private func AssertBindings( _ packages: [(identity: PackageIdentity, version: BoundVersion)], file: StaticString = #file, line: UInt = #line +) { + AssertBindings( + bindings, + packages.map { + (identity: $0, kind: nil, version: $1) + }, + file: file, + line: line + ) +} + +private func AssertBindings( + _ bindings: [DependencyResolverBinding], + _ packages: [(identity: PackageIdentity, kind: PackageReference.Kind?, version: BoundVersion)], + file: StaticString = #file, + line: UInt = #line ) { if bindings.count > packages.count { let unexpectedBindings = bindings @@ -3314,7 +3345,17 @@ private func AssertBindings( ) } for package in packages { - guard let binding = bindings.first(where: { $0.package.identity == package.identity }) else { + guard let binding = bindings.first(where: { + if $0.package.identity != package.identity { + return false + } + + if let kind = package.kind, $0.package.kind != kind { + return false + } + + return true + }) else { XCTFail("No binding found for \(package.identity).", file: file, line: line) continue } @@ -3335,10 +3376,24 @@ private func AssertResult( _ packages: [(identifier: String, version: BoundVersion)], file: StaticString = #file, line: UInt = #line +) { + AssertResult(result, packages.map { ($0, nil, $1) }, file: file, line: line) +} + +private func AssertResult( + _ result: Result<[DependencyResolverBinding], Error>, + _ packages: [(identifier: String, kind: PackageReference.Kind?, version: BoundVersion)], + file: StaticString = #file, + line: UInt = #line ) { switch result { case .success(let bindings): - AssertBindings(bindings, packages.map { (PackageIdentity($0.identifier), $0.version) }, file: file, line: line) + AssertBindings( + bindings, + packages.map { (PackageIdentity($0.identifier), $0.kind, $0.version) }, + file: file, + line: line + ) case .failure(let error): XCTFail("Unexpected error: \(error)", file: file, line: line) } @@ -3544,6 +3599,18 @@ public struct MockProvider: PackageContainerProvider { ) -> Void ) { queue.async { + if case .providedLibrary(_, _) = package.kind { + do { + let container = try ProvidedLibraryPackageContainer( + package: package, + observabilityScope: observabilityScope + ) + return completion(.success(container)) + } catch { + return completion(.failure(error)) + } + } + completion( self.containersByIdentifier[package].map { .success($0) } ?? .failure(_MockLoadingError.unknownModule) From 557f3c1cd618e29c393f88518cdcd2ed279c3d17 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 22 Apr 2024 11:17:43 -0700 Subject: [PATCH 09/16] Remove all extraneous uses of provided libraries throughout code base Only workspace and PubGrub dependency resolver need to know about provided libraries. Post-resolution everything acts based on managed dependencies and package identifiers. --- .../PackageCommands/EditCommands.swift | 1 - Sources/CoreCommands/SwiftCommandState.swift | 1 - Sources/PackageGraph/ModulesGraph.swift | 1 - .../ProvidedLibraryPackageContainer.swift | 4 +-- .../PubGrub/ContainerProvider.swift | 7 ++--- .../PubGrub/PubGrubDependencyResolver.swift | 7 +++-- Sources/SPMTestSupport/MockWorkspace.swift | 6 ---- .../Workspace/Workspace+Dependencies.swift | 25 ++-------------- Sources/Workspace/Workspace+Editing.swift | 2 -- Sources/Workspace/Workspace+Manifests.swift | 29 ------------------- Sources/Workspace/Workspace.swift | 13 ++------- Tests/CommandsTests/PackageCommandTests.swift | 1 - Tests/FunctionalTests/PluginTests.swift | 4 --- .../PluginInvocationTests.swift | 6 ---- 14 files changed, 12 insertions(+), 95 deletions(-) diff --git a/Sources/Commands/PackageCommands/EditCommands.swift b/Sources/Commands/PackageCommands/EditCommands.swift index db6a4e8004c..f20d5c44a2f 100644 --- a/Sources/Commands/PackageCommands/EditCommands.swift +++ b/Sources/Commands/PackageCommands/EditCommands.swift @@ -74,7 +74,6 @@ extension SwiftPackageCommand { packageName: packageName, forceRemove: shouldForceRemove, root: swiftCommandState.getWorkspaceRoot(), - availableLibraries: swiftCommandState.getHostToolchain().providedLibraries, observabilityScope: swiftCommandState.observabilityScope ) } diff --git a/Sources/CoreCommands/SwiftCommandState.swift b/Sources/CoreCommands/SwiftCommandState.swift index 754a5401ea4..b94ff2268cd 100644 --- a/Sources/CoreCommands/SwiftCommandState.swift +++ b/Sources/CoreCommands/SwiftCommandState.swift @@ -607,7 +607,6 @@ package final class SwiftCommandState { explicitProduct: explicitProduct, forceResolvedVersions: options.resolver.forceResolvedVersions, testEntryPointPath: testEntryPointPath, - availableLibraries: self.getHostToolchain().providedLibraries, observabilityScope: self.observabilityScope ) diff --git a/Sources/PackageGraph/ModulesGraph.swift b/Sources/PackageGraph/ModulesGraph.swift index 49b2ea1f979..345724c2971 100644 --- a/Sources/PackageGraph/ModulesGraph.swift +++ b/Sources/PackageGraph/ModulesGraph.swift @@ -490,7 +490,6 @@ public func loadModulesGraph( shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts, createREPLProduct: createREPLProduct, customXCTestMinimumDeploymentTargets: customXCTestMinimumDeploymentTargets, - availableLibraries: [], fileSystem: fileSystem, observabilityScope: observabilityScope ) diff --git a/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift b/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift index 8f95ca0f7ad..73c3bf6252a 100644 --- a/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift +++ b/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift @@ -46,7 +46,7 @@ public struct ProvidedLibraryPackageContainer: PackageContainer { } public func toolsVersion(for version: Version) throws -> ToolsVersion { - .v6_0 + .v4 } public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] { @@ -54,7 +54,7 @@ public struct ProvidedLibraryPackageContainer: PackageContainer { } public func versionsAscending() throws -> [Version] { - [] // TODO + [] } public func getDependencies(at version: Version, productFilter: ProductFilter) throws -> [PackageContainerConstraint] { diff --git a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift index 95e43367f6e..dacf263937d 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift @@ -115,12 +115,9 @@ final class ContainerProvider { } /// Starts prefetching the given containers. - func prefetch(containers identifiers: [PackageReference], availableLibraries: [ProvidedLibrary]) { - let filteredIdentifiers = identifiers.filter { - $0.matchingPrebuiltLibrary(in: availableLibraries) == nil - } + func prefetch(containers identifiers: [PackageReference]) { // Process each container. - for identifier in filteredIdentifiers { + for identifier in identifiers { var needsFetching = false self.prefetches.memoize(identifier) { let group = DispatchGroup() diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index 00eead9a16f..2aaecf83cba 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -203,7 +203,7 @@ public struct PubGrubDependencyResolver { let pins = self.pins.values .map(\.packageRef) .filter { !inputs.overriddenPackages.keys.contains($0) } - self.provider.prefetch(containers: pins, availableLibraries: self.availableLibraries) + self.provider.prefetch(containers: pins) } let state = State(root: root, overriddenPackages: inputs.overriddenPackages) @@ -500,8 +500,9 @@ public struct PubGrubDependencyResolver { // initiate prefetch of known packages that will be used to make the decision on the next step self.provider.prefetch( - containers: state.solution.undecided.map(\.node.package), - availableLibraries: self.availableLibraries + containers: state.solution.undecided.map(\.node.package).filter { + $0.matchingPrebuiltLibrary(in: self.availableLibraries) == nil + } ) // If decision making determines that no more decisions are to be diff --git a/Sources/SPMTestSupport/MockWorkspace.swift b/Sources/SPMTestSupport/MockWorkspace.swift index 81056281f22..a09adf4079d 100644 --- a/Sources/SPMTestSupport/MockWorkspace.swift +++ b/Sources/SPMTestSupport/MockWorkspace.swift @@ -368,7 +368,6 @@ package final class MockWorkspace { packageName: packageName, forceRemove: forceRemove, root: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) } @@ -467,7 +466,6 @@ package final class MockWorkspace { let graph = try workspace.loadPackageGraph( rootInput: rootInput, forceResolvedVersions: forceResolvedVersions, - availableLibraries: [], // assume no provided libraries for testing. expectedSigningEntities: expectedSigningEntities, observabilityScope: observability.topScope ) @@ -506,7 +504,6 @@ package final class MockWorkspace { try workspace.loadPackageGraph( rootInput: rootInput, forceResolvedVersions: forceResolvedVersions, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) } @@ -532,7 +529,6 @@ package final class MockWorkspace { let dependencyManifests = try workspace.loadDependencyManifests( root: root, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) @@ -541,7 +537,6 @@ package final class MockWorkspace { dependencyManifests: dependencyManifests, pinsStore: pinsStore, constraints: [], - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) @@ -752,7 +747,6 @@ package final class MockWorkspace { let graphRoot = PackageGraphRoot(input: rootInput, manifests: rootManifests, observabilityScope: observability.topScope) let manifests = try workspace.loadDependencyManifests( root: graphRoot, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) result(manifests, observability.diagnostics) diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 6b9bd9897a3..0fab90aa3f1 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -58,7 +58,6 @@ extension Workspace { root: PackageGraphRootInput, packages: [String] = [], dryRun: Bool = false, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> [(PackageReference, Workspace.PackageStateChange)]? { let start = DispatchTime.now() @@ -89,7 +88,6 @@ extension Workspace { ) let currentManifests = try self.loadDependencyManifests( root: graphRoot, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) @@ -124,7 +122,6 @@ extension Workspace { // Resolve the dependencies. let resolver = try self.createResolver( pins: pins, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) self.activeResolver = resolver @@ -167,7 +164,6 @@ extension Workspace { // Load the updated manifests. let updatedDependencyManifests = try self.loadDependencyManifests( root: graphRoot, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) // If we have missing packages, something is fundamentally wrong with the resolution of the graph @@ -201,7 +197,6 @@ extension Workspace { func _resolve( root: PackageGraphRootInput, explicitProduct: String?, - availableLibraries: [ProvidedLibrary], resolvedFileStrategy: ResolvedFileStrategy, observabilityScope: ObservabilityScope ) throws -> DependencyManifests { @@ -217,7 +212,6 @@ extension Workspace { return try self._resolveBasedOnResolvedVersionsFile( root: root, explicitProduct: explicitProduct, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) case .update(let forceResolution): @@ -254,7 +248,6 @@ extension Workspace { let (manifests, precomputationResult) = try self.tryResolveBasedOnResolvedVersionsFile( root: root, explicitProduct: explicitProduct, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) switch precomputationResult { @@ -278,7 +271,6 @@ extension Workspace { return try self.resolveAndUpdateResolvedFile( root: root, explicitProduct: explicitProduct, - availableLibraries: availableLibraries, forceResolution: forceResolution, constraints: [], observabilityScope: observabilityScope @@ -305,13 +297,11 @@ extension Workspace { func _resolveBasedOnResolvedVersionsFile( root: PackageGraphRootInput, explicitProduct: String?, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> DependencyManifests { let (manifests, precomputationResult) = try self.tryResolveBasedOnResolvedVersionsFile( root: root, explicitProduct: explicitProduct, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) switch precomputationResult { @@ -344,7 +334,6 @@ extension Workspace { fileprivate func tryResolveBasedOnResolvedVersionsFile( root: PackageGraphRootInput, explicitProduct: String?, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> (DependencyManifests, ResolutionPrecomputationResult) { // Ensure the cache path exists. @@ -371,7 +360,6 @@ extension Workspace { return try ( self.loadDependencyManifests( root: graphRoot, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ), .notRequired @@ -464,7 +452,6 @@ extension Workspace { let currentManifests = try self.loadDependencyManifests( root: graphRoot, automaticallyAddManagedDependencies: true, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) @@ -479,7 +466,6 @@ extension Workspace { dependencyManifests: currentManifests, pinsStore: pinsStore, constraints: [], - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) @@ -496,7 +482,6 @@ extension Workspace { func resolveAndUpdateResolvedFile( root: PackageGraphRootInput, explicitProduct: String? = nil, - availableLibraries: [ProvidedLibrary], forceResolution: Bool, constraints: [PackageContainerConstraint], observabilityScope: ObservabilityScope @@ -524,7 +509,6 @@ extension Workspace { ) let currentManifests = try self.loadDependencyManifests( root: graphRoot, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) guard !observabilityScope.errorsReported else { @@ -561,7 +545,6 @@ extension Workspace { dependencyManifests: currentManifests, pinsStore: pinsStore, constraints: constraints, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) @@ -597,7 +580,6 @@ extension Workspace { // Perform dependency resolution. let resolver = try self.createResolver( pins: pinsStore.pins, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) self.activeResolver = resolver @@ -628,7 +610,6 @@ extension Workspace { // Update the pinsStore. let updatedDependencyManifests = try self.loadDependencyManifests( root: graphRoot, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) // If we still have missing packages, something is fundamentally wrong with the resolution of the graph @@ -850,7 +831,6 @@ extension Workspace { dependencyManifests: DependencyManifests, pinsStore: PinsStore, constraints: [PackageContainerConstraint], - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> ResolutionPrecomputationResult { let computedConstraints = @@ -867,7 +847,7 @@ extension Workspace { let resolver = PubGrubDependencyResolver( provider: precomputationProvider, pins: pinsStore.pins, - availableLibraries: availableLibraries, + availableLibraries: self.providedLibraries, observabilityScope: observabilityScope ) let result = resolver.solve(constraints: computedConstraints) @@ -1145,7 +1125,6 @@ extension Workspace { /// Creates resolver for the workspace. fileprivate func createResolver( pins: PinsStore.Pins, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> PubGrubDependencyResolver { var delegate: DependencyResolverDelegate @@ -1162,7 +1141,7 @@ extension Workspace { return PubGrubDependencyResolver( provider: packageContainerProvider, pins: pins, - availableLibraries: availableLibraries, + availableLibraries: self.providedLibraries, skipDependenciesUpdates: self.configuration.skipDependenciesUpdates, prefetchBasedOnResolvedFile: self.configuration.prefetchBasedOnResolvedFile, observabilityScope: observabilityScope, diff --git a/Sources/Workspace/Workspace+Editing.swift b/Sources/Workspace/Workspace+Editing.swift index 03939149bed..cf429ad2eaa 100644 --- a/Sources/Workspace/Workspace+Editing.swift +++ b/Sources/Workspace/Workspace+Editing.swift @@ -173,7 +173,6 @@ extension Workspace { dependency: ManagedDependency, forceRemove: Bool, root: PackageGraphRootInput? = nil, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws { // Compute if we need to force remove. @@ -238,7 +237,6 @@ extension Workspace { try self._resolve( root: root, explicitProduct: .none, - availableLibraries: availableLibraries, resolvedFileStrategy: .update(forceResolution: false), observabilityScope: observabilityScope ) diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index e388d5d9feb..3f6f36c051b 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -61,8 +61,6 @@ extension Workspace { private let workspace: Workspace - private let availableLibraries: [ProvidedLibrary] - private let observabilityScope: ObservabilityScope private let _dependencies: LoadableResult<( @@ -81,20 +79,17 @@ extension Workspace { fileSystem: FileSystem )], workspace: Workspace, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) { self.root = root self.dependencies = dependencies self.workspace = workspace - self.availableLibraries = availableLibraries self.observabilityScope = observabilityScope self._dependencies = LoadableResult { try Self.computeDependencies( root: root, dependencies: dependencies, workspace: workspace, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) } @@ -173,7 +168,6 @@ extension Workspace { fileSystem: FileSystem )], workspace: Workspace, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> ( @@ -204,17 +198,6 @@ extension Workspace { return PackageReference(identity: $0.key, kind: $0.1.packageKind) }) - let identitiesAvailableInSDK = availableLibraries.flatMap { - $0.metadata.identities.map { - $0.ref - }.filter { - // We "trust the process" here, if an identity from the SDK is available, filter it. - !availableIdentities.contains($0) - }.map { - $0.identity - } - } - var inputIdentities: OrderedCollections.OrderedSet = [] let inputNodes: [GraphLoadingNode] = root.packages.map { identity, package in inputIdentities.append(package.reference) @@ -292,11 +275,6 @@ extension Workspace { } requiredIdentities = inputIdentities.union(requiredIdentities) - let identitiesToFilter = requiredIdentities.filter { - return identitiesAvailableInSDK.contains($0.identity) - } - requiredIdentities = requiredIdentities.subtracting(identitiesToFilter) - // We should never have loaded a manifest we don't need. assert( availableIdentities.isSubset(of: requiredIdentities), @@ -438,7 +416,6 @@ extension Workspace { public func loadDependencyManifests( root: PackageGraphRoot, automaticallyAddManagedDependencies: Bool = false, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws -> DependencyManifests { let prepopulateManagedDependencies: ([PackageReference]) throws -> Void = { refs in @@ -481,7 +458,6 @@ extension Workspace { // Validates that all the managed dependencies are still present in the file system. self.fixManagedDependencies( - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) guard !observabilityScope.errorsReported else { @@ -490,7 +466,6 @@ extension Workspace { root: root, dependencies: [], workspace: self, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) } @@ -564,7 +539,6 @@ extension Workspace { root: root, dependencies: [], workspace: self, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) } @@ -625,7 +599,6 @@ extension Workspace { root: root, dependencies: dependencies, workspace: self, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) } @@ -821,7 +794,6 @@ extension Workspace { /// If some edited dependency is removed from the file system, mark it as unedited and /// fallback on the original checkout. private func fixManagedDependencies( - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) { // Reset managed dependencies if the state file was removed during the lifetime of the Workspace object. @@ -895,7 +867,6 @@ extension Workspace { try self.unedit( dependency: dependency, forceRemove: true, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 4e81b3d7e81..c27e9c5f523 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -572,9 +572,9 @@ public class Workspace { ) } - fileprivate var providedLibraries: [ProvidedLibrary] { + var providedLibraries: [ProvidedLibrary] { // Note: Eventually, we should get these from the individual SDKs, but the first step is providing the metadata centrally in the toolchain. - return self.hostToolchain.providedLibraries + self.hostToolchain.providedLibraries } } @@ -626,7 +626,6 @@ extension Workspace { packageName: String, forceRemove: Bool, root: PackageGraphRootInput, - availableLibraries: [ProvidedLibrary], observabilityScope: ObservabilityScope ) throws { guard let dependency = self.state.dependencies[.plain(packageName)] else { @@ -643,7 +642,6 @@ extension Workspace { dependency: dependency, forceRemove: forceRemove, root: root, - availableLibraries: availableLibraries, observabilityScope: observabilityScope ) } @@ -664,7 +662,6 @@ extension Workspace { try self._resolve( root: root, explicitProduct: explicitProduct, - availableLibraries: self.providedLibraries, resolvedFileStrategy: forceResolvedVersions ? .lockFile : forceResolution ? .update(forceResolution: true) : .bestEffort, observabilityScope: observabilityScope @@ -738,7 +735,6 @@ extension Workspace { // Run the resolution. try self.resolveAndUpdateResolvedFile( root: root, - availableLibraries: self.providedLibraries, forceResolution: false, constraints: [constraint], observabilityScope: observabilityScope @@ -756,7 +752,6 @@ extension Workspace { try self._resolveBasedOnResolvedVersionsFile( root: root, explicitProduct: .none, - availableLibraries: self.providedLibraries, observabilityScope: observabilityScope ) } @@ -867,7 +862,6 @@ extension Workspace { root: root, packages: packages, dryRun: dryRun, - availableLibraries: self.providedLibraries, observabilityScope: observabilityScope ) } @@ -879,7 +873,6 @@ extension Workspace { forceResolvedVersions: Bool = false, customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none, testEntryPointPath: AbsolutePath? = nil, - availableLibraries: [ProvidedLibrary], expectedSigningEntities: [PackageIdentity: RegistryReleaseMetadata.SigningEntity] = [:], observabilityScope: ObservabilityScope ) throws -> ModulesGraph { @@ -899,7 +892,6 @@ extension Workspace { let manifests = try self._resolve( root: root, explicitProduct: explicitProduct, - availableLibraries: availableLibraries, resolvedFileStrategy: forceResolvedVersions ? .lockFile : .bestEffort, observabilityScope: observabilityScope ) @@ -947,7 +939,6 @@ extension Workspace { try self.loadPackageGraph( rootInput: PackageGraphRootInput(packages: [rootPath]), explicitProduct: explicitProduct, - availableLibraries: self.providedLibraries, observabilityScope: observabilityScope ) } diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index 43a71bcf1a7..3dcad8f34d1 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -3328,7 +3328,6 @@ final class PackageCommandTests: CommandsTestCase { // Load the package graph. let _ = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssertNoDiagnostics(observability.diagnostics) diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index 47218b62df3..c81842f822a 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -445,7 +445,6 @@ final class PluginTests: XCTestCase { // Load the package graph. let packageGraph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssertNoDiagnostics(observability.diagnostics) @@ -632,7 +631,6 @@ final class PluginTests: XCTestCase { // Load the package graph. let packageGraph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssertNoDiagnostics(observability.diagnostics) @@ -730,7 +728,6 @@ final class PluginTests: XCTestCase { // Load the package graph. let packageGraph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssertNoDiagnostics(observability.diagnostics) @@ -1046,7 +1043,6 @@ final class PluginTests: XCTestCase { // Load the package graph. let packageGraph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssert(packageGraph.packages.count == 4, "\(packageGraph.packages)") diff --git a/Tests/SPMBuildCoreTests/PluginInvocationTests.swift b/Tests/SPMBuildCoreTests/PluginInvocationTests.swift index 58668f9e5db..eb9be4cd970 100644 --- a/Tests/SPMBuildCoreTests/PluginInvocationTests.swift +++ b/Tests/SPMBuildCoreTests/PluginInvocationTests.swift @@ -306,7 +306,6 @@ final class PluginInvocationTests: XCTestCase { // Load the package graph. let packageGraph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssertNoDiagnostics(observability.diagnostics) @@ -687,7 +686,6 @@ final class PluginInvocationTests: XCTestCase { // Load the package graph. XCTAssertThrowsError(try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope )) { error in var diagnosed = false @@ -767,7 +765,6 @@ final class PluginInvocationTests: XCTestCase { // Load the package graph. XCTAssertThrowsError(try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope)) { error in var diagnosed = false if let realError = error as? PackageGraphError, @@ -877,7 +874,6 @@ final class PluginInvocationTests: XCTestCase { // Load the package graph. let packageGraph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssertNoDiagnostics(observability.diagnostics) @@ -1061,7 +1057,6 @@ final class PluginInvocationTests: XCTestCase { let graph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) let dict = try await workspace.loadPluginImports(packageGraph: graph) @@ -1209,7 +1204,6 @@ final class PluginInvocationTests: XCTestCase { // Load the package graph. let packageGraph = try workspace.loadPackageGraph( rootInput: rootInput, - availableLibraries: [], // assume no provided libraries for testing. observabilityScope: observability.topScope ) XCTAssertNoDiagnostics(observability.diagnostics) From f28d0ec6b182f900165a40d77779f82b967cfa6e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 23 Apr 2024 11:18:38 -0700 Subject: [PATCH 10/16] Switch ProvidedLibrary.vesion to produce a `Version` --- .../Resolution/PubGrub/PubGrubDependencyResolver.swift | 10 ++++------ .../InstalledLibrariesSupport/LibraryMetadata.swift | 6 +++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index 2aaecf83cba..c576bde35cf 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -242,7 +242,7 @@ public struct PubGrubDependencyResolver { } if let library = package.matchingPrebuiltLibrary(in: availableLibraries), - boundVersion == .version(.init(stringLiteral: library.version)) + boundVersion == .version(library.version) { guard case .remoteSourceControl(let url) = package.kind else { throw InternalError("Matched provided library against invalid package: \(package)") @@ -748,11 +748,9 @@ public struct PubGrubDependencyResolver { continue } - let version = Version(stringLiteral: library.version) - - if pkgTerm.requirement.contains(version) { - self.delegate?.didResolve(term: pkgTerm, version: version, duration: start.distance(to: .now())) - state.decide(pkgTerm.node, at: version) + if pkgTerm.requirement.contains(library.version) { + self.delegate?.didResolve(term: pkgTerm, version: library.version, duration: start.distance(to: .now())) + state.decide(pkgTerm.node, at: library.version) return completion(.success(pkgTerm.node)) } } diff --git a/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift b/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift index 362dae238cf..bde7b74ecfc 100644 --- a/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift +++ b/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift @@ -11,14 +11,14 @@ //===----------------------------------------------------------------------===// import Basics -import Foundation +import struct TSCUtility.Version public struct ProvidedLibrary { public let location: AbsolutePath public let metadata: LibraryMetadata - public var version: String { - metadata.version + public var version: Version { + .init(stringLiteral: metadata.version) } } From 6343d1de11f33e5b556de7d710f0a0747e6f60b6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 24 Apr 2024 10:00:40 -0700 Subject: [PATCH 11/16] [PubGrub] Note that selected package version is backed by a provided library First step on the path to remove `ProvidedLibraryPackageContainer` and `PackageReference.providedLibrary` and fix a problem where a package backed by a provided library has to be always treated as "new" or "updated" by `precomputeResolution` which triggers full resolution. --- Sources/PackageGraph/BoundVersion.swift | 5 +- .../PubGrub/PubGrubDependencyResolver.swift | 35 +++++---- .../LibraryMetadata.swift | 6 +- .../SourceControlPackageContainer.swift | 2 +- .../Workspace/Workspace+Dependencies.swift | 2 +- .../DependencyResolverPerfTests.swift | 2 +- Tests/PackageGraphTests/PubgrubTests.swift | 71 ++++--------------- 7 files changed, 38 insertions(+), 85 deletions(-) diff --git a/Sources/PackageGraph/BoundVersion.swift b/Sources/PackageGraph/BoundVersion.swift index 7aa90a33ff6..459dee7f956 100644 --- a/Sources/PackageGraph/BoundVersion.swift +++ b/Sources/PackageGraph/BoundVersion.swift @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +import struct PackageModel.ProvidedLibrary import struct TSCUtility.Version /// A bound version for a package within an assignment. @@ -22,7 +23,7 @@ public enum BoundVersion: Equatable, Hashable { case excluded /// The version of the package to include. - case version(Version) + case version(Version, library: ProvidedLibrary? = nil) /// The package assignment is unversioned. case unversioned @@ -36,7 +37,7 @@ extension BoundVersion: CustomStringConvertible { switch self { case .excluded: return "excluded" - case .version(let version): + case .version(let version, _): return version.description case .unversioned: return "unversioned" diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index c576bde35cf..00f3f1b1013 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -231,35 +231,32 @@ public struct PubGrubDependencyResolver { continue } - var package = assignment.term.node.package + let package = assignment.term.node.package let boundVersion: BoundVersion switch assignment.term.requirement { case .exact(let version): - boundVersion = .version(version) + if let library = package.matchingPrebuiltLibrary(in: availableLibraries), + version == library.version + { + boundVersion = .version(version, library: library) + } else { + boundVersion = .version(version) + } case .range, .any, .empty, .ranges: throw InternalError("unexpected requirement value for assignment \(assignment.term)") } - if let library = package.matchingPrebuiltLibrary(in: availableLibraries), - boundVersion == .version(library.version) - { - guard case .remoteSourceControl(let url) = package.kind else { - throw InternalError("Matched provided library against invalid package: \(package)") - } - - package = .providedLibrary( - identity: package.identity, - origin: url, - path: library.location - ) - } - let products = assignment.term.node.productFilter - // TODO: replace with async/await when available - let container = try temp_await { self.provider.getContainer(for: package, completion: $0) } - let updatePackage = try container.underlying.loadPackageReference(at: boundVersion) + let updatePackage: PackageReference + if case .version(_, let library) = boundVersion, library != nil { + updatePackage = package + } else { + // TODO: replace with async/await when available + let container = try temp_await { self.provider.getContainer(for: package, completion: $0) } + updatePackage = try container.underlying.loadPackageReference(at: boundVersion) + } if var existing = flattenedAssignments[updatePackage] { guard existing.binding == boundVersion else { diff --git a/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift b/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift index bde7b74ecfc..4a0312b6e78 100644 --- a/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift +++ b/Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift @@ -13,7 +13,7 @@ import Basics import struct TSCUtility.Version -public struct ProvidedLibrary { +public struct ProvidedLibrary: Hashable { public let location: AbsolutePath public let metadata: LibraryMetadata @@ -22,8 +22,8 @@ public struct ProvidedLibrary { } } -public struct LibraryMetadata: Decodable { - public enum Identity: Equatable, Decodable { +public struct LibraryMetadata: Hashable, Decodable { + public enum Identity: Hashable, Decodable { case packageIdentity(scope: String, name: String) case sourceControl(url: SourceControlURL) } diff --git a/Sources/Workspace/PackageContainer/SourceControlPackageContainer.swift b/Sources/Workspace/PackageContainer/SourceControlPackageContainer.swift index 6e634df9fd4..03f48dd0120 100644 --- a/Sources/Workspace/PackageContainer/SourceControlPackageContainer.swift +++ b/Sources/Workspace/PackageContainer/SourceControlPackageContainer.swift @@ -354,7 +354,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri let revision: Revision var version: Version? switch boundVersion { - case .version(let v): + case .version(let v, _): guard let tag = try self.knownVersions()[v] else { throw StringError("unknown tag \(v)") } diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 0fab90aa3f1..6490577d92b 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -1096,7 +1096,7 @@ extension Workspace { packageStateChanges[binding.package.identity] = (binding.package, .added(newState)) } - case .version(let version): + case .version(let version, _): let stateChange: PackageStateChange switch currentDependency?.state { case .sourceControlCheckout(.version(version, _)), diff --git a/Tests/PackageGraphPerformanceTests/DependencyResolverPerfTests.swift b/Tests/PackageGraphPerformanceTests/DependencyResolverPerfTests.swift index 9d92488bbf3..054cf84ea20 100644 --- a/Tests/PackageGraphPerformanceTests/DependencyResolverPerfTests.swift +++ b/Tests/PackageGraphPerformanceTests/DependencyResolverPerfTests.swift @@ -69,7 +69,7 @@ class DependencyResolverRealWorldPerfTests: XCTestCasePerf { switch resolver.solve(constraints: graph.constraints) { case .success(let result): let result: [(container: PackageReference, version: Version)] = result.compactMap { - guard case .version(let version) = $0.boundVersion else { + guard case .version(let version, _) = $0.boundVersion else { XCTFail("Unexpected result") return nil } diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift index 8920d14211e..5e0f7e21ad4 100644 --- a/Tests/PackageGraphTests/PubgrubTests.swift +++ b/Tests/PackageGraphTests/PubgrubTests.swift @@ -2046,16 +2046,12 @@ final class PubGrubTestsBasicGraphs: XCTestCase { let result = resolver.solve(constraints: dependencies1) print(try result.get()) AssertResult(result, [ - ( - "foo", - .providedLibrary(.init("https://example.com/org/foo"), .init("/foo")), - .version(.init(stringLiteral: "1.0.0")) - ), + ("foo", .version(.init(stringLiteral: "1.0.0"), library: availableLibraries.first!)), ]) let result2 = resolver.solve(constraints: dependencies2) AssertResult(result2, [ - ("foo", fooRef.kind, .version(.init(stringLiteral: "1.2.0"))), + ("foo", .version(.init(stringLiteral: "1.2.0"))), ]) } @@ -2114,16 +2110,8 @@ final class PubGrubTestsBasicGraphs: XCTestCase { let result = resolver.solve(constraints: dependencies) AssertResult(result, [ - ( - "foo", - .providedLibrary(.init("https://example.com/org/foo"), .init("/foo")), - .version(.init(stringLiteral: "1.1.0")) - ), - ( - "target", - .localSourceControl("/target"), - .version(.init(stringLiteral: "2.0.0")) - ), + ("foo", .version(.init(stringLiteral: "1.1.0"), library: availableLibraries.first!)), + ("target", .version(.init(stringLiteral: "2.0.0"))), ]) } @@ -2167,8 +2155,8 @@ final class PubGrubTestsBasicGraphs: XCTestCase { let result = resolver.solve(constraints: dependencies) AssertResult(result, [ - ("foo", fooRef.kind, .version(.init(stringLiteral: "1.1.0"))), - ("bar", .localSourceControl("/bar"), .version(.init(stringLiteral: "1.0.0"))), + ("foo", .version(.init(stringLiteral: "1.1.0"))), + ("bar", .version(.init(stringLiteral: "1.0.0"))), ]) } } @@ -3312,22 +3300,6 @@ private func AssertBindings( _ packages: [(identity: PackageIdentity, version: BoundVersion)], file: StaticString = #file, line: UInt = #line -) { - AssertBindings( - bindings, - packages.map { - (identity: $0, kind: nil, version: $1) - }, - file: file, - line: line - ) -} - -private func AssertBindings( - _ bindings: [DependencyResolverBinding], - _ packages: [(identity: PackageIdentity, kind: PackageReference.Kind?, version: BoundVersion)], - file: StaticString = #file, - line: UInt = #line ) { if bindings.count > packages.count { let unexpectedBindings = bindings @@ -3346,15 +3318,7 @@ private func AssertBindings( } for package in packages { guard let binding = bindings.first(where: { - if $0.package.identity != package.identity { - return false - } - - if let kind = package.kind, $0.package.kind != kind { - return false - } - - return true + $0.package.identity == package.identity }) else { XCTFail("No binding found for \(package.identity).", file: file, line: line) continue @@ -3376,21 +3340,12 @@ private func AssertResult( _ packages: [(identifier: String, version: BoundVersion)], file: StaticString = #file, line: UInt = #line -) { - AssertResult(result, packages.map { ($0, nil, $1) }, file: file, line: line) -} - -private func AssertResult( - _ result: Result<[DependencyResolverBinding], Error>, - _ packages: [(identifier: String, kind: PackageReference.Kind?, version: BoundVersion)], - file: StaticString = #file, - line: UInt = #line ) { switch result { case .success(let bindings): AssertBindings( bindings, - packages.map { (PackageIdentity($0.identifier), $0.kind, $0.version) }, + packages.map { (PackageIdentity($0.identifier), $0.version) }, file: file, line: line ) @@ -3440,7 +3395,7 @@ public class MockContainer: PackageContainer { public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] { var versions: [Version] = [] for version in self._versions.reversed() { - guard case .version(let v) = version else { continue } + guard case .version(let v, _) = version else { continue } versions.append(v) } return versions @@ -3449,7 +3404,7 @@ public class MockContainer: PackageContainer { public func versionsAscending() throws -> [Version] { var versions: [Version] = [] for version in self._versions { - guard case .version(let v) = version else { continue } + guard case .version(let v, _) = version else { continue } versions.append(v) } return versions @@ -3516,7 +3471,7 @@ public class MockContainer: PackageContainer { self._versions.append(version) self._versions = self._versions .sorted(by: { lhs, rhs -> Bool in - guard case .version(let lv) = lhs, case .version(let rv) = rhs else { + guard case .version(let lv, _) = lhs, case .version(let rv, _) = rhs else { return true } return lv < rv @@ -3571,7 +3526,7 @@ public class MockContainer: PackageContainer { let versions = dependencies.keys.compactMap(Version.init(_:)) self._versions = versions .sorted() - .map(BoundVersion.version) + .map { .version($0) } } } @@ -3742,7 +3697,7 @@ class DependencyGraphBuilder { let container = self .containers[packageReference.identity.description] ?? MockContainer(package: packageReference) - if case .version(let v) = version { + if case .version(let v, _) = version { container.versionsToolsVersions[v] = toolsVersion ?? container.toolsVersion } From e77b2a6d35855a3407abfaa6978a4b7b7451347a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 24 Apr 2024 14:03:41 -0700 Subject: [PATCH 12/16] [Workspace] NFC: Handle loading of provided library managed dependencies directly Instead of going through the ManifestLoader machinery and special cases in validator let's just produce mainfest directly. --- .../ManifestLoader+Validation.swift | 5 -- Sources/PackageLoading/ManifestLoader.swift | 60 ------------------- Sources/PackageModel/Manifest/Manifest.swift | 47 +++++++++++++++ Sources/Workspace/Workspace+Manifests.swift | 11 +++- 4 files changed, 55 insertions(+), 68 deletions(-) diff --git a/Sources/PackageLoading/ManifestLoader+Validation.swift b/Sources/PackageLoading/ManifestLoader+Validation.swift index 768286b05ef..af215dd5298 100644 --- a/Sources/PackageLoading/ManifestLoader+Validation.swift +++ b/Sources/PackageLoading/ManifestLoader+Validation.swift @@ -34,11 +34,6 @@ public struct ManifestValidator { /// Validate the provided manifest. public func validate() -> [Basics.Diagnostic] { - // Provided library manifest is synthesized by the package manager. - if case .providedLibrary = self.manifest.packageKind { - return [] - } - var diagnostics = [Basics.Diagnostic]() diagnostics += self.validateTargets() diff --git a/Sources/PackageLoading/ManifestLoader.swift b/Sources/PackageLoading/ManifestLoader.swift index bf8f55fda45..9424af7e617 100644 --- a/Sources/PackageLoading/ManifestLoader.swift +++ b/Sources/PackageLoading/ManifestLoader.swift @@ -206,19 +206,6 @@ extension ManifestLoaderProtocol { completion: @escaping (Result) -> Void ) { do { - if case .providedLibrary = packageKind { - let manifest = try self.loadLibrary( - fileSystem: fileSystem, - packagePath: packagePath, - packageKind: packageKind, - packageIdentity: packageIdentity, - packageLocation: packageLocation, - packageVersion: packageVersion?.version - ) - completion(.success(manifest)) - return - } - // find the manifest path and parse it's tools-version let manifestPath = try ManifestLoader.findManifest(packagePath: packagePath, fileSystem: fileSystem, currentToolsVersion: currentToolsVersion) let manifestToolsVersion = try ToolsVersionParser.parse(manifestPath: manifestPath, fileSystem: fileSystem) @@ -279,53 +266,6 @@ extension ManifestLoaderProtocol { ) } } - - private func loadLibrary( - fileSystem: FileSystem, - packagePath: AbsolutePath, - packageKind: PackageReference.Kind, - packageIdentity: PackageIdentity, - packageLocation: String, - packageVersion: Version? - ) throws -> Manifest { - let names = try fileSystem.getDirectoryContents(packagePath).filter { - $0.hasSuffix("swiftmodule") - }.map { - let components = $0.split(separator: ".") - return String(components[0]) - } - - let products: [ProductDescription] = try names.map { - try .init(name: $0, type: .library(.automatic), targets: [$0]) - } - - let targets: [TargetDescription] = try names.map { - try .init( - name: $0, - path: packagePath.pathString, - type: .providedLibrary - ) - } - - return .init( - displayName: packageIdentity.description, - path: packagePath.appending(component: "provided-libraries.json"), - packageKind: packageKind, - packageLocation: packageLocation, - defaultLocalization: nil, - platforms: [], - version: packageVersion, - revision: nil, - toolsVersion: .v6_0, - pkgConfig: nil, - providers: nil, - cLanguageStandard: nil, - cxxLanguageStandard: nil, - swiftLanguageVersions: nil, - products: products, - targets: targets - ) - } } // MARK: - ManifestLoader diff --git a/Sources/PackageModel/Manifest/Manifest.swift b/Sources/PackageModel/Manifest/Manifest.swift index 7556b79f7b9..73adb0802e0 100644 --- a/Sources/PackageModel/Manifest/Manifest.swift +++ b/Sources/PackageModel/Manifest/Manifest.swift @@ -554,3 +554,50 @@ extension Manifest: Encodable { try container.encode(self.packageKind, forKey: .packageKind) } } + +extension Manifest { + package static func forProvidedLibrary( + fileSystem: FileSystem, + package: PackageReference, + libraryPath: AbsolutePath, + version: Version + ) throws -> Manifest { + let names = try fileSystem.getDirectoryContents(libraryPath).filter { + $0.hasSuffix("swiftmodule") + }.map { + let components = $0.split(separator: ".") + return String(components[0]) + } + + let products: [ProductDescription] = try names.map { + try .init(name: $0, type: .library(.automatic), targets: [$0]) + } + + let targets: [TargetDescription] = try names.map { + try .init( + name: $0, + path: libraryPath.pathString, + type: .providedLibrary + ) + } + + return .init( + displayName: package.identity.description, + path: libraryPath.appending(component: "provided-library.json"), + packageKind: package.kind, + packageLocation: package.locationString, + defaultLocalization: nil, + platforms: [], + version: version, + revision: nil, + toolsVersion: .v6_0, + pkgConfig: nil, + providers: nil, + cLanguageStandard: nil, + cxxLanguageStandard: nil, + swiftLanguageVersions: nil, + products: products, + targets: targets + ) + } +} diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 3f6f36c051b..511dd191d50 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -659,9 +659,14 @@ extension Workspace { case .registryDownload(let downloadedVersion): packageKind = managedDependency.packageRef.kind packageVersion = downloadedVersion - case .providedLibrary(_, let version): - packageKind = managedDependency.packageRef.kind - packageVersion = version + case .providedLibrary(let path, let version): + let manifest: Manifest? = try? .forProvidedLibrary( + fileSystem: fileSystem, + package: managedDependency.packageRef, + libraryPath: path, + version: version + ) + return completion(manifest) case .custom(let availableVersion, _): packageKind = managedDependency.packageRef.kind packageVersion = availableVersion From db22f5b8bb61a4e058302586ea6d8d3695fe5886 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 24 Apr 2024 14:17:00 -0700 Subject: [PATCH 13/16] [Workspace] Introduce a new package state - `usesLibrary` All non-edited packages are eligible to use a provided library based on the dependency resolution information. `.usesLibrary` state would result in a new `.providedLibrary` managed dependency injected into workspace state instead of a regular local/remote package. --- Sources/Commands/PackageCommands/Update.swift | 2 +- Sources/Workspace/ManagedDependency.swift | 10 +-- .../Workspace/Workspace+Dependencies.swift | 63 +++++++++++-------- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/Sources/Commands/PackageCommands/Update.swift b/Sources/Commands/PackageCommands/Update.swift index b4ced1bad0e..fa2586c2c4f 100644 --- a/Sources/Commands/PackageCommands/Update.swift +++ b/Sources/Commands/PackageCommands/Update.swift @@ -73,7 +73,7 @@ extension SwiftPackageCommand { case .removed: report += "\n" report += "- \(package.identity) \(currentVersion)" - case .unchanged: + case .unchanged, .usesLibrary: continue } } diff --git a/Sources/Workspace/ManagedDependency.swift b/Sources/Workspace/ManagedDependency.swift index 4f476c99965..5f2fd75cfc5 100644 --- a/Sources/Workspace/ManagedDependency.swift +++ b/Sources/Workspace/ManagedDependency.swift @@ -153,15 +153,11 @@ extension Workspace { public static func providedLibrary( packageRef: PackageReference, - version: Version + library: ProvidedLibrary ) throws -> ManagedDependency { - guard case .providedLibrary(_, let path) = packageRef.kind else { - throw InternalError("invalid package type: \(packageRef.kind)") - } - - return ManagedDependency( + ManagedDependency( packageRef: packageRef, - state: .providedLibrary(at: path, version: version), + state: .providedLibrary(at: library.location, version: library.version), subpath: try RelativePath(validating: packageRef.identity.description) ) } diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 6490577d92b..f8b3f45b575 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -672,7 +672,7 @@ extension Workspace { metadata: packageRef.diagnosticsMetadata ).trap { switch state { - case .added, .updated, .unchanged: + case .added, .updated, .unchanged, .usesLibrary: break case .removed: try self.remove(package: packageRef) @@ -701,12 +701,27 @@ extension Workspace { productFilter: state.products, observabilityScope: observabilityScope ) - case .removed, .unchanged: + case .removed, .unchanged, .usesLibrary: break } } } + // Handle provided libraries + for (packageRef, state) in packageStateChanges { + observabilityScope.makeChildScope( + description: "adding provided libraries", + metadata: packageRef.diagnosticsMetadata + ).trap { + if case .usesLibrary(let library) = state { + try self.state.dependencies.add( + .providedLibrary(packageRef: packageRef, library: library) + ) + try self.state.save() + } + } + } + // Inform the delegate if nothing was updated. if packageStateChanges.filter({ $0.1 == .unchanged }).count == packageStateChanges.count { delegate?.dependenciesUpToDate() @@ -763,19 +778,6 @@ extension Workspace { state: .custom(version: version, path: path), subpath: RelativePath(validating: "") ) - self.state.dependencies.add(dependency) - try self.state.save() - return path - } else if let libraryContainer = container as? ProvidedLibraryPackageContainer { - guard case .providedLibrary(_, let path) = libraryContainer.package.kind else { - throw InternalError("invalid container for \(package.identity) of type \(package.kind)") - } - - let dependency: ManagedDependency = try .providedLibrary( - packageRef: libraryContainer.package, - version: version - ) - self.state.dependencies.add(dependency) try self.state.save() return path @@ -960,6 +962,9 @@ extension Workspace { /// The package is updated. case updated(State) + /// The package is replaced with a prebuilt library + case usesLibrary(ProvidedLibrary) + public var description: String { switch self { case .added(let requirement): @@ -970,15 +975,17 @@ extension Workspace { return "unchanged" case .updated(let requirement): return "updated(\(requirement))" + case .usesLibrary(let library): + return "usesLibrary(\(library.metadata.productName))" } } public var isAddedOrUpdated: Bool { switch self { case .added, .updated: - return true - case .unchanged, .removed: - return false + true + case .unchanged, .removed, .usesLibrary: + false } } } @@ -1096,18 +1103,20 @@ extension Workspace { packageStateChanges[binding.package.identity] = (binding.package, .added(newState)) } - case .version(let version, _): - let stateChange: PackageStateChange - switch currentDependency?.state { + case .version(let version, let library): + let stateChange: PackageStateChange = switch currentDependency?.state { case .sourceControlCheckout(.version(version, _)), - .registryDownload(version), - .providedLibrary(_, version: version), - .custom(version, _): - stateChange = .unchanged + .registryDownload(version), + .providedLibrary(_, version: version), + .custom(version, _): + library.flatMap { .usesLibrary($0) } ?? .unchanged case .edited, .fileSystem, .sourceControlCheckout, .registryDownload, .providedLibrary, .custom: - stateChange = .updated(.init(requirement: .version(version), products: binding.products)) + .updated(.init(requirement: .version(version), products: binding.products)) case nil: - stateChange = .added(.init(requirement: .version(version), products: binding.products)) + library.flatMap { .usesLibrary($0) } ?? .added(.init( + requirement: .version(version), + products: binding.products + )) } packageStateChanges[binding.package.identity] = (binding.package, stateChange) } From 2725f3bcd311f2de9c9de5cd6428c74bf5f7a553 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 24 Apr 2024 18:26:17 -0700 Subject: [PATCH 14/16] NFC: Remove `.providedLibrary` from PackageReference and its package container Dependency resolution is switched to use `BoundVersion` to carry provided library information, `.providedLibrary` is fully obsolete. --- Sources/Build/BuildPlan/BuildPlan.swift | 2 +- Sources/PackageGraph/CMakeLists.txt | 1 - Sources/PackageGraph/ModulesGraph.swift | 2 - .../ProvidedLibraryPackageContainer.swift | 75 ------------------- .../PubGrub/ContainerProvider.swift | 7 -- .../PubGrub/PubGrubDependencyResolver.swift | 2 +- .../PackageLoading/ManifestJSONParser.swift | 2 +- Sources/PackageModel/IdentityResolver.swift | 2 - Sources/PackageModel/PackageReference.swift | 26 ------- .../Plugins/PluginContextSerializer.swift | 2 - .../SPMTestSupport/MockManifestLoader.swift | 6 -- Sources/SPMTestSupport/MockWorkspace.swift | 2 - Sources/Workspace/Diagnostics.swift | 2 +- .../Workspace/Workspace+Dependencies.swift | 1 - .../Workspace+PackageContainer.swift | 9 --- Sources/Workspace/Workspace+State.swift | 38 ---------- Sources/Workspace/Workspace.swift | 2 - Tests/PackageGraphTests/PubgrubTests.swift | 12 --- .../PackageLoadingTests/PDLoadingTests.swift | 2 - 19 files changed, 4 insertions(+), 191 deletions(-) delete mode 100644 Sources/PackageGraph/ProvidedLibraryPackageContainer.swift diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 7a74eb5cf3f..3fd34b1f65c 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -740,7 +740,7 @@ extension ResolvedPackage { switch self.underlying.manifest.packageKind { case .registry, .remoteSourceControl, .localSourceControl: return true - case .root, .fileSystem, .providedLibrary: + case .root, .fileSystem: return false } } diff --git a/Sources/PackageGraph/CMakeLists.txt b/Sources/PackageGraph/CMakeLists.txt index 13d010b11a7..b95b1eb9feb 100644 --- a/Sources/PackageGraph/CMakeLists.txt +++ b/Sources/PackageGraph/CMakeLists.txt @@ -20,7 +20,6 @@ add_library(PackageGraph PackageModel+Extensions.swift PackageRequirement.swift PinsStore.swift - ProvidedLibraryPackageContainer.swift Resolution/PubGrub/Assignment.swift Resolution/PubGrub/ContainerProvider.swift Resolution/PubGrub/DiagnosticReportBuilder.swift diff --git a/Sources/PackageGraph/ModulesGraph.swift b/Sources/PackageGraph/ModulesGraph.swift index 345724c2971..e37b0ab4d54 100644 --- a/Sources/PackageGraph/ModulesGraph.swift +++ b/Sources/PackageGraph/ModulesGraph.swift @@ -362,8 +362,6 @@ extension PackageGraphError: CustomStringConvertible { description += " (at '\(path)')" case .remoteSourceControl(let url): description += " (from '\(url)')" - case .providedLibrary(let url, let path): - description += " (from \(url) at \(path))" case .registry: break } diff --git a/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift b/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift deleted file mode 100644 index 73c3bf6252a..00000000000 --- a/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift +++ /dev/null @@ -1,75 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift open source project -// -// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See http://swift.org/LICENSE.txt for license information -// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// -//===----------------------------------------------------------------------===// - -import Basics -import Dispatch -import PackageLoading -import PackageModel - -import struct TSCUtility.Version - -/// TODO: This could be removed once logic to handle provided libraries is integrated -/// into a \c PubGrubPackageContainer. -public struct ProvidedLibraryPackageContainer: PackageContainer { - public let package: PackageReference - - /// Observability scope to emit diagnostics - private let observabilityScope: ObservabilityScope - - public init( - package: PackageReference, - observabilityScope: ObservabilityScope - ) throws { - switch package.kind { - case .providedLibrary: - break - default: - throw InternalError("invalid package type \(package.kind)") - } - self.package = package - self.observabilityScope = observabilityScope.makeChildScope( - description: "ProvidedLibraryPackageContainer", - metadata: package.diagnosticsMetadata) - } - - public func isToolsVersionCompatible(at version: Version) -> Bool { - true - } - - public func toolsVersion(for version: Version) throws -> ToolsVersion { - .v4 - } - - public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] { - return try versionsDescending() - } - - public func versionsAscending() throws -> [Version] { - [] - } - - public func getDependencies(at version: Version, productFilter: ProductFilter) throws -> [PackageContainerConstraint] { - [] - } - - public func getDependencies(at revision: String, productFilter: ProductFilter) throws -> [PackageContainerConstraint] { - [] - } - - public func getUnversionedDependencies(productFilter: ProductFilter) throws -> [PackageContainerConstraint] { - [] - } - - public func loadPackageReference(at boundVersion: BoundVersion) throws -> PackageReference { - package - } -} diff --git a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift index dacf263937d..c20021d3642 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift @@ -98,13 +98,6 @@ final class ContainerProvider { let result = result.tryMap { container -> PubGrubPackageContainer in let pubGrubContainer = PubGrubPackageContainer(underlying: container, pins: self.pins) - // This container is not cached because it's intended to be transparent - // and requested only when forming final assignments. Caching it would - // mean that subsequent calls to `solve` would pick it up. - if case .providedLibrary = package.kind { - return pubGrubContainer - } - // only cache positive results self.containersCache[package] = pubGrubContainer return pubGrubContainer diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index 00f3f1b1013..c2e28f21713 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -895,7 +895,7 @@ extension PackageRequirement { extension PackageReference { public func matchingPrebuiltLibrary(in availableLibraries: [ProvidedLibrary]) -> ProvidedLibrary? { switch self.kind { - case .fileSystem, .localSourceControl, .root, .providedLibrary: + case .fileSystem, .localSourceControl, .root: return nil // can never match a prebuilt library case .registry(let identity): if let registryIdentity = identity.registry { diff --git a/Sources/PackageLoading/ManifestJSONParser.swift b/Sources/PackageLoading/ManifestJSONParser.swift index 3818c565828..564e5991982 100644 --- a/Sources/PackageLoading/ManifestJSONParser.swift +++ b/Sources/PackageLoading/ManifestJSONParser.swift @@ -83,7 +83,7 @@ enum ManifestJSONParser { case .localSourceControl(let _packagePath): // we have a more accurate path than the virtual one packagePath = _packagePath - case .root(let _packagePath), .fileSystem(let _packagePath), .providedLibrary(_, let _packagePath): + case .root(let _packagePath), .fileSystem(let _packagePath): // we dont have a more accurate path, and they should be the same // asserting (debug only) to make sure refactoring is correct 11/2023 assert(packagePath == _packagePath, "expecting package path '\(packagePath)' to be the same as '\(_packagePath)'") diff --git a/Sources/PackageModel/IdentityResolver.swift b/Sources/PackageModel/IdentityResolver.swift index 9755d7e26f3..847836ecf2d 100644 --- a/Sources/PackageModel/IdentityResolver.swift +++ b/Sources/PackageModel/IdentityResolver.swift @@ -46,8 +46,6 @@ public struct DefaultIdentityResolver: IdentityResolver { return try self.resolveIdentity(for: url) case .registry(let identity): return identity - case .providedLibrary(let url, _): - return try self.resolveIdentity(for: url) } } diff --git a/Sources/PackageModel/PackageReference.swift b/Sources/PackageModel/PackageReference.swift index 545a0f6ade8..d4cec83d3ac 100644 --- a/Sources/PackageModel/PackageReference.swift +++ b/Sources/PackageModel/PackageReference.swift @@ -34,9 +34,6 @@ public struct PackageReference { /// A package from a registry. case registry(PackageIdentity) - /// A prebuilt library provided by a toolchain for a package identified by the given "origin" URL. - case providedLibrary(SourceControlURL, AbsolutePath) - // FIXME: we should not need this once we migrate off URLs //@available(*, deprecated) public var locationString: String { @@ -52,8 +49,6 @@ public struct PackageReference { case .registry(let identity): // FIXME: this is a placeholder return identity.description - case .providedLibrary(let url, _): - return url.absoluteString } } @@ -75,8 +70,6 @@ public struct PackageReference { return "remoteSourceControl \(url)" case .registry(let identity): return "registry \(identity)" - case .providedLibrary(let url, let path): - return "provided library for \(url) @ \(path)" } } @@ -131,8 +124,6 @@ public struct PackageReference { case .registry(let identity): // FIXME: this is a placeholder self.deprecatedName = name ?? identity.description - case .providedLibrary(let url, _): - self.deprecatedName = name ?? PackageIdentityParser.computeDefaultName(fromURL: url) } } @@ -160,14 +151,6 @@ public struct PackageReference { public static func registry(identity: PackageIdentity) -> PackageReference { PackageReference(identity: identity, kind: .registry(identity)) } - - public static func providedLibrary( - identity: PackageIdentity, - origin: SourceControlURL, - path: AbsolutePath - ) -> PackageReference { - PackageReference(identity: identity, kind: .providedLibrary(origin, path)) - } } extension PackageReference: Equatable { @@ -187,11 +170,6 @@ extension PackageReference: Equatable { switch (self.kind, other.kind) { case (.remoteSourceControl(let lurl), .remoteSourceControl(let rurl)): return lurl.canonicalURL == rurl.canonicalURL - case (.remoteSourceControl(let lurl), .providedLibrary(let rurl, _)), - (.providedLibrary(let lurl, _), .remoteSourceControl(let rurl)): - return lurl.canonicalURL == rurl.canonicalURL - case (.providedLibrary(_, let lpath), .providedLibrary(_, let rpath)): - return lpath == rpath default: return true } @@ -246,10 +224,6 @@ extension PackageReference.Kind: Encodable { case .registry: var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .registry) try unkeyedContainer.encode(self.isRoot) - case .providedLibrary(let url, let path): - var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .providedLibrary) - try unkeyedContainer.encode(url) - try unkeyedContainer.encode(path) } } } diff --git a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift index 9f84bbf9598..0ea946cb38e 100644 --- a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift +++ b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift @@ -240,8 +240,6 @@ internal struct PluginContextSerializer { return .repository(url: url.absoluteString, displayVersion: String(describing: package.manifest.version), scmRevision: String(describing: package.manifest.revision)) case .registry(let identity): return .registry(identity: identity.description, displayVersion: String(describing: package.manifest.version)) - case .providedLibrary(_, _): - throw InternalError("provided libraries are not supported in plugin context") } } diff --git a/Sources/SPMTestSupport/MockManifestLoader.swift b/Sources/SPMTestSupport/MockManifestLoader.swift index 70da34b5cc6..fc1f345ed64 100644 --- a/Sources/SPMTestSupport/MockManifestLoader.swift +++ b/Sources/SPMTestSupport/MockManifestLoader.swift @@ -109,9 +109,6 @@ extension ManifestLoader { packageIdentity = identity // FIXME: placeholder packageLocation = identity.description - case .providedLibrary(let url, let path): - packageIdentity = try identityResolver.resolveIdentity(for: url) - packageLocation = path.pathString } return try await self.load( manifestPath: manifestPath, @@ -159,9 +156,6 @@ extension ManifestLoader { packageIdentity = identity // FIXME: placeholder packageLocation = identity.description - case .providedLibrary(let url, let path): - packageIdentity = try identityResolver.resolveIdentity(for: url) - packageLocation = path.pathString } return try await self.load( packagePath: packagePath, diff --git a/Sources/SPMTestSupport/MockWorkspace.swift b/Sources/SPMTestSupport/MockWorkspace.swift index a09adf4079d..f2f24792322 100644 --- a/Sources/SPMTestSupport/MockWorkspace.swift +++ b/Sources/SPMTestSupport/MockWorkspace.swift @@ -1035,8 +1035,6 @@ extension PackageReference.Kind { return "remoteSourceControl" case .registry: return "registry" - case .providedLibrary: - return "library" } } } diff --git a/Sources/Workspace/Diagnostics.swift b/Sources/Workspace/Diagnostics.swift index aef1442693b..8bf918875bc 100644 --- a/Sources/Workspace/Diagnostics.swift +++ b/Sources/Workspace/Diagnostics.swift @@ -183,7 +183,7 @@ extension Basics.Diagnostic { switch $0.kind { case .registry(let identity): return "'\(identity.description)'" - case .remoteSourceControl(let url), .providedLibrary(let url, _): + case .remoteSourceControl(let url): return "'\($0.identity)' from \(url)" case .localSourceControl(let path), .fileSystem(let path), .root(let path): return "'\($0.identity)' at \(path)" diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index f8b3f45b575..5d494ba0252 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -33,7 +33,6 @@ import struct PackageGraph.ObservabilityDependencyResolverDelegate import struct PackageGraph.PackageContainerConstraint import struct PackageGraph.PackageGraphRoot import struct PackageGraph.PackageGraphRootInput -import struct PackageGraph.ProvidedLibraryPackageContainer import class PackageGraph.PinsStore import struct PackageGraph.PubGrubDependencyResolver import struct PackageGraph.Term diff --git a/Sources/Workspace/Workspace+PackageContainer.swift b/Sources/Workspace/Workspace+PackageContainer.swift index ca5e857b343..4787e3adef4 100644 --- a/Sources/Workspace/Workspace+PackageContainer.swift +++ b/Sources/Workspace/Workspace+PackageContainer.swift @@ -17,7 +17,6 @@ import enum PackageFingerprint.FingerprintCheckingMode import enum PackageGraph.ContainerUpdateStrategy import protocol PackageGraph.PackageContainer import protocol PackageGraph.PackageContainerProvider -import struct PackageGraph.ProvidedLibraryPackageContainer import struct PackageModel.PackageReference // MARK: - Package container provider @@ -94,14 +93,6 @@ extension Workspace: PackageContainerProvider { queue.async { completion(.success(container)) } - - case .providedLibrary: - let container = try ProvidedLibraryPackageContainer( - package: package, - observabilityScope: observabilityScope) - queue.async { - completion(.success(container)) - } } } catch { queue.async { diff --git a/Sources/Workspace/Workspace+State.swift b/Sources/Workspace/Workspace+State.swift index 8d827cb562f..4c4e971b16f 100644 --- a/Sources/Workspace/Workspace+State.swift +++ b/Sources/Workspace/Workspace+State.swift @@ -433,7 +433,6 @@ extension WorkspaceStateStorage { let kind: Kind let location: String let name: String - let originURL: String? init(_ reference: PackageModel.PackageReference) { self.identity = reference.identity.description @@ -441,28 +440,19 @@ extension WorkspaceStateStorage { case .root(let path): self.kind = .root self.location = path.pathString - self.originURL = nil case .fileSystem(let path): self.kind = .fileSystem self.location = path.pathString - self.originURL = nil case .localSourceControl(let path): self.kind = .localSourceControl self.location = path.pathString - self.originURL = nil case .remoteSourceControl(let url): self.kind = .remoteSourceControl self.location = url.absoluteString - self.originURL = nil case .registry: self.kind = .registry // FIXME: placeholder self.location = self.identity.description - self.originURL = nil - case .providedLibrary(let url, let path): - self.kind = .providedLibrary - self.originURL = url.absoluteString - self.location = path.pathString } self.name = reference.deprecatedName } @@ -473,7 +463,6 @@ extension WorkspaceStateStorage { case localSourceControl case remoteSourceControl case registry - case providedLibrary } } } @@ -516,14 +505,6 @@ extension PackageModel.PackageReference { kind = .remoteSourceControl(SourceControlURL(reference.location)) case .registry: kind = .registry(identity) - case .providedLibrary: - guard let url = reference.originURL else { - throw InternalError("Cannot form provided library reference without origin: \(reference)") - } - kind = try .providedLibrary( - SourceControlURL(url), - .init(validating: reference.location) - ) } self.init( @@ -791,7 +772,6 @@ extension WorkspaceStateStorage { let kind: Kind let location: String let name: String - let originURL: String? init(_ reference: PackageModel.PackageReference) { self.identity = reference.identity.description @@ -799,28 +779,19 @@ extension WorkspaceStateStorage { case .root(let path): self.kind = .root self.location = path.pathString - self.originURL = nil case .fileSystem(let path): self.kind = .fileSystem self.location = path.pathString - self.originURL = nil case .localSourceControl(let path): self.kind = .localSourceControl self.location = path.pathString - self.originURL = nil case .remoteSourceControl(let url): self.kind = .remoteSourceControl self.location = url.absoluteString - self.originURL = nil case .registry: self.kind = .registry // FIXME: placeholder self.location = self.identity.description - self.originURL = nil - case .providedLibrary(let url, let path): - self.kind = .providedLibrary - self.originURL = url.absoluteString - self.location = path.pathString } self.name = reference.deprecatedName } @@ -831,7 +802,6 @@ extension WorkspaceStateStorage { case localSourceControl case remoteSourceControl case registry - case providedLibrary } } } @@ -875,14 +845,6 @@ extension PackageModel.PackageReference { kind = .remoteSourceControl(SourceControlURL(reference.location)) case .registry: kind = .registry(identity) - case .providedLibrary: - guard let url = reference.originURL else { - throw InternalError("Cannot form a provided library reference without origin: \(reference)") - } - kind = try .providedLibrary( - SourceControlURL(url), - .init(validating: reference.location) - ) } self.init( diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index c27e9c5f523..26bcd60543c 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -1262,8 +1262,6 @@ extension Workspace { try self.removeRepository(dependency: dependencyToRemove) case .registry: try self.removeRegistryArchive(for: dependencyToRemove) - case .providedLibrary: - break // NOOP } // Save the state. diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift index 5e0f7e21ad4..850ab22a48b 100644 --- a/Tests/PackageGraphTests/PubgrubTests.swift +++ b/Tests/PackageGraphTests/PubgrubTests.swift @@ -3554,18 +3554,6 @@ public struct MockProvider: PackageContainerProvider { ) -> Void ) { queue.async { - if case .providedLibrary(_, _) = package.kind { - do { - let container = try ProvidedLibraryPackageContainer( - package: package, - observabilityScope: observabilityScope - ) - return completion(.success(container)) - } catch { - return completion(.failure(error)) - } - } - completion( self.containersByIdentifier[package].map { .success($0) } ?? .failure(_MockLoadingError.unknownModule) diff --git a/Tests/PackageLoadingTests/PDLoadingTests.swift b/Tests/PackageLoadingTests/PDLoadingTests.swift index b8b04ef0e91..aa2bfd7af44 100644 --- a/Tests/PackageLoadingTests/PDLoadingTests.swift +++ b/Tests/PackageLoadingTests/PDLoadingTests.swift @@ -97,8 +97,6 @@ class PackageDescriptionLoadingTests: XCTestCase, ManifestLoaderDelegate { packagePath = path case .remoteSourceControl, .registry: packagePath = .root - case .providedLibrary(_, let path): - packagePath = path } let toolsVersion = toolsVersion From 39177684470fdec049616c7e04b101660931c49f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 25 Apr 2024 00:06:00 -0700 Subject: [PATCH 15/16] [Tests] NFC: Add build plan tests for provided libraries --- Tests/BuildTests/BuildPlanTests.swift | 108 ++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 6ce71ad87e5..24a4570325d 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -6260,4 +6260,112 @@ final class BuildPlanTests: XCTestCase { let dylibs = Array(buildProduct.dylibs.map({$0.product.name})).sorted() XCTAssertEqual(dylibs, ["BarLogging", "FooLogging"]) } + + func testSwiftPackageWithProvidedLibraries() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/A/Sources/ATarget/main.swift", + "/Libraries/B/BTarget.swiftmodule", + "/Libraries/C/CTarget.swiftmodule" + ) + + let observability = ObservabilitySystem.makeForTesting() + let graph = try loadModulesGraph( + fileSystem: fs, + manifests: [ + Manifest.createRootManifest( + displayName: "A", + path: "/A", + dependencies: [ + .localSourceControl(path: "/B", requirement: .upToNextMajor(from: "1.0.0")), + .localSourceControl(path: "/C", requirement: .upToNextMajor(from: "1.0.0")), + ], + products: [ + ProductDescription( + name: "A", + type: .executable, + targets: ["ATarget"] + ) + ], + targets: [ + TargetDescription(name: "ATarget", dependencies: ["BLibrary", "CLibrary"]) + ] + ), + Manifest.createFileSystemManifest( + displayName: "B", + path: "/B", + products: [ + ProductDescription(name: "BLibrary", type: .library(.automatic), targets: ["BTarget"]), + ], + targets: [ + TargetDescription( + name: "BTarget", + path: "/Libraries/B", + type: .providedLibrary + ) + ] + ), + Manifest.createFileSystemManifest( + displayName: "C", + path: "/C", + products: [ + ProductDescription(name: "CLibrary", type: .library(.automatic), targets: ["CTarget"]), + ], + targets: [ + TargetDescription( + name: "CTarget", + path: "/Libraries/C", + type: .providedLibrary + ) + ] + ), + ], + observabilityScope: observability.topScope + ) + + XCTAssertNoDiagnostics(observability.diagnostics) + + let plan = try BuildPlan( + buildParameters: mockBuildParameters(), + graph: graph, + fileSystem: fs, + observabilityScope: observability.topScope + ) + let result = try BuildPlanResult(plan: plan) + + result.checkProductsCount(1) + result.checkTargetsCount(1) + + XCTAssertMatch( + try result.target(for: "ATarget").swiftTarget().compileArguments(), + [ + .anySequence, + "-I", "/Libraries/C", + "-I", "/Libraries/B", + .anySequence + ] + ) + + let linkerArgs = try result.buildProduct(for: "A").linkArguments() + + XCTAssertMatch( + linkerArgs, + [ + .anySequence, + "-L", "/Libraries/B", + "-l", "BTarget", + .anySequence + ] + ) + + XCTAssertMatch( + linkerArgs, + [ + .anySequence, + "-L", "/Libraries/C", + "-l", "CTarget", + .anySequence + ] + ) + } } From 5b01dde835d3c755767801b495226cfb3c5dd179 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 25 Apr 2024 13:52:03 -0700 Subject: [PATCH 16/16] [Workspace] Augment resolution based on Package.resolved to support provided libraries If a package with a provided library is mentioned in the Package.resolved let's check whether the version of the library matches pinned version of the package and is so, switch to the library. --- Sources/Workspace/Workspace+Dependencies.swift | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 5d494ba0252..2e99b7d2107 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -371,6 +371,20 @@ extension Workspace { // automatically manage the parallelism. let group = DispatchGroup() for pin in pinsStore.pins.values { + // Provided library doesn't have a container, we need to inject a special depedency. + if let library = pin.packageRef.matchingPrebuiltLibrary(in: self.providedLibraries), + case .version(library.version, _) = pin.state + { + try self.state.dependencies.add( + .providedLibrary( + packageRef: pin.packageRef, + library: library + ) + ) + try self.state.save() + continue + } + group.enter() let observabilityScope = observabilityScope.makeChildScope( description: "requesting package containers", @@ -418,7 +432,9 @@ extension Workspace { return !pin.state.equals(checkoutState) case .registryDownload(let version): return !pin.state.equals(version) - case .edited, .fileSystem, .providedLibrary, .custom: + case .providedLibrary: + return false + case .edited, .fileSystem, .custom: return true } }