Skip to content

Improvements to prebuilt or provided library handling #7496

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 16 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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 @@ -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] = []

Expand Down Expand Up @@ -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()]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -279,8 +279,8 @@ package final class BuildOperation: PackageStructureDelegate, SPMBuildCore.Build
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 @@ -115,6 +115,8 @@ extension BuildPlan {
}
buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths

buildProduct.providedLibraries = dependencies.providedLibraries

buildProduct.availableTools = dependencies.availableTools
}

Expand All @@ -127,6 +129,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 @@ -204,6 +207,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 @@ -257,6 +261,8 @@ extension BuildPlan {
}
case .plugin:
continue
case .providedLibrary:
providedLibraries[target.name] = target.underlying.path
}

case .product(let product, _):
Expand All @@ -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
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 @@ -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)")
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 @@ -74,7 +74,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 @@ -73,7 +73,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 @@ -153,6 +153,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 @@ -607,7 +607,6 @@ package 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 @@ -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 {
Expand Down Expand Up @@ -160,7 +159,6 @@ extension ModulesGraph {
unsafeAllowedPackages: unsafeAllowedPackages,
platformRegistry: customPlatformsRegistry ?? .default,
platformVersionProvider: platformVersionProvider,
availableLibraries: availableLibraries,
fileSystem: fileSystem,
observabilityScope: observabilityScope
)
Expand Down Expand Up @@ -250,7 +248,6 @@ private func createResolvedPackages(
unsafeAllowedPackages: Set<PackageReference>,
platformRegistry: PlatformRegistry,
platformVersionProvider: PlatformVersionProvider,
availableLibraries: [LibraryMetadata],
fileSystem: FileSystem,
observabilityScope: ObservabilityScope
) throws -> [ResolvedPackage] {
Expand Down Expand Up @@ -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
}
Expand Down
1 change: 0 additions & 1 deletion Sources/PackageGraph/ModulesGraph.swift
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,6 @@ public func loadModulesGraph(
shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
createREPLProduct: createREPLProduct,
customXCTestMinimumDeploymentTargets: customXCTestMinimumDeploymentTargets,
availableLibraries: [],
fileSystem: fileSystem,
observabilityScope: observabilityScope
)
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