Skip to content

[6.0] Improvements to prebuilt/provided library handling #7713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9e40ceb
[PackageGraph] Remove uses of `availableLibraries` from ModulesGraph.…
xedin Apr 15, 2024
e207faa
Add new `providedLibrary` package reference kind
xedin Apr 16, 2024
2f6f8e7
Expanded `providedLibrary` PackageReference to include origin
xedin Apr 16, 2024
14cda08
Augment ManagedDependency to handle provided libraries
xedin Apr 16, 2024
a2b7d16
[PackageModel] Add a new target kind - provided library
xedin Apr 17, 2024
7dd056c
Package LibraryMetadata together with its toolchain location
xedin Apr 18, 2024
18ba6d0
[Resolution/Loading] Handle packages backed by a provided library
xedin Apr 18, 2024
0f582d4
[PubGrub] Expand provided library resolution tests
xedin Apr 19, 2024
44b5c0c
Remove all extraneous uses of provided libraries throughout code base
xedin Apr 22, 2024
47e30cf
Switch ProvidedLibrary.vesion to produce a `Version`
xedin Apr 23, 2024
33d2e2c
[PubGrub] Note that selected package version is backed by a provided …
xedin Apr 24, 2024
57bc1ce
[Workspace] NFC: Handle loading of provided library managed dependenc…
xedin Apr 24, 2024
8b86f63
[Workspace] Introduce a new package state - `usesLibrary`
xedin Apr 24, 2024
51eafa7
NFC: Remove `.providedLibrary` from PackageReference and its package …
xedin Apr 25, 2024
165881c
[Tests] NFC: Add build plan tests for provided libraries
xedin Apr 25, 2024
7b2c33d
[Workspace] Augment resolution based on Package.resolved to support p…
xedin Apr 25, 2024
270fcc0
Fix build error after merge
MaxDesiatov Jun 24, 2024
cad8355
UserToolchain: Unify handling of confg.json and provided-libraries.json
xedin Mar 29, 2024
03b6f39
Resolve merge conflicts
MaxDesiatov Jun 25, 2024
a43baa7
Fix build error in SourceKit-LSP after merge
MaxDesiatov Jun 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
// 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] = []

Expand Down Expand Up @@ -157,6 +160,8 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
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()]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,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 && !prepareForIndexing {
Expand Down
6 changes: 3 additions & 3 deletions Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS

// 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.
Expand All @@ -285,8 +285,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
Self.didEmitUnexpressedDependencies = true

let availableFrameworks = Dictionary<String, PackageIdentity>(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
}
Expand Down
8 changes: 7 additions & 1 deletion Sources/Build/BuildPlan/BuildPlan+Product.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ extension BuildPlan {
}
buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths

buildProduct.providedLibraries = dependencies.providedLibraries

buildProduct.availableTools = dependencies.availableTools
}

Expand All @@ -128,6 +130,7 @@ extension BuildPlan {
staticTargets: [ResolvedModule],
systemModules: [ResolvedModule],
libraryBinaryPaths: Set<AbsolutePath>,
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
Expand Down Expand Up @@ -205,6 +208,7 @@ extension BuildPlan {
var staticTargets = [ResolvedModule]()
var systemModules = [ResolvedModule]()
var libraryBinaryPaths: Set<AbsolutePath> = []
var providedLibraries = [String: AbsolutePath]()
var availableTools = [String: AbsolutePath]()

for dependency in allTargets {
Expand Down Expand Up @@ -258,6 +262,8 @@ extension BuildPlan {
}
case .plugin:
continue
case .providedLibrary:
providedLibraries[target.name] = target.underlying.path
}

case .product(let product, _):
Expand All @@ -275,7 +281,7 @@ extension BuildPlan {
}
}

return (linkLibraries, staticTargets, systemModules, libraryBinaryPaths, availableTools)
return (linkLibraries, staticTargets, systemModules, libraryBinaryPaths, providedLibraries, availableTools)
}

/// Extracts the artifacts from an artifactsArchive
Expand Down
5 changes: 5 additions & 0 deletions Sources/Build/BuildPlan/BuildPlan+Swift.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -48,6 +49,10 @@ extension BuildPlan {
swiftTarget.libraryBinaryPaths.insert(library.libraryPath)
}
}
case let target as ProvidedLibraryTarget:
swiftTarget.additionalFlags += [
"-I", target.path.pathString
]
default:
break
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildPlan/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,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)")
Expand Down
1 change: 0 additions & 1 deletion Sources/Commands/PackageCommands/EditCommands.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ extension SwiftPackageCommand {
packageName: packageName,
forceRemove: shouldForceRemove,
root: swiftCommandState.getWorkspaceRoot(),
availableLibraries: swiftCommandState.getHostToolchain().providedLibraries,
observabilityScope: swiftCommandState.observabilityScope
)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Commands/PackageCommands/Update.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ extension SwiftPackageCommand {
case .removed:
report += "\n"
report += "- \(package.identity) \(currentVersion)"
case .unchanged:
case .unchanged, .usesLibrary:
continue
}
}
Expand Down
2 changes: 2 additions & 0 deletions Sources/Commands/Snippets/Cards/TopCard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ fileprivate extension Target.Kind {
return "snippets"
case .macro:
return "macros"
case .providedLibrary:
return "provided libraries"
}
}
}
1 change: 0 additions & 1 deletion Sources/CoreCommands/SwiftCommandState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,6 @@ public final class SwiftCommandState {
explicitProduct: explicitProduct,
forceResolvedVersions: options.resolver.forceResolvedVersions,
testEntryPointPath: testEntryPointPath,
availableLibraries: self.getHostToolchain().providedLibraries,
observabilityScope: self.observabilityScope
)

Expand Down
5 changes: 3 additions & 2 deletions Sources/PackageGraph/BoundVersion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//

import struct PackageModel.ProvidedLibrary
import struct TSCUtility.Version

/// A bound version for a package within an assignment.
Expand All @@ -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
Expand All @@ -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"
Expand Down
45 changes: 18 additions & 27 deletions Sources/PackageGraph/ModulesGraph+Loading.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ extension ModulesGraph {
customPlatformsRegistry: PlatformRegistry? = .none,
customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none,
testEntryPointPath: AbsolutePath? = nil,
availableLibraries: [LibraryMetadata],
fileSystem: FileSystem,
observabilityScope: ObservabilityScope
) throws -> ModulesGraph {
Expand Down Expand Up @@ -172,7 +171,6 @@ extension ModulesGraph {
unsafeAllowedPackages: unsafeAllowedPackages,
platformRegistry: customPlatformsRegistry ?? .default,
platformVersionProvider: platformVersionProvider,
availableLibraries: availableLibraries,
fileSystem: fileSystem,
observabilityScope: observabilityScope
)
Expand Down Expand Up @@ -272,7 +270,6 @@ private func createResolvedPackages(
unsafeAllowedPackages: Set<PackageReference>,
platformRegistry: PlatformRegistry,
platformVersionProvider: PlatformVersionProvider,
availableLibraries: [LibraryMetadata],
fileSystem: FileSystem,
observabilityScope: ObservabilityScope
) throws -> IdentifiableSet<ResolvedPackage> {
Expand Down Expand Up @@ -551,32 +548,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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ final class ContainerProvider {
) { result in
let result = result.tryMap { container -> PubGrubPackageContainer in
let pubGrubContainer = PubGrubPackageContainer(underlying: container, pins: self.pins)

// only cache positive results
self.containersCache[package] = pubGrubContainer
return pubGrubContainer
Expand All @@ -107,12 +108,9 @@ final class ContainerProvider {
}

/// Starts prefetching the given containers.
func prefetch(containers identifiers: [PackageReference], availableLibraries: [LibraryMetadata]) {
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()
Expand Down
Loading