Skip to content

Fix crash in macros caused by mixing prebuilts and source. (#9261) #9264

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix crash in macros caused by mixing prebuilts and source. (#9261) #9264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
129 changes: 106 additions & 23 deletions Sources/PackageGraph/ModulesGraph+Loading.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ extension ModulesGraph {
requiredDependencies: [PackageReference] = [],
unsafeAllowedPackages: Set<PackageReference> = [],
binaryArtifacts: [PackageIdentity: [String: BinaryArtifact]],
prebuilts: [PackageIdentity: [String: PrebuiltLibrary]], // Product name to library mapping
prebuilts: [PackageIdentity: [String: PrebuiltLibrary]], // Package -> Product name -> library
shouldCreateMultipleTestProducts: Bool = false,
createREPLProduct: Bool = false,
customPlatformsRegistry: PlatformRegistry? = .none,
Expand Down Expand Up @@ -167,7 +167,6 @@ extension ModulesGraph {
path: packagePath,
additionalFileRules: additionalFileRules,
binaryArtifacts: binaryArtifacts[node.identity] ?? [:],
prebuilts: prebuilts,
shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
testEntryPointPath: testEntryPointPath,
createREPLProduct: manifest.packageKind.isRoot ? createREPLProduct : false,
Expand Down Expand Up @@ -214,7 +213,7 @@ extension ModulesGraph {
let rootPackages = resolvedPackages.filter { root.manifests.values.contains($0.manifest) }
checkAllDependenciesAreUsed(
packages: resolvedPackages,
rootPackages,
rootPackages: rootPackages,
prebuilts: prebuilts,
observabilityScope: observabilityScope
)
Expand All @@ -231,7 +230,7 @@ extension ModulesGraph {

private func checkAllDependenciesAreUsed(
packages: IdentifiableSet<ResolvedPackage>,
_ rootPackages: [ResolvedPackage],
rootPackages: [ResolvedPackage],
prebuilts: [PackageIdentity: [String: PrebuiltLibrary]],
observabilityScope: ObservabilityScope
) {
Expand Down Expand Up @@ -311,7 +310,7 @@ private func checkAllDependenciesAreUsed(
// We check if any of the products of this dependency is guarded by a trait.
let traitGuarded = traitGuardedProductDependencies.contains(product.name)
// Consider prebuilts as used
let prebuilt = prebuilts[dependency.identity]?.keys.contains(product.name) ?? false
let prebuilt = prebuilts[dependency.identity]?[product.name] != nil

return usedByPackage || traitGuarded || prebuilt
}
Expand Down Expand Up @@ -729,24 +728,6 @@ private func createResolvedPackages(

// Establish product dependencies.
for case .product(let productRef, let conditions) in moduleBuilder.module.dependencies {
if let package = productRef.package, prebuilts[.plain(package)]?[productRef.name] != nil {
// See if we're using a prebuilt instead
if moduleBuilder.module.type == .macro {
continue
} else if moduleBuilder.module.type == .test {
// use prebuilt if this is a test that depends a macro target
// these are guaranteed built for host
if moduleBuilder.module.dependencies.contains(where: { dep in
guard let module = dep.module else {
return false
}
return module.type == .macro
}) {
continue
}
}
}

// Find the product in this package's dependency products.
// Look it up by ID if module aliasing is used, otherwise by name.
let product = lookupByProductIDs ? productDependencyMap[productRef.identity] :
Expand Down Expand Up @@ -800,6 +781,91 @@ private func createResolvedPackages(
}
}

// Prebuilts
for packageBuilder in packageBuilders {
for moduleBuilder in packageBuilder.modules {
// Currently we only support prebuilts for macros and their tests
switch moduleBuilder.module.type {
case .macro:
// Skip if the module has a dependency that depends on a prebuilt
// This is causing a mix of release and debug modes which is causing
// macros to crash.
// TODO: Find a way to build those dependencies with the prebuilts on host
if moduleBuilder.dependencies.contains(where: {
switch $0 {
case .module(let moduleBuilder, conditions: _):
return moduleBuilder.recursiveDependenciesContains(where: {
switch $0 {
case .product(let productBuilder, conditions: _):
return prebuilts[productBuilder.packageBuilder.package.identity]?[productBuilder.product.name] != nil
case .module:
return false
}
})
case .product(let productBuilder, conditions: _):
return productBuilder.moduleBuilders.contains(where: {
$0.recursiveDependenciesContains(where: {
switch $0 {
case .product(let productBuilder, conditions: _):
return prebuilts[productBuilder.packageBuilder.package.identity]?[productBuilder.product.name] != nil
case .module:
return false
}
})
})
}
}) {
// Skip
continue
}
case .test:
// Check for macro tests
if !moduleBuilder.dependencies.contains(where: {
switch $0 {
case .module(let depModuleBuilder, conditions: _):
return depModuleBuilder.module.type == .macro
case .product:
return false
}
}) {
// Skip
continue
}
default:
// Skip
continue
}

var prebuiltLibraries: [PrebuiltLibrary] = []
for dep in moduleBuilder.dependencies {
switch dep {
case .product(let productBuilder, conditions: _):
if let prebuilt = prebuilts[productBuilder.packageBuilder.package.identity]?[productBuilder.product.name] {
// Use the prebuilt
if !prebuiltLibraries.contains(where: { $0.libraryName == prebuilt.libraryName }) {
prebuiltLibraries.append(prebuilt)
}
}
case .module:
break
}
}

for prebuiltLibrary in prebuiltLibraries {
moduleBuilder.module.use(prebuiltLibrary: prebuiltLibrary)

moduleBuilder.dependencies = moduleBuilder.dependencies.filter({
switch $0 {
case .product(let productBuilder, conditions: _):
return prebuilts[productBuilder.packageBuilder.package.identity]?[productBuilder.product.name] == nil
case .module:
return true
}
})
}
}
}

// If a module with similar name was encountered before, we emit a diagnostic.
if foundDuplicateModule {
var duplicateModules = [String: [Package]]()
Expand Down Expand Up @@ -1337,6 +1403,23 @@ private final class ResolvedModuleBuilder: ResolvedBuilder<ResolvedModule> {
/// The module dependencies of this module.
var dependencies: [Dependency] = []

func recursiveDependenciesContains(where check: (Dependency) -> Bool) -> Bool {
dependencies.contains(where: {
if check($0) {
return true
} else {
switch $0 {
case .module(let moduleBuilder, conditions: _):
return moduleBuilder.recursiveDependenciesContains(where: check)
case .product(let productBuilder, conditions: _):
return productBuilder.moduleBuilders.contains(where: {
$0.recursiveDependenciesContains(where: check)
})
}
}
})
}

/// The defaultLocalization for this package
var defaultLocalization: String? = nil

Expand Down
83 changes: 0 additions & 83 deletions Sources/PackageLoading/PackageBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,48 +279,6 @@ public struct BinaryArtifact {
}
}

/// A structure representing a prebuilt library to be used instead of a source dependency
public struct PrebuiltLibrary {
/// The package identity.
public let identity: PackageIdentity

/// The name of the binary target the artifact corresponds to.
public let libraryName: String

/// The path to the extracted prebuilt artifacts
public let path: AbsolutePath

/// The path to the checked out source
public let checkoutPath: AbsolutePath?

/// The products in the library
public let products: [String]

/// The include path relative to the checkouts dir
public let includePath: [RelativePath]?

/// The C modules that need their includes directory added to the include path
public let cModules: [String]

public init(
identity: PackageIdentity,
libraryName: String,
path: AbsolutePath,
checkoutPath: AbsolutePath?,
products: [String],
includePath: [RelativePath]? = nil,
cModules: [String] = []
) {
self.identity = identity
self.libraryName = libraryName
self.path = path
self.checkoutPath = checkoutPath
self.products = products
self.includePath = includePath
self.cModules = cModules
}
}

/// Helper for constructing a package following the convention system.
///
/// The 'builder' here refers to the builder pattern and not any build system
Expand Down Expand Up @@ -348,9 +306,6 @@ public final class PackageBuilder {
/// Information concerning the different downloaded or local (archived) binary target artifacts.
private let binaryArtifacts: [String: BinaryArtifact]

/// Prebuilts that may referenced from this package's targets
private let prebuilts: [PackageIdentity: [Product.ID: PrebuiltLibrary]]

/// Create multiple test products.
///
/// If set to true, one test product will be created for each test target.
Expand Down Expand Up @@ -407,7 +362,6 @@ public final class PackageBuilder {
path: AbsolutePath,
additionalFileRules: [FileRuleDescription],
binaryArtifacts: [String: BinaryArtifact],
prebuilts: [PackageIdentity: [String: PrebuiltLibrary]],
shouldCreateMultipleTestProducts: Bool = false,
testEntryPointPath: AbsolutePath? = nil,
warnAboutImplicitExecutableTargets: Bool = true,
Expand All @@ -422,7 +376,6 @@ public final class PackageBuilder {
self.packagePath = path
self.additionalFileRules = additionalFileRules
self.binaryArtifacts = binaryArtifacts
self.prebuilts = prebuilts
self.shouldCreateMultipleTestProducts = shouldCreateMultipleTestProducts
self.testEntryPointPath = testEntryPointPath
self.createREPLProduct = createREPLProduct
Expand Down Expand Up @@ -1407,42 +1360,6 @@ public final class PackageBuilder {
table.add(assignment, for: .SWIFT_ACTIVE_COMPILATION_CONDITIONS)
}

// Add in flags for prebuilts if the target is a macro or a macro test.
// Currently we only support prebuilts for macros.
if target.type == .macro || target.isMacroTest(in: manifest) {
let prebuiltLibraries: [String: PrebuiltLibrary] = target.dependencies.reduce(into: .init()) {
guard case let .product(name: name, package: package, moduleAliases: _, condition: _) = $1,
let package = package,
let prebuilt = prebuilts[.plain(package)]?[name]
else {
return
}

$0[prebuilt.libraryName] = prebuilt
}

for prebuilt in prebuiltLibraries.values {
let lib = prebuilt.path.appending(components: ["lib", "lib\(prebuilt.libraryName).a"]).pathString
var ldFlagsAssignment = BuildSettings.Assignment()
ldFlagsAssignment.values = [lib]
table.add(ldFlagsAssignment, for: .OTHER_LDFLAGS)

var includeDirs: [AbsolutePath] = [prebuilt.path.appending(component: "Modules")]
if let checkoutPath = prebuilt.checkoutPath, let includePath = prebuilt.includePath {
for includeDir in includePath {
includeDirs.append(checkoutPath.appending(includeDir))
}
} else {
for cModule in prebuilt.cModules {
includeDirs.append(prebuilt.path.appending(components: "include", cModule))
}
}
var includeAssignment = BuildSettings.Assignment()
includeAssignment.values = includeDirs.map({ "-I\($0.pathString)" })
table.add(includeAssignment, for: .OTHER_SWIFT_FLAGS)
}
}

return table
}

Expand Down
1 change: 1 addition & 0 deletions Sources/PackageModel/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ add_library(PackageModel
Module/BinaryModule.swift
Module/ClangModule.swift
Module/PluginModule.swift
Module/PrebuiltLibrary.swift
Module/SwiftModule.swift
Module/SystemLibraryModule.swift
Module/Module.swift
Expand Down
38 changes: 37 additions & 1 deletion Sources/PackageModel/Module/Module.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,44 @@ public class Module {
return false
}

public func use(prebuiltLibrary: PrebuiltLibrary) {
dependencies = dependencies.filter({
switch $0 {
case .product(let product, conditions: _):
if let packageName = product.package,
prebuiltLibrary.identity == .plain(packageName) && prebuiltLibrary.products.contains(product.name) {
return false
} else {
return true
}
case .module:
return true
}
})

// Add build settings to use the prebuilts
let lib = prebuiltLibrary.path.appending(components: ["lib", "lib\(prebuiltLibrary.libraryName).a"]).pathString
var ldFlagsAssignment = BuildSettings.Assignment()
ldFlagsAssignment.values = [lib]
buildSettings.add(ldFlagsAssignment, for: .OTHER_LDFLAGS)

var includeDirs: [AbsolutePath] = [prebuiltLibrary.path.appending(component: "Modules")]
if let checkoutPath = prebuiltLibrary.checkoutPath, let includePath = prebuiltLibrary.includePath {
for includeDir in includePath {
includeDirs.append(checkoutPath.appending(includeDir))
}
} else {
for cModule in prebuiltLibrary.cModules {
includeDirs.append(prebuiltLibrary.path.appending(components: "include", cModule))
}
}
var includeAssignment = BuildSettings.Assignment()
includeAssignment.values = includeDirs.map({ "-I\($0.pathString)" })
buildSettings.add(includeAssignment, for: .OTHER_SWIFT_FLAGS)
}

/// The dependencies of this module.
public let dependencies: [Dependency]
public private(set) var dependencies: [Dependency]

/// The language-level module name.
public private(set) var c99name: String
Expand Down
Loading