From ed30d3404da30cf6712b349a17eee1aa23f5da77 Mon Sep 17 00:00:00 2001 From: Doug Schaefer <167107236+dschaefer2@users.noreply.github.com> Date: Thu, 10 Apr 2025 16:55:00 -0400 Subject: [PATCH 1/7] Fix duplicate modulemap errors with macro and plugin deps (#8472) We were including flags to hook up modulemaps and include files to C library dependencies in macros and plugin tools through to the modules that depend on those. This adds the capability to prune the depth first searches through the dependencies to ensure these are skipped when crossing macro and plugin boundaries. --- Sources/Basics/Graph/GraphAlgorithms.swift | 47 +++++++++++++ .../ModuleBuildDescription.swift | 22 +++++++ .../SwiftModuleBuildDescription.swift | 6 ++ Sources/Build/BuildPlan/BuildPlan+Swift.swift | 3 +- Sources/Build/BuildPlan/BuildPlan.swift | 12 ++-- Tests/BuildTests/BuildPlanTests.swift | 66 +++++++++++++++++++ .../BuildTests/BuildPlanTraversalTests.swift | 2 + 7 files changed, 150 insertions(+), 8 deletions(-) diff --git a/Sources/Basics/Graph/GraphAlgorithms.swift b/Sources/Basics/Graph/GraphAlgorithms.swift index 8ccc6038cc0..deee4941985 100644 --- a/Sources/Basics/Graph/GraphAlgorithms.swift +++ b/Sources/Basics/Graph/GraphAlgorithms.swift @@ -131,3 +131,50 @@ public func depthFirstSearch( } } } + +/// Implements a pre-order depth-first search that traverses the whole graph and +/// doesn't distinguish between unique and duplicate nodes. The visitor can abort +/// a path as needed to prune the tree. +/// The method expects the graph to be acyclic but doesn't check that. +/// +/// - Parameters: +/// - nodes: The list of input nodes to sort. +/// - successors: A closure for fetching the successors of a particular node. +/// - onNext: A callback to indicate the node currently being processed +/// including its parent (if any) and its depth. Returns whether to +/// continue down the current path. +/// +/// - Complexity: O(v + e) where (v, e) are the number of vertices and edges +/// reachable from the input nodes via the relation. +public enum DepthFirstContinue { + case `continue` + case abort +} + +public func depthFirstSearch( + _ nodes: [T], + successors: (T) throws -> [T], + visitNext: (T, _ parent: T?) throws -> DepthFirstContinue +) rethrows { + var stack = OrderedSet>() + + for node in nodes { + precondition(stack.isEmpty) + stack.append(TraversalNode(parent: nil, curr: node)) + + while !stack.isEmpty { + let node = stack.removeLast() + + if try visitNext(node.curr, node.parent) == .continue { + for succ in try successors(node.curr) { + stack.append( + TraversalNode( + parent: node.curr, + curr: succ + ) + ) + } + } + } + } +} diff --git a/Sources/Build/BuildDescription/ModuleBuildDescription.swift b/Sources/Build/BuildDescription/ModuleBuildDescription.swift index 8f52d45f370..06c52486045 100644 --- a/Sources/Build/BuildDescription/ModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/ModuleBuildDescription.swift @@ -187,8 +187,30 @@ extension ModuleBuildDescription { var dependencies: [Dependency] = [] plan.traverseDependencies(of: self) { product, _, description in dependencies.append(.product(product, description)) + return .continue } onModule: { module, _, description in dependencies.append(.module(module, description)) + return .continue + } + return dependencies + } + + package func recursiveLinkDependencies(using plan: BuildPlan) -> [Dependency] { + var dependencies: [Dependency] = [] + plan.traverseDependencies(of: self) { product, _, description in + guard product.type != .macro && product.type != .plugin else { + return .abort + } + + dependencies.append(.product(product, description)) + return .continue + } onModule: { module, _, description in + guard module.type != .macro && module.type != .plugin else { + return .abort + } + + dependencies.append(.module(module, description)) + return .continue } return dependencies } diff --git a/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift b/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift index 94e902be0e4..b7e506d3afd 100644 --- a/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift @@ -1042,6 +1042,12 @@ extension SwiftModuleBuildDescription { ModuleBuildDescription.swift(self).dependencies(using: plan) } + package func recursiveLinkDependencies( + using plan: BuildPlan + ) -> [ModuleBuildDescription.Dependency] { + ModuleBuildDescription.swift(self).recursiveLinkDependencies(using: plan) + } + package func recursiveDependencies( using plan: BuildPlan ) -> [ModuleBuildDescription.Dependency] { diff --git a/Sources/Build/BuildPlan/BuildPlan+Swift.swift b/Sources/Build/BuildPlan/BuildPlan+Swift.swift index 7d387f8cfd5..f0c0b256cbb 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Swift.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Swift.swift @@ -19,7 +19,7 @@ import class PackageModel.SystemLibraryModule extension BuildPlan { func plan(swiftTarget: SwiftModuleBuildDescription) throws { // We need to iterate recursive dependencies because Swift compiler needs to see all the targets a target - // depends on. + // builds against for case .module(let dependency, let description) in swiftTarget.recursiveDependencies(using: self) { switch dependency.underlying { case let underlyingTarget as ClangModule where underlyingTarget.type == .library: @@ -53,5 +53,4 @@ extension BuildPlan { } } } - } diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index f9b70e2aead..5bafd2acfeb 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -1153,8 +1153,8 @@ extension BuildPlan { package func traverseDependencies( of description: ModuleBuildDescription, - onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void, - onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> Void + onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> DepthFirstContinue, + onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> DepthFirstContinue ) { var visited = Set() func successors( @@ -1195,16 +1195,16 @@ extension BuildPlan { case .package: [] } - } onNext: { module, _ in + } visitNext: { module, _ in switch module { case .package: - break + return .continue case .product(let product, let destination): - onProduct(product, destination, self.description(for: product, context: destination)) + return onProduct(product, destination, self.description(for: product, context: destination)) case .module(let module, let destination): - onModule(module, destination, self.description(for: module, context: destination)) + return onModule(module, destination, self.description(for: module, context: destination)) } } } diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 96861cba0a3..2c6c3d40a88 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -6842,4 +6842,70 @@ final class BuildPlanTests: XCTestCase { XCTAssertMatch(contents, .regex(#"args: \[.*"-I","/testpackagedep/SomeArtifact.xcframework/macos/Headers".*,"/testpackage/Sources/CLib/lib.c".*]"#)) XCTAssertMatch(contents, .regex(#"args: \[.*"-module-name","SwiftLib",.*"-I","/testpackagedep/SomeArtifact.xcframework/macos/Headers".*]"#)) } + + func testMacroPluginDependencyLeakage() async throws { + // Make sure the include paths from macro and plugin executables don't leak into dependents + let observability = ObservabilitySystem.makeForTesting() + let fs = InMemoryFileSystem(emptyFiles: [ + "/LeakTest/Sources/CLib/include/Clib.h", + "/LeakTest/Sources/CLib/Clib.c", + "/LeakTest/Sources/MyMacro/MyMacro.swift", + "/LeakTest/Sources/MyPluginTool/MyPluginTool.swift", + "/LeakTest/Plugins/MyPlugin/MyPlugin.swift", + "/LeakTest/Sources/MyLib/MyLib.swift", + "/LeakLib/Sources/CLib2/include/Clib.h", + "/LeakLib/Sources/CLib2/Clib.c", + "/LeakLib/Sources/MyMacro2/MyMacro.swift", + "/LeakLib/Sources/MyPluginTool2/MyPluginTool.swift", + "/LeakLib/Plugins/MyPlugin2/MyPlugin.swift", + "/LeakLib/Sources/MyLib2/MyLib.swift" + ]) + + let graph = try loadModulesGraph(fileSystem: fs, manifests: [ + Manifest.createFileSystemManifest( + displayName: "LeakLib", + path: "/LeakLib", + products: [ + ProductDescription(name: "MyLib2", type: .library(.automatic), targets: ["MyLib2"]), + ], + targets: [ + TargetDescription(name: "CLib2"), + TargetDescription(name: "MyMacro2", dependencies: ["CLib2"], type: .macro), + TargetDescription(name: "MyPluginTool2", dependencies: ["CLib2"], type: .executable), + TargetDescription(name: "MyPlugin2", dependencies: ["MyPluginTool2"], type: .plugin, pluginCapability: .buildTool), + TargetDescription(name: "MyLib2", dependencies: ["CLib2", "MyMacro2"], pluginUsages: [.plugin(name: "MyPlugin2", package: nil)]), + ] + ), + Manifest.createRootManifest( + displayName: "LeakTest", + path: "/LeakTest", + dependencies: [ + .fileSystem(path: "/LeakLib") + ], + targets: [ + TargetDescription(name: "CLib"), + TargetDescription(name: "MyMacro", dependencies: ["CLib"], type: .macro), + TargetDescription(name: "MyPluginTool", dependencies: ["CLib"], type: .executable), + TargetDescription(name: "MyPlugin", dependencies: ["MyPluginTool"], type: .plugin, pluginCapability: .buildTool), + TargetDescription( + name: "MyLib", + dependencies: ["CLib", "MyMacro", .product(name: "MyLib2", package: "LeakLib")], + pluginUsages: [.plugin(name: "MyPlugin", package: nil)] + ), + ] + ) + ], observabilityScope: observability.topScope) + XCTAssertNoDiagnostics(observability.diagnostics) + + let plan = try await mockBuildPlan( + graph: graph, + fileSystem: fs, + observabilityScope: observability.topScope + ) + XCTAssertNoDiagnostics(observability.diagnostics) + + let myLib = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyLib" })).swift() + print(myLib.additionalFlags) + XCTAssertFalse(myLib.additionalFlags.contains(where: { $0.contains("-tool/include")}), "flags shouldn't contain tools items") + } } diff --git a/Tests/BuildTests/BuildPlanTraversalTests.swift b/Tests/BuildTests/BuildPlanTraversalTests.swift index 2b898cabd76..ed469887d60 100644 --- a/Tests/BuildTests/BuildPlanTraversalTests.swift +++ b/Tests/BuildTests/BuildPlanTraversalTests.swift @@ -146,8 +146,10 @@ final class BuildPlanTraversalTests: XCTestCase { XCTAssertEqual(product.name, "SwiftSyntax") XCTAssertEqual(destination, .host) XCTAssertNil(description) + return .continue } onModule: { module, destination, description in moduleDependencies.append((module, destination, description)) + return .continue } XCTAssertEqual(moduleDependencies.count, 2) From 1eee90418698be0da786f664a721493a0748f344 Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Mon, 14 Apr 2025 15:54:54 -0400 Subject: [PATCH 2/7] Apply the real fix --- Sources/Build/BuildPlan/BuildPlan+Swift.swift | 2 +- Tests/BuildTests/BuildPlanTests.swift | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan+Swift.swift b/Sources/Build/BuildPlan/BuildPlan+Swift.swift index f0c0b256cbb..51a9ad4679d 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Swift.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Swift.swift @@ -20,7 +20,7 @@ extension BuildPlan { func plan(swiftTarget: SwiftModuleBuildDescription) throws { // We need to iterate recursive dependencies because Swift compiler needs to see all the targets a target // builds against - for case .module(let dependency, let description) in swiftTarget.recursiveDependencies(using: self) { + for case .module(let dependency, let description) in swiftTarget.recursiveLinkDependencies(using: self) { switch dependency.underlying { case let underlyingTarget as ClangModule where underlyingTarget.type == .library: guard case let .clang(target)? = description else { diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 2c6c3d40a88..1bc3096bfdf 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -6905,7 +6905,6 @@ final class BuildPlanTests: XCTestCase { XCTAssertNoDiagnostics(observability.diagnostics) let myLib = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyLib" })).swift() - print(myLib.additionalFlags) - XCTAssertFalse(myLib.additionalFlags.contains(where: { $0.contains("-tool/include")}), "flags shouldn't contain tools items") + XCTAssertFalse(myLib.additionalFlags.contains(where: { $0.contains("-tool")}), "flags shouldn't contain tools items") } } From f5c441329051838a43e3f5df4bac4db54a6946fe Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Mon, 21 Apr 2025 10:26:20 -0400 Subject: [PATCH 3/7] Apply fix for link dependencies from main Missing the fix for link dependencies to also capture dependencies from test targets to macros. --- Sources/Basics/Graph/GraphAlgorithms.swift | 47 ------ .../ModuleBuildDescription.swift | 14 +- Sources/Build/BuildPlan/BuildPlan.swift | 136 ++++++++++++++---- 3 files changed, 107 insertions(+), 90 deletions(-) diff --git a/Sources/Basics/Graph/GraphAlgorithms.swift b/Sources/Basics/Graph/GraphAlgorithms.swift index deee4941985..8ccc6038cc0 100644 --- a/Sources/Basics/Graph/GraphAlgorithms.swift +++ b/Sources/Basics/Graph/GraphAlgorithms.swift @@ -131,50 +131,3 @@ public func depthFirstSearch( } } } - -/// Implements a pre-order depth-first search that traverses the whole graph and -/// doesn't distinguish between unique and duplicate nodes. The visitor can abort -/// a path as needed to prune the tree. -/// The method expects the graph to be acyclic but doesn't check that. -/// -/// - Parameters: -/// - nodes: The list of input nodes to sort. -/// - successors: A closure for fetching the successors of a particular node. -/// - onNext: A callback to indicate the node currently being processed -/// including its parent (if any) and its depth. Returns whether to -/// continue down the current path. -/// -/// - Complexity: O(v + e) where (v, e) are the number of vertices and edges -/// reachable from the input nodes via the relation. -public enum DepthFirstContinue { - case `continue` - case abort -} - -public func depthFirstSearch( - _ nodes: [T], - successors: (T) throws -> [T], - visitNext: (T, _ parent: T?) throws -> DepthFirstContinue -) rethrows { - var stack = OrderedSet>() - - for node in nodes { - precondition(stack.isEmpty) - stack.append(TraversalNode(parent: nil, curr: node)) - - while !stack.isEmpty { - let node = stack.removeLast() - - if try visitNext(node.curr, node.parent) == .continue { - for succ in try successors(node.curr) { - stack.append( - TraversalNode( - parent: node.curr, - curr: succ - ) - ) - } - } - } - } -} diff --git a/Sources/Build/BuildDescription/ModuleBuildDescription.swift b/Sources/Build/BuildDescription/ModuleBuildDescription.swift index 06c52486045..95f657f9b46 100644 --- a/Sources/Build/BuildDescription/ModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/ModuleBuildDescription.swift @@ -187,30 +187,18 @@ extension ModuleBuildDescription { var dependencies: [Dependency] = [] plan.traverseDependencies(of: self) { product, _, description in dependencies.append(.product(product, description)) - return .continue } onModule: { module, _, description in dependencies.append(.module(module, description)) - return .continue } return dependencies } package func recursiveLinkDependencies(using plan: BuildPlan) -> [Dependency] { var dependencies: [Dependency] = [] - plan.traverseDependencies(of: self) { product, _, description in - guard product.type != .macro && product.type != .plugin else { - return .abort - } - + plan.traverseLinkDependencies(of: self) { product, _, description in dependencies.append(.product(product, description)) - return .continue } onModule: { module, _, description in - guard module.type != .macro && module.type != .plugin else { - return .abort - } - dependencies.append(.module(module, description)) - return .continue } return dependencies } diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 5bafd2acfeb..28eba581dc4 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -941,23 +941,23 @@ extension BuildPlan { extension BuildPlan { fileprivate typealias Destination = BuildParameters.Destination - + enum TraversalNode: Hashable { case package(ResolvedPackage) case product(ResolvedProduct, BuildParameters.Destination) case module(ResolvedModule, BuildParameters.Destination) - + var destination: BuildParameters.Destination { switch self { case .package: - .target + .target case .product(_, let destination): destination case .module(_, let destination): destination } } - + init( product: ResolvedProduct, context destination: BuildParameters.Destination @@ -971,7 +971,7 @@ extension BuildPlan { self = .product(product, destination) } } - + init( module: ResolvedModule, context destination: BuildParameters.Destination @@ -990,7 +990,7 @@ extension BuildPlan { } } } - + /// Traverse the modules graph and find a destination for every product and module. /// All non-macro/plugin products and modules have `target` destination with one /// notable exception - test products/modules with direct macro dependency. @@ -1007,23 +1007,23 @@ extension BuildPlan { { continue } - + successors.append(.init(product: product, context: .target)) } - + for module in package.modules { // Tests are discovered through an aggregate product which also // informs their destination. if case .test = module.underlying.type { continue } - + successors.append(.init(module: module, context: .target)) } - + return successors } - + func successors( for product: ResolvedProduct, destination: Destination @@ -1031,12 +1031,12 @@ extension BuildPlan { guard destination == .host || product.underlying.type == .test else { return [] } - + return product.modules.map { module in TraversalNode(module: module, context: destination) } } - + func successors( for module: ResolvedModule, destination: Destination @@ -1044,7 +1044,7 @@ extension BuildPlan { guard destination == .host else { return [] } - + return module.dependencies.reduce(into: [TraversalNode]()) { partial, dependency in switch dependency { case .product(let product, conditions: _): @@ -1054,7 +1054,7 @@ extension BuildPlan { } } } - + try await depthFirstSearch(graph.packages.map { TraversalNode.package($0) }) { node in switch node { case .package(let package): @@ -1070,7 +1070,7 @@ extension BuildPlan { break case .product(let product, let destination): try onProduct(product, destination) - + case .module(let module, let destination): try await onModule(module, destination) } @@ -1078,7 +1078,7 @@ extension BuildPlan { // No de-duplication is necessary we only want unique nodes. } } - + /// Traverses the modules graph, computes destination of every module reference and /// provides the data to the caller by means of `onModule` callback. The products /// are completely transparent to this method and are represented by their module dependencies. @@ -1089,7 +1089,7 @@ extension BuildPlan { ) -> Void ) { var visited = Set() - + func successors(for package: ResolvedPackage) -> [TraversalNode] { guard visited.insert(.package(package)).inserted else { return [] @@ -1103,7 +1103,7 @@ extension BuildPlan { return .init(module: $0, context: .target) } } - + func successors( for module: ResolvedModule, destination: Destination @@ -1123,7 +1123,7 @@ extension BuildPlan { } } } - + depthFirstSearch(self.graph.packages.map { TraversalNode.package($0) }) { switch $0 { case .package(let package): @@ -1140,21 +1140,21 @@ extension BuildPlan { case .module(let module, let destination): (module, destination) } - + switch current { case .package, .product: break - + case .module(let module, let destination): onModule((module, destination), parentModule) } } } - + package func traverseDependencies( of description: ModuleBuildDescription, - onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> DepthFirstContinue, - onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> DepthFirstContinue + onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void, + onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> Void ) { var visited = Set() func successors( @@ -1167,7 +1167,7 @@ extension BuildPlan { visited.insert($0).inserted } } - + func successors( for module: ResolvedModule, destination: Destination @@ -1185,6 +1185,82 @@ extension BuildPlan { visited.insert($0).inserted } } + + depthFirstSearch(successors(for: description.module, destination: description.destination)) { + switch $0 { + case .module(let module, let destination): + successors(for: module, destination: destination) + case .product(let product, let destination): + successors(for: product, destination: destination) + case .package: + [] + } + } onNext: { module, _ in + switch module { + case .package: + break + + case .product(let product, let destination): + onProduct(product, destination, self.description(for: product, context: destination)) + + case .module(let module, let destination): + onModule(module, destination, self.description(for: module, context: destination)) + } + } + } + + // Only follow link time dependencies, i.e. skip dependencies on macros and plugins + // except for testTargets that depend on macros. + package func traverseLinkDependencies( + of description: ModuleBuildDescription, + onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void, + onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> Void + ) { + var visited = Set() + func successors( + for product: ResolvedProduct, + destination: Destination + ) -> [TraversalNode] { + product.modules.map { module in + TraversalNode(module: module, context: destination) + }.filter { + visited.insert($0).inserted + } + } + + func successors( + for parentModule: ResolvedModule, + destination: Destination + ) -> [TraversalNode] { + parentModule + .dependencies(satisfying: description.buildParameters.buildEnvironment) + .reduce(into: [TraversalNode]()) { partial, dependency in + switch dependency { + case .product(let product, _): + guard product.type != .plugin else { + return + } + + guard product.type != .macro || parentModule.type == .test else { + return + } + + partial.append(.init(product: product, context: destination)) + case .module(let childModule, _): + guard childModule.type != .plugin else { + return + } + + guard childModule.type != .macro || parentModule.type == .test else { + return + } + + partial.append(.init(module: childModule, context: destination)) + } + }.filter { + visited.insert($0).inserted + } + } depthFirstSearch(successors(for: description.module, destination: description.destination)) { switch $0 { @@ -1195,16 +1271,16 @@ extension BuildPlan { case .package: [] } - } visitNext: { module, _ in + } onNext: { module, _ in switch module { case .package: - return .continue + break case .product(let product, let destination): - return onProduct(product, destination, self.description(for: product, context: destination)) + onProduct(product, destination, self.description(for: product, context: destination)) case .module(let module, let destination): - return onModule(module, destination, self.description(for: module, context: destination)) + onModule(module, destination, self.description(for: module, context: destination)) } } } From f52c354d1c449d18119f14755b712f00aeb2d51d Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Mon, 21 Apr 2025 10:42:40 -0400 Subject: [PATCH 4/7] Fix whitespace and tests. --- Sources/Build/BuildPlan/BuildPlan.swift | 48 +++++++++---------- .../BuildTests/BuildPlanTraversalTests.swift | 2 - 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 28eba581dc4..612f2c2cfcc 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -941,16 +941,16 @@ extension BuildPlan { extension BuildPlan { fileprivate typealias Destination = BuildParameters.Destination - + enum TraversalNode: Hashable { case package(ResolvedPackage) case product(ResolvedProduct, BuildParameters.Destination) case module(ResolvedModule, BuildParameters.Destination) - + var destination: BuildParameters.Destination { switch self { case .package: - .target + .target case .product(_, let destination): destination case .module(_, let destination): @@ -971,7 +971,7 @@ extension BuildPlan { self = .product(product, destination) } } - + init( module: ResolvedModule, context destination: BuildParameters.Destination @@ -990,7 +990,7 @@ extension BuildPlan { } } } - + /// Traverse the modules graph and find a destination for every product and module. /// All non-macro/plugin products and modules have `target` destination with one /// notable exception - test products/modules with direct macro dependency. @@ -1007,23 +1007,23 @@ extension BuildPlan { { continue } - + successors.append(.init(product: product, context: .target)) } - + for module in package.modules { // Tests are discovered through an aggregate product which also // informs their destination. if case .test = module.underlying.type { continue } - + successors.append(.init(module: module, context: .target)) } - + return successors } - + func successors( for product: ResolvedProduct, destination: Destination @@ -1031,12 +1031,12 @@ extension BuildPlan { guard destination == .host || product.underlying.type == .test else { return [] } - + return product.modules.map { module in TraversalNode(module: module, context: destination) } } - + func successors( for module: ResolvedModule, destination: Destination @@ -1044,7 +1044,7 @@ extension BuildPlan { guard destination == .host else { return [] } - + return module.dependencies.reduce(into: [TraversalNode]()) { partial, dependency in switch dependency { case .product(let product, conditions: _): @@ -1054,7 +1054,7 @@ extension BuildPlan { } } } - + try await depthFirstSearch(graph.packages.map { TraversalNode.package($0) }) { node in switch node { case .package(let package): @@ -1078,7 +1078,7 @@ extension BuildPlan { // No de-duplication is necessary we only want unique nodes. } } - + /// Traverses the modules graph, computes destination of every module reference and /// provides the data to the caller by means of `onModule` callback. The products /// are completely transparent to this method and are represented by their module dependencies. @@ -1089,7 +1089,7 @@ extension BuildPlan { ) -> Void ) { var visited = Set() - + func successors(for package: ResolvedPackage) -> [TraversalNode] { guard visited.insert(.package(package)).inserted else { return [] @@ -1103,7 +1103,7 @@ extension BuildPlan { return .init(module: $0, context: .target) } } - + func successors( for module: ResolvedModule, destination: Destination @@ -1123,7 +1123,7 @@ extension BuildPlan { } } } - + depthFirstSearch(self.graph.packages.map { TraversalNode.package($0) }) { switch $0 { case .package(let package): @@ -1140,17 +1140,17 @@ extension BuildPlan { case .module(let module, let destination): (module, destination) } - + switch current { case .package, .product: break - + case .module(let module, let destination): onModule((module, destination), parentModule) } } } - + package func traverseDependencies( of description: ModuleBuildDescription, onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void, @@ -1167,7 +1167,7 @@ extension BuildPlan { visited.insert($0).inserted } } - + func successors( for module: ResolvedModule, destination: Destination @@ -1185,7 +1185,7 @@ extension BuildPlan { visited.insert($0).inserted } } - + depthFirstSearch(successors(for: description.module, destination: description.destination)) { switch $0 { case .module(let module, let destination): @@ -1208,7 +1208,7 @@ extension BuildPlan { } } } - + // Only follow link time dependencies, i.e. skip dependencies on macros and plugins // except for testTargets that depend on macros. package func traverseLinkDependencies( diff --git a/Tests/BuildTests/BuildPlanTraversalTests.swift b/Tests/BuildTests/BuildPlanTraversalTests.swift index ed469887d60..2b898cabd76 100644 --- a/Tests/BuildTests/BuildPlanTraversalTests.swift +++ b/Tests/BuildTests/BuildPlanTraversalTests.swift @@ -146,10 +146,8 @@ final class BuildPlanTraversalTests: XCTestCase { XCTAssertEqual(product.name, "SwiftSyntax") XCTAssertEqual(destination, .host) XCTAssertNil(description) - return .continue } onModule: { module, destination, description in moduleDependencies.append((module, destination, description)) - return .continue } XCTAssertEqual(moduleDependencies.count, 2) From df20ebf05c1af779596c8f381c8d3b900a1b8ec6 Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Mon, 21 Apr 2025 10:44:24 -0400 Subject: [PATCH 5/7] Fix whitespace. --- Sources/Build/BuildPlan/BuildPlan.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 612f2c2cfcc..bae69b79a68 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -957,7 +957,7 @@ extension BuildPlan { destination } } - + init( product: ResolvedProduct, context destination: BuildParameters.Destination @@ -1070,7 +1070,7 @@ extension BuildPlan { break case .product(let product, let destination): try onProduct(product, destination) - + case .module(let module, let destination): try await onModule(module, destination) } From 007f295803671ffe8ec1dd3355fd9bb927d05c93 Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Mon, 21 Apr 2025 11:20:35 -0400 Subject: [PATCH 6/7] Update test to cover test target dependency on macro target. --- Tests/BuildTests/BuildPlanTests.swift | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 1bc3096bfdf..d00b262d449 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -6851,14 +6851,16 @@ final class BuildPlanTests: XCTestCase { "/LeakTest/Sources/CLib/Clib.c", "/LeakTest/Sources/MyMacro/MyMacro.swift", "/LeakTest/Sources/MyPluginTool/MyPluginTool.swift", - "/LeakTest/Plugins/MyPlugin/MyPlugin.swift", "/LeakTest/Sources/MyLib/MyLib.swift", + "/LeakTest/Plugins/MyPlugin/MyPlugin.swift", + "/LeakTest/Tests/MyMacroTests/MyMacroTests.swift", + "/LeakTest/Tests/MyMacro2Tests/MyMacro2Tests.swift", "/LeakLib/Sources/CLib2/include/Clib.h", "/LeakLib/Sources/CLib2/Clib.c", "/LeakLib/Sources/MyMacro2/MyMacro.swift", "/LeakLib/Sources/MyPluginTool2/MyPluginTool.swift", + "/LeakLib/Sources/MyLib2/MyLib.swift", "/LeakLib/Plugins/MyPlugin2/MyPlugin.swift", - "/LeakLib/Sources/MyLib2/MyLib.swift" ]) let graph = try loadModulesGraph(fileSystem: fs, manifests: [ @@ -6867,6 +6869,7 @@ final class BuildPlanTests: XCTestCase { path: "/LeakLib", products: [ ProductDescription(name: "MyLib2", type: .library(.automatic), targets: ["MyLib2"]), + ProductDescription(name: "MyMacros2", type: .macro, targets: ["MyMacro2"]), ], targets: [ TargetDescription(name: "CLib2"), @@ -6892,6 +6895,12 @@ final class BuildPlanTests: XCTestCase { dependencies: ["CLib", "MyMacro", .product(name: "MyLib2", package: "LeakLib")], pluginUsages: [.plugin(name: "MyPlugin", package: nil)] ), + TargetDescription(name: "MyMacroTests", dependencies: ["MyMacro"], type: .test), + TargetDescription( + name: "MyMacro2Tests", + dependencies: [.product(name: "MyMacros2", package: "LeakLib")], + type: .test + ) ] ) ], observabilityScope: observability.topScope) @@ -6906,5 +6915,13 @@ final class BuildPlanTests: XCTestCase { let myLib = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyLib" })).swift() XCTAssertFalse(myLib.additionalFlags.contains(where: { $0.contains("-tool")}), "flags shouldn't contain tools items") + + // Make sure the tests do have the include path and the module map from the lib + let myMacroTests = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyMacroTests" })).swift() + XCTAssertTrue(myMacroTests.additionalFlags.contains(where: { $0.contains("CLib/include") })) + XCTAssertTrue(myMacroTests.additionalFlags.contains(where: { $0.contains("CLib-tool.build/module.modulemap") })) + let myMacro2Tests = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyMacro2Tests" })).swift() + XCTAssertTrue(myMacro2Tests.additionalFlags.contains(where: { $0.contains("CLib2/include") })) + XCTAssertTrue(myMacro2Tests.additionalFlags.contains(where: { $0.contains("CLib2-tool.build/module.modulemap") })) } } From 1b06f6f3131df710f214f3cdb221a5be838d9707 Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Wed, 30 Apr 2025 15:50:58 -0400 Subject: [PATCH 7/7] Bring in the build-using-self script to enable Windows CI. --- Utilities/build-using-self | 234 +++++++++++++++++++++++++++---------- Utilities/helpers.py | 96 +++++++++++++-- 2 files changed, 258 insertions(+), 72 deletions(-) diff --git a/Utilities/build-using-self b/Utilities/build-using-self index c1277db00e1..5daa3898117 100755 --- a/Utilities/build-using-self +++ b/Utilities/build-using-self @@ -1,62 +1,176 @@ -#!/bin/bash -##===----------------------------------------------------------------------===## -## -## This source file is part of the Swift open source project -## -## Copyright (c) 2014-2022 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 -## -##===----------------------------------------------------------------------===## - -set -eu - -__dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -root_dir="$(cd ${__dir}/.. && pwd)" - -cd "${root_dir}/" -echo "Current directory is ${PWD}" - -CONFIGURATION=debug -export SWIFTCI_IS_SELF_HOSTED=1 - -# Ensure SDKROOT is configured -host=$(uname) -if [ "${host}" == "Darwin" ]; then - export SDKROOT=$(xcrun --show-sdk-path --sdk macosx) -fi - -set -x - -env | sort - -# Display toolchain version -swift --version - -# Perform package update in order to get the latest commits for the dependencies. -swift package update -swift build -c $CONFIGURATION -swift test -c $CONFIGURATION --parallel - -# Run the integration tests with just built SwiftPM. -export SWIFTPM_BIN_DIR=$(swift build -c $CONFIGURATION --show-bin-path) -( - cd ${root_dir}/IntegrationTests - # Perform package update in order to get the latest commits for the dependencies. - swift package update - $SWIFTPM_BIN_DIR/swift-test --parallel +#!/usr/bin/env python3 +# ===----------------------------------------------------------------------===## +# +# This source file is part of the Swift open source project +# +# Copyright (c) 2025 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 argparse +import logging +import os +import pathlib +import platform +import shlex +import sys +import typing as t + +from helpers import ( + Configuration, + change_directory, + call, + call_output, +) + +logging.basicConfig( + format=" | ".join( + [ + # Prefix script name to the log in an attempt to avoid confusion when parsing logs + f"{pathlib.Path(sys.argv[0]).name}", + "%(asctime)s", + "%(levelname)-8s", + "%(module)s", + "%(funcName)s", + "Line:%(lineno)d", + "%(message)s", + ] + ), + level=logging.INFO, ) -if [ "${host}" == "Darwin" ]; then - echo "Current working directory is ${PWD}" - echo "Bootstrapping with the XCBuild codepath..." - ${root_dir}/Utilities/bootstrap \ - build \ - --release \ - --verbose \ - --cross-compile-hosts macosx-arm64 \ - --skip-cmake-bootstrap \ - --swift-build-path "${SWIFTPM_BIN_DIR}/swift-build" -fi + +REPO_ROOT_PATH = pathlib.Path(__file__).parent.parent.resolve() + + +def get_arguments() -> argparse.Namespace: + parser = argparse.ArgumentParser( + formatter_class=argparse.ArgumentDefaultsHelpFormatter + ) + + parser.add_argument( + "-v", + "--verbose", + dest="is_verbose", + action="store_true", + help="When set, prints verbose information.", + ) + parser.add_argument( + "-c", + "--configuration", + type=Configuration, + dest="config", + default="debug", + choices=[e.value for e in Configuration], + help="The configuraiton to use.", + ) + parser.add_argument( + "--enable-swift-testing", + action="store_true", + ) + parser.add_argument( + "--enable-xctest", + action="store_true", + ) + args = parser.parse_args() + return args + + +def log_environment() -> None: + logging.info("Environment Variables") + for key, value in sorted(os.environ.items()): + logging.info(" --> %s=%r", key, value) + + +def get_swiftpm_bin_dir(config: Configuration) -> pathlib.Path: + logging.info("Retrieving Swift PM binary directory.") + swiftpm_bin_dir = pathlib.Path( + call_output(["swift", "build", "--configuration", config, "--show-bin-path"]) + ) + logging.info("SwiftPM BIN DIR: %s", swiftpm_bin_dir) + return swiftpm_bin_dir + + +def is_on_darwin() -> bool: + return platform.uname().system == "Darwin" + + +def set_environment(*, swiftpm_bin_dir: pathlib.Path,) -> None: + os.environ["SWIFTCI_IS_SELF_HOSTED"] = "1" + + # Set the SWIFTPM_CUSTOM_BIN_DIR path + os.environ["SWIFTPM_CUSTOM_BIN_DIR"] = str(swiftpm_bin_dir) + + # Ensure SDKROOT is configure + if is_on_darwin(): + sdk_root = call_output(shlex.split("xcrun --show-sdk-path --sdk macosx")) + logging.debug("macos sdk root = %r", sdk_root) + os.environ["SDKROOT"] = sdk_root + log_environment() + + +def run_bootstrap(swiftpm_bin_dir: pathlib.Path) -> None: + logging.info("Current working directory is %s", pathlib.Path.cwd()) + logging.info("Bootstrapping with the XCBuild codepath...") + call( + [ + REPO_ROOT_PATH / "Utilities" / "bootstrap", + "build", + "--release", + "--verbose", + "--cross-compile-hosts", + "macosx-arm64", + "--skip-cmake-bootstrap", + "--swift-build-path", + (swiftpm_bin_dir / "swift-build").resolve(), + ], + ) + + +def main() -> None: + args = get_arguments() + ignore = "-Xlinker /ignore:4217" if os.name == "nt" else "" + logging.getLogger().setLevel(logging.DEBUG if args.is_verbose else logging.INFO) + logging.debug("Args: %r", args) + + with change_directory(REPO_ROOT_PATH): + swiftpm_bin_dir = get_swiftpm_bin_dir(config=args.config) + set_environment(swiftpm_bin_dir=swiftpm_bin_dir) + + call( + shlex.split("swift --version"), + ) + + call( + shlex.split("swift package update"), + ) + call( + shlex.split(f"swift build --configuration {args.config} {ignore}"), + ) + + if os.name != "nt": + swift_testing_arg= "--enable-swift-testing" if args.enable_swift_testing else "" + xctest_arg= "--enable-xctest" if args.enable_swift_testing else "" + call( + shlex.split(f"swift run swift-test --configuration {args.config} --parallel {swift_testing_arg} {xctest_arg} --scratch-path .test {ignore}"), + ) + + integration_test_dir = (REPO_ROOT_PATH / "IntegrationTests").as_posix() + call( + shlex.split(f"swift package --package-path {integration_test_dir} update"), + ) + call( + shlex.split(f"swift run swift-test --package-path {integration_test_dir} --parallel {ignore}"), + ) + + if is_on_darwin(): + run_bootstrap(swiftpm_bin_dir=swiftpm_bin_dir) + logging.info("Done") + + +if __name__ == "__main__": + main() diff --git a/Utilities/helpers.py b/Utilities/helpers.py index cea6d02f51b..b5a4a9df9a9 100644 --- a/Utilities/helpers.py +++ b/Utilities/helpers.py @@ -3,7 +3,7 @@ ## ## This source file is part of the Swift open source project ## -## Copyright (c) 2014-2020 Apple Inc. and the Swift project authors +## Copyright (c) 2014-2025 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 @@ -11,12 +11,37 @@ ## ##===----------------------------------------------------------------------===## -import datetime +import contextlib +import enum +import errno import logging -import subprocess -import sys import os -import errno +import pathlib +import subprocess +import typing as t + + +@contextlib.contextmanager +def change_directory(directory: pathlib.Path) -> t.Iterator[pathlib.Path]: + current_directory = pathlib.Path.cwd() + logging.info("Current directory is %s", current_directory) + logging.info("Changing directory to: %s", directory) + os.chdir(directory) + + try: + yield directory + finally: + logging.debug("Chaning directory back to %s", current_directory) + os.chdir(current_directory) + + +class Configuration(str, enum.Enum): + DEBUG = "debug" + RELEASE = "release" + + def __str__(self) -> str: + return self.value + def symlink_force(source, destination): try: @@ -26,6 +51,7 @@ def symlink_force(source, destination): os.remove(destination) os.symlink(source, destination) + def mkdir_p(path): """Create the given directory, if it does not exist.""" try: @@ -35,22 +61,68 @@ def mkdir_p(path): if e.errno != errno.EEXIST: raise + def call(cmd, cwd=None, verbose=False): """Calls a subprocess.""" - logging.info("executing command >>> %s", ' '.join(cmd)) + cwd = cwd or pathlib.Path.cwd() + logging.info("executing command >>> %r with cwd %s", " ".join([str(c) for c in cmd]), cwd) try: subprocess.check_call(cmd, cwd=cwd) except subprocess.CalledProcessError as cpe: - logging.debug("executing command >>> %s", ' '.join(cmd)) - logging.error("Process failure: %s", str(cpe)) + logging.debug("executing command >>> %r with cwd %s", " ".join([str(c) for c in cmd]), cwd) + logging.error( + "\n".join([ + "Process failure with return code %d: %s", + "[---- START stdout ----]", + "%s", + "[---- END stdout ----]", + "[---- START stderr ----]", + "%s", + "[---- END stderr ----]", + "[---- START OUTPUT ----]", + "%s", + "[---- END OUTPUT ----]", + ]), + cpe.returncode, + str(cpe), + cpe.stdout, + cpe.stderr, + cpe.output, + ) raise cpe + def call_output(cmd, cwd=None, stderr=False, verbose=False): """Calls a subprocess for its return data.""" - logging.info(' '.join(cmd)) + stderr = subprocess.STDOUT if stderr else False + cwd = cwd or pathlib.Path.cwd() + logging.info("executing command >>> %r with cwd %s", " ".join([str(c) for c in cmd]), cwd) try: - return subprocess.check_output(cmd, cwd=cwd, stderr=stderr, universal_newlines=True).strip() + return subprocess.check_output( + cmd, + cwd=cwd, + stderr=stderr, + universal_newlines=True, + ).strip() except subprocess.CalledProcessError as cpe: - logging.debug(' '.join(cmd)) - logging.error(str(cpe)) + logging.debug("executing command >>> %r with cwd %s", " ".join([str(c) for c in cmd]), cwd) + logging.error( + "\n".join([ + "Process failure with return code %d: %s", + "[---- START stdout ----]", + "%s", + "[---- END stdout ----]", + "[---- START stderr ----]", + "%s", + "[---- END stderr ----]", + "[---- START OUTPUT ----]", + "%s", + "[---- END OUTPUT ----]", + ]), + cpe.returncode, + str(cpe), + cpe.stdout, + cpe.stderr, + cpe.output, + ) raise cpe