From 60ee80dd0830e4ef22adffb4a92fc287be9ce1fe Mon Sep 17 00:00:00 2001 From: kishore pusukuri Date: Wed, 29 Nov 2023 21:18:30 -0800 Subject: [PATCH 01/12] A fix for overriding environment variables rdar://118814900 From c6da47b587538747ffed8527454d90f1424cf105 Mon Sep 17 00:00:00 2001 From: kishore pusukuri Date: Fri, 8 Dec 2023 12:26:37 -0800 Subject: [PATCH 02/12] track SDK dependencies rdar://115777026 --- .../SwiftTargetBuildDescription.swift | 6 +++++ Sources/CoreCommands/SwiftTool.swift | 1 + Sources/PackageModel/SwiftSDKs/SwiftSDK.swift | 4 +-- .../BuildParameters/BuildParameters.swift | 5 ++++ .../SPMTestSupport/MockBuildTestHelper.swift | 1 + Sources/swift-bootstrap/main.swift | 1 + Tests/BuildTests/IncrementalBuildTests.swift | 25 +++++++++++++++++++ 7 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index a934404205f..ac41266b56b 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -645,6 +645,12 @@ public final class SwiftTargetBuildDescription { } } + // rdar://115777026 + // Compiler commands need to track SDK dependencies to trigger rebuilds when the SDK changes + if let sdkPath = self.buildParameters.sdkPath, self.buildParameters.targetTriple.isDarwin() { + args += ["-sdk", sdkPath.pathString] + } + return args } diff --git a/Sources/CoreCommands/SwiftTool.swift b/Sources/CoreCommands/SwiftTool.swift index 0ed863b0098..3c81d93116d 100644 --- a/Sources/CoreCommands/SwiftTool.swift +++ b/Sources/CoreCommands/SwiftTool.swift @@ -695,6 +695,7 @@ public final class SwiftTool { return try BuildParameters( dataPath: dataPath, + sdkPath: toolchain.sdkRootPath, configuration: options.build.configuration, toolchain: toolchain, triple: triple, diff --git a/Sources/PackageModel/SwiftSDKs/SwiftSDK.swift b/Sources/PackageModel/SwiftSDKs/SwiftSDK.swift index 07ff540dea7..a0526838745 100644 --- a/Sources/PackageModel/SwiftSDKs/SwiftSDK.swift +++ b/Sources/PackageModel/SwiftSDKs/SwiftSDK.swift @@ -478,8 +478,8 @@ public struct SwiftSDK: Equatable { let sdkPath: AbsolutePath? #if os(macOS) // Get the SDK. - if let value = lookupExecutablePath(filename: ProcessEnv.vars["SDKROOT"]) { - sdkPath = value + if let value = ProcessEnv.vars["SDKROOT"] { + sdkPath = try AbsolutePath(validating: value) } else { // No value in env, so search for it. let sdkPathStr = try TSCBasic.Process.checkNonZeroExit( diff --git a/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift index cbc5cb5499c..f0351963984 100644 --- a/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift +++ b/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift @@ -29,6 +29,9 @@ public struct BuildParameters: Encodable { /// The path to the data directory. public var dataPath: AbsolutePath + /// The path to the SDKROOT directory. + public var sdkPath: AbsolutePath? + /// The build configuration. public var configuration: BuildConfiguration @@ -119,6 +122,7 @@ public struct BuildParameters: Encodable { public init( dataPath: AbsolutePath, + sdkPath: AbsolutePath?, configuration: BuildConfiguration, toolchain: Toolchain, triple: Triple? = nil, @@ -145,6 +149,7 @@ public struct BuildParameters: Encodable { ) self.dataPath = dataPath + self.sdkPath = toolchain.sdkRootPath self.configuration = configuration self._toolchain = _Toolchain(toolchain: toolchain) self.triple = triple diff --git a/Sources/SPMTestSupport/MockBuildTestHelper.swift b/Sources/SPMTestSupport/MockBuildTestHelper.swift index c92fe8b1753..0da329bb365 100644 --- a/Sources/SPMTestSupport/MockBuildTestHelper.swift +++ b/Sources/SPMTestSupport/MockBuildTestHelper.swift @@ -85,6 +85,7 @@ public func mockBuildParameters( ) -> BuildParameters { try! BuildParameters( dataPath: buildPath, + sdkPath: toolchain.sdkRootPath, configuration: config, toolchain: toolchain, triple: targetTriple, diff --git a/Sources/swift-bootstrap/main.swift b/Sources/swift-bootstrap/main.swift index 7891cd06ade..4e8819b2487 100644 --- a/Sources/swift-bootstrap/main.swift +++ b/Sources/swift-bootstrap/main.swift @@ -279,6 +279,7 @@ struct SwiftBootstrapBuildTool: ParsableCommand { let buildParameters = try BuildParameters( dataPath: dataPath, + sdkPath: self.targetToolchain.sdkRootPath, configuration: configuration, toolchain: self.targetToolchain, triple: self.hostToolchain.targetTriple, diff --git a/Tests/BuildTests/IncrementalBuildTests.swift b/Tests/BuildTests/IncrementalBuildTests.swift index c8e966d8b0f..81a9ff55a84 100644 --- a/Tests/BuildTests/IncrementalBuildTests.swift +++ b/Tests/BuildTests/IncrementalBuildTests.swift @@ -14,6 +14,7 @@ import Basics import PackageModel import SPMTestSupport import XCTest +import class TSCBasic.Process /// Functional tests of incremental builds. These are fairly ad hoc at this /// point, and because of the time they take, they need to be kept minimal. @@ -145,4 +146,28 @@ final class IncrementalBuildTests: XCTestCase { XCTAssertNoMatch(log2, .contains("Planning build")) } } + // testing the fix for tracking SDK dependencies to avoid triggering rebuilds when the SDK changes (rdar://115777026) + func testSDKTracking() throws { +#if os(macOS) + try XCTSkipIf(!UserToolchain.default.supportsSDKDependentTests(), "skipping because test environment doesn't support this test") + + try fixture(name: "ValidLayouts/SingleModule/Library") { fixturePath in + let dummySwiftcPath = SwiftPM.xctestBinaryPath(for: "dummy-swiftc") + let swiftCompilerPath = try UserToolchain.default.swiftCompilerPath + let environment = [ + "SWIFT_EXEC": dummySwiftcPath.pathString, + "SWIFT_ORIGINAL_PATH": swiftCompilerPath.pathString + ] + let sdkPathStr = try TSCBasic.Process.checkNonZeroExit( + arguments: ["/usr/bin/xcrun", "--sdk", "macosx", "--show-sdk-path"], + environment: environment + ).spm_chomp() + + let newSdkPathStr = "/tmp/../\(sdkPathStr)" + // Perform a full build again because SDK changed. + let log1 = try executeSwiftBuild(fixturePath, env: ["SDKROOT": newSdkPathStr]).stdout + XCTAssertMatch(log1, .contains("Compiling Library")) + } +#endif + } } From ef5ce43f11e8a31dda49007c81edcfa6414a8bcf Mon Sep 17 00:00:00 2001 From: kishore pusukuri Date: Wed, 17 Jan 2024 07:19:55 -0800 Subject: [PATCH 03/12] removed unnecessary variable --- .../Build/BuildDescription/SwiftTargetBuildDescription.swift | 2 +- Sources/CoreCommands/SwiftTool.swift | 1 - Sources/SPMBuildCore/BuildParameters/BuildParameters.swift | 5 ----- Sources/SPMTestSupport/MockBuildTestHelper.swift | 1 - Sources/swift-bootstrap/main.swift | 1 - 5 files changed, 1 insertion(+), 9 deletions(-) diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index ac41266b56b..4324d734a3e 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -647,7 +647,7 @@ public final class SwiftTargetBuildDescription { // rdar://115777026 // Compiler commands need to track SDK dependencies to trigger rebuilds when the SDK changes - if let sdkPath = self.buildParameters.sdkPath, self.buildParameters.targetTriple.isDarwin() { + if let sdkPath = self.buildParameters.toolchain.sdkRootPath, self.buildParameters.triple.isDarwin() { args += ["-sdk", sdkPath.pathString] } diff --git a/Sources/CoreCommands/SwiftTool.swift b/Sources/CoreCommands/SwiftTool.swift index 3c81d93116d..0ed863b0098 100644 --- a/Sources/CoreCommands/SwiftTool.swift +++ b/Sources/CoreCommands/SwiftTool.swift @@ -695,7 +695,6 @@ public final class SwiftTool { return try BuildParameters( dataPath: dataPath, - sdkPath: toolchain.sdkRootPath, configuration: options.build.configuration, toolchain: toolchain, triple: triple, diff --git a/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift index f0351963984..cbc5cb5499c 100644 --- a/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift +++ b/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift @@ -29,9 +29,6 @@ public struct BuildParameters: Encodable { /// The path to the data directory. public var dataPath: AbsolutePath - /// The path to the SDKROOT directory. - public var sdkPath: AbsolutePath? - /// The build configuration. public var configuration: BuildConfiguration @@ -122,7 +119,6 @@ public struct BuildParameters: Encodable { public init( dataPath: AbsolutePath, - sdkPath: AbsolutePath?, configuration: BuildConfiguration, toolchain: Toolchain, triple: Triple? = nil, @@ -149,7 +145,6 @@ public struct BuildParameters: Encodable { ) self.dataPath = dataPath - self.sdkPath = toolchain.sdkRootPath self.configuration = configuration self._toolchain = _Toolchain(toolchain: toolchain) self.triple = triple diff --git a/Sources/SPMTestSupport/MockBuildTestHelper.swift b/Sources/SPMTestSupport/MockBuildTestHelper.swift index 0da329bb365..c92fe8b1753 100644 --- a/Sources/SPMTestSupport/MockBuildTestHelper.swift +++ b/Sources/SPMTestSupport/MockBuildTestHelper.swift @@ -85,7 +85,6 @@ public func mockBuildParameters( ) -> BuildParameters { try! BuildParameters( dataPath: buildPath, - sdkPath: toolchain.sdkRootPath, configuration: config, toolchain: toolchain, triple: targetTriple, diff --git a/Sources/swift-bootstrap/main.swift b/Sources/swift-bootstrap/main.swift index 4e8819b2487..7891cd06ade 100644 --- a/Sources/swift-bootstrap/main.swift +++ b/Sources/swift-bootstrap/main.swift @@ -279,7 +279,6 @@ struct SwiftBootstrapBuildTool: ParsableCommand { let buildParameters = try BuildParameters( dataPath: dataPath, - sdkPath: self.targetToolchain.sdkRootPath, configuration: configuration, toolchain: self.targetToolchain, triple: self.hostToolchain.targetTriple, From 1a9d270d1a677a02c90b925a6c8c9d53b6d63559 Mon Sep 17 00:00:00 2001 From: kishore pusukuri Date: Thu, 18 Jan 2024 07:35:11 -0800 Subject: [PATCH 04/12] removed unnecessary check --- .../BuildDescription/SwiftTargetBuildDescription.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index 4324d734a3e..a934404205f 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -645,12 +645,6 @@ public final class SwiftTargetBuildDescription { } } - // rdar://115777026 - // Compiler commands need to track SDK dependencies to trigger rebuilds when the SDK changes - if let sdkPath = self.buildParameters.toolchain.sdkRootPath, self.buildParameters.triple.isDarwin() { - args += ["-sdk", sdkPath.pathString] - } - return args } From a99e152f60b8cd4fdbc668f727c8eae0d207cdc2 Mon Sep 17 00:00:00 2001 From: kishore Date: Mon, 22 Jan 2024 10:27:22 -0800 Subject: [PATCH 05/12] derive dynamic libraries for explicitly linking rdar://108561857 --- .../Build/BuildPlan/BuildPlan+Product.swift | 10 ++- Tests/BuildTests/BuildPlanTests.swift | 69 +++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/Sources/Build/BuildPlan/BuildPlan+Product.swift b/Sources/Build/BuildPlan/BuildPlan+Product.swift index 5d11f4d1b44..d14ec2418ad 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Product.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Product.swift @@ -143,6 +143,12 @@ extension BuildPlan { topLevelDependencies = [] } + // get the dynamic libraries for explicitly linking rdar://108561857 + func recursiveDynamicLibraries(for product: ResolvedProduct) throws -> [ResolvedProduct] { + let dylibs = try computeDependencies(of: product, buildParameters: buildParameters).dylibs + return try dylibs + dylibs.flatMap { try recursiveDynamicLibraries(for: $0) } + } + // Sort the product targets in topological order. let nodes: [ResolvedTarget.Dependency] = product.targets.map { .target($0, conditions: []) } let allTargets = try topologicalSort(nodes, successors: { dependency in @@ -175,7 +181,9 @@ extension BuildPlan { return productDependencies case .plugin: return shouldExcludePlugins ? [] : productDependencies - case .library(.dynamic), .test, .executable, .snippet, .macro: + case .library(.dynamic): + return try recursiveDynamicLibraries(for: product).map { .product($0, conditions: []) } + case .test, .executable, .snippet, .macro: return [] } } diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index d94c734d46b..fb00cba82f4 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -6198,4 +6198,73 @@ final class BuildPlanTests: XCTestCase { XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), linkArguments) XCTAssertNoDiagnostics(observability.diagnostics) } + + // testing of deriving dynamic libraries for explicitly linking rdar://108561857 + func testDerivingDylibs() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/thisPkg/Sources/exe/main.swift", + "/fooPkg/Sources/FooLogging/file.swift", + "/barPkg/Sources/BarLogging/file.swift" + ) + let observability = ObservabilitySystem.makeForTesting() + let graph = try loadPackageGraph( + fileSystem: fs, + manifests: [ + Manifest.createFileSystemManifest( + displayName: "fooPkg", + path: "/fooPkg", + dependencies: [ + .localSourceControl(path: "/barPkg", requirement: .upToNextMajor(from: "1.0.0")), + ], + products: [ + ProductDescription(name: "FooLogging", type: .library(.dynamic), targets: ["FooLogging"]), + ], + targets: [ + TargetDescription( + name: "FooLogging", + dependencies: [.product(name: "BarLogging", package: "barPkg")] + ), + ] + ), + Manifest.createFileSystemManifest( + displayName: "barPkg", + path: "/barPkg", + products: [ + ProductDescription(name: "BarLogging", type: .library(.dynamic), targets: ["BarLogging"]), + ], + targets: [ + TargetDescription(name: "BarLogging", dependencies: []), + ] + ), + Manifest.createRootManifest( + displayName: "thisPkg", + path: "/thisPkg", + toolsVersion: .v5_8, + dependencies: [ + .localSourceControl(path: "/fooPkg", requirement: .upToNextMajor(from: "1.0.0")), + ], + targets: [ + TargetDescription( + name: "exe", + dependencies: [.product(name: "FooLogging", package: "fooPkg"),], + type: .executable + ), + ] + ), + ], + observabilityScope: observability.topScope + ) + XCTAssertNoDiagnostics(observability.diagnostics) + let result = try BuildPlanResult(plan: BuildPlan( + buildParameters: mockBuildParameters(shouldLinkStaticSwiftStdlib: true), + graph: graph, + fileSystem: fs, + observabilityScope: observability.topScope + )) + result.checkProductsCount(3) + result.checkTargetsCount(3) + XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "FooLogging" }) + XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "BarLogging" }) + } } From 53fbf705c7a2d63e7d50520506cb1762719a5143 Mon Sep 17 00:00:00 2001 From: kishore Date: Tue, 23 Jan 2024 09:12:04 -0800 Subject: [PATCH 06/12] improved the test case --- Tests/BuildTests/BuildPlanTests.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index fb00cba82f4..4909f43d701 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -6266,5 +6266,8 @@ final class BuildPlanTests: XCTestCase { result.checkTargetsCount(3) XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "FooLogging" }) XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "BarLogging" }) + let buildProduct = try XCTUnwrap(result.productMap["exe"]) + let dylibs = Array(buildProduct.dylibs.map({$0.product.name})).sorted() + XCTAssertEqual(dylibs, ["BarLogging", "FooLogging"]) } } From c8503f0b5025f820a2a428476d9b61dc99e8e313 Mon Sep 17 00:00:00 2001 From: kishore Date: Thu, 25 Jan 2024 17:20:14 -0800 Subject: [PATCH 07/12] rdar://113256834 consider plugin build command input files into consideration and don't emit unnecessary warnings --- .../Package.swift | 67 ++++++++++++++ .../MySourceGenBuildToolPlugin/plugin.swift | 36 ++++++++ .../Sources/MyLocalTool/foo.dat | 1 + .../Sources/MyLocalTool/main.swift | 1 + .../Sources/MyOtherLocalTool/bar.dat | 1 + .../Sources/MyOtherLocalTool/baz.dat | 1 + .../Sources/MyOtherLocalTool/main.swift | 2 + .../Sources/MySourceGenBuildTool/main.swift | 18 ++++ .../MySourceGenBuildToolLib/library.swift | 11 +++ .../MySourceGenRuntimeLib/library.swift | 3 + ...urceGenPluginNoPreBuildCommandsTests.swift | 79 ++++++++++++++++ Sources/Build/BuildOperation.swift | 92 ++++++++++++------- Tests/FunctionalTests/PluginTests.swift | 10 ++ 13 files changed, 291 insertions(+), 31 deletions(-) create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Package.swift create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Plugins/MySourceGenBuildToolPlugin/plugin.swift create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/foo.dat create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/main.swift create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/bar.dat create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/baz.dat create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/main.swift create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildTool/main.swift create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildToolLib/library.swift create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenRuntimeLib/library.swift create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Package.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Package.swift new file mode 100644 index 00000000000..54f7cead18c --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Package.swift @@ -0,0 +1,67 @@ +// swift-tools-version: 5.9 +import PackageDescription + +let package = Package( + name: "MySourceGenPluginNoPreBuildCommands", + products: [ + // The product that vends MySourceGenBuildToolPlugin to client packages. + .plugin( + name: "MySourceGenBuildToolPlugin", + targets: ["MySourceGenBuildToolPlugin"] + ), + // The product that vends the MySourceGenBuildTool executable to client packages. + .executable( + name: "MySourceGenBuildTool", + targets: ["MySourceGenBuildTool"] + ), + ], + targets: [ + // A local tool that uses a build tool plugin. + .executableTarget( + name: "MyLocalTool", + plugins: [ + "MySourceGenBuildToolPlugin", + ] + ), + // A local tool that uses a prebuild plugin. + .executableTarget( + name: "MyOtherLocalTool", + plugins: [ + "MySourceGenBuildToolPlugin", + ] + ), + // The plugin that generates build tool commands to invoke MySourceGenBuildTool. + .plugin( + name: "MySourceGenBuildToolPlugin", + capability: .buildTool(), + dependencies: [ + "MySourceGenBuildTool", + ] + ), + // The command line tool that generates source files. + .executableTarget( + name: "MySourceGenBuildTool", + dependencies: [ + "MySourceGenBuildToolLib", + ] + ), + // A library used by MySourceGenBuildTool (not the client). + .target( + name: "MySourceGenBuildToolLib" + ), + // A runtime library that the client needs to link against. + .target( + name: "MySourceGenRuntimeLib" + ), + // Unit tests for the plugin. + .testTarget( + name: "MySourceGenPluginTests", + dependencies: [ + "MySourceGenRuntimeLib", + ], + plugins: [ + "MySourceGenBuildToolPlugin", + ] + ) + ] +) diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Plugins/MySourceGenBuildToolPlugin/plugin.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Plugins/MySourceGenBuildToolPlugin/plugin.swift new file mode 100644 index 00000000000..5ccedebc704 --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Plugins/MySourceGenBuildToolPlugin/plugin.swift @@ -0,0 +1,36 @@ +import PackagePlugin + +@main +struct MyPlugin: BuildToolPlugin { + #if USE_CREATE + let verb = "Creating" + #else + let verb = "Generating" + #endif + + func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] { + print("Hello from the Build Tool Plugin!") + guard let target = target as? SourceModuleTarget else { return [] } + return try target.sourceFiles.map{ $0.path }.compactMap { + guard $0.extension == "dat" else { return .none } + let outputName = $0.stem + ".swift" + let outputPath = context.pluginWorkDirectory.appending(outputName) + return .buildCommand( + displayName: + "\(verb) \(outputName) from \($0.lastComponent)", + executable: + try context.tool(named: "MySourceGenBuildTool").path, + arguments: [ + "\($0)", + "\(outputPath)" + ], + inputFiles: [ + $0, + ], + outputFiles: [ + outputPath + ] + ) + } + } +} diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/foo.dat b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/foo.dat new file mode 100644 index 00000000000..55cef2c81ff --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/foo.dat @@ -0,0 +1 @@ +I am Foo! diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/main.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/main.swift new file mode 100644 index 00000000000..1bbab0f2fc3 --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/main.swift @@ -0,0 +1 @@ +print("Generated string Foo: '\(foo)'") diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/bar.dat b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/bar.dat new file mode 100644 index 00000000000..f44d2914ea9 --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/bar.dat @@ -0,0 +1 @@ +I am Bar! diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/baz.dat b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/baz.dat new file mode 100644 index 00000000000..e8ebf1dcae7 --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/baz.dat @@ -0,0 +1 @@ +I am Baz! diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/main.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/main.swift new file mode 100644 index 00000000000..bdb816d993b --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/main.swift @@ -0,0 +1,2 @@ +// print("Generated string Bar: '\(bar)'") +// print("Generated string Baz: '\(baz)'") diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildTool/main.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildTool/main.swift new file mode 100644 index 00000000000..94f7766628c --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildTool/main.swift @@ -0,0 +1,18 @@ +import Foundation +import MySourceGenBuildToolLib + +// Sample source generator tool that emits a Swift variable declaration of a string containing the hex representation of the contents of a file as a quoted string. The variable name is the base name of the input file. The input file is the first argument and the output file is the second. +if ProcessInfo.processInfo.arguments.count != 3 { + print("usage: MySourceGenBuildTool ") + exit(1) +} +let inputFile = ProcessInfo.processInfo.arguments[1] +let outputFile = ProcessInfo.processInfo.arguments[2] + +let variableName = URL(fileURLWithPath: inputFile).deletingPathExtension().lastPathComponent + +let inputData = FileManager.default.contents(atPath: inputFile) ?? Data() +let dataAsHex = inputData.map { String(format: "%02hhx", $0) }.joined() +let outputString = "public var \(variableName) = \(dataAsHex.quotedForSourceCode)\n" +let outputData = outputString.data(using: .utf8) +FileManager.default.createFile(atPath: outputFile, contents: outputData) diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildToolLib/library.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildToolLib/library.swift new file mode 100644 index 00000000000..25d612434d7 --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildToolLib/library.swift @@ -0,0 +1,11 @@ +import Foundation + +extension String { + + public var quotedForSourceCode: String { + return "\"" + self + .replacingOccurrences(of: "\\", with: "\\\\") + .replacingOccurrences(of: "\"", with: "\\\"") + + "\"" + } +} diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenRuntimeLib/library.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenRuntimeLib/library.swift new file mode 100644 index 00000000000..af09355050f --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenRuntimeLib/library.swift @@ -0,0 +1,3 @@ +public func GetLibraryName() -> String { + return "MySourceGenRuntimeLib" +} diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift new file mode 100644 index 00000000000..fc5d18d5a1a --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift @@ -0,0 +1,79 @@ +import XCTest +import class Foundation.Bundle + +// for rdar://113256834 +final class SwiftyProtobufTests: XCTestCase { + func testMyLocalTool() throws { + // This is an example of a functional test case. + // Use XCTAssert and related functions to verify your tests produce the correct + // results. + + // Some of the APIs that we use below are available in macOS 10.13 and above. + guard #available(macOS 10.13, *) else { + return + } + + // Mac Catalyst won't have `Process`, but it is supported for executables. + #if !targetEnvironment(macCatalyst) + + print("### ", productsDirectory) + let fooBinary = productsDirectory.appendingPathComponent("MyLocalTool") + + let process = Process() + process.executableURL = fooBinary + + let pipe = Pipe() + process.standardOutput = pipe + + try process.run() + process.waitUntilExit() + + let data = pipe.fileHandleForReading.readDataToEndOfFile() + let output = String(data: data, encoding: .utf8) + XCTAssertEqual(output, "Generated string Foo: \'4920616d20466f6f210a\'\n") + #endif + } + + func testMyOtherLocalTool() throws { + // This is an example of a functional test case. + // Use XCTAssert and related functions to verify your tests produce the correct + // results. + + // Some of the APIs that we use below are available in macOS 10.13 and above. + guard #available(macOS 10.13, *) else { + return + } + + // Mac Catalyst won't have `Process`, but it is supported for executables. + #if !targetEnvironment(macCatalyst) + + print("### ", productsDirectory) + let fooBinary = productsDirectory.appendingPathComponent("MyOtherLocalTool") + + let process = Process() + process.executableURL = fooBinary + + let pipe = Pipe() + process.standardOutput = pipe + + try process.run() + process.waitUntilExit() + + let data = pipe.fileHandleForReading.readDataToEndOfFile() + let output = String(data: data, encoding: .utf8) + XCTAssertEqual(output, "") + #endif + } + + /// Returns path to the built products directory. + var productsDirectory: URL { + #if os(macOS) + for bundle in Bundle.allBundles where bundle.bundlePath.hasSuffix(".xctest") { + return bundle.bundleURL.deletingLastPathComponent() + } + fatalError("couldn't find the products directory") + #else + return Bundle.main.bundleURL + #endif + } +} diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index cde0828dba9..7fb9f23016b 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -147,7 +147,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS /// /// This will try skip build planning if build manifest caching is enabled /// and the package structure hasn't changed. - public func getBuildDescription() throws -> BuildDescription { + public func getBuildDescription(subset: BuildSubset? = nil) throws -> BuildDescription { return try self.buildDescription.memoize { if self.cacheBuildManifest { do { @@ -171,7 +171,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS } } // We need to perform actual planning if we reach here. - return try self.plan().description + return try self.plan(subset: subset).description } } @@ -261,7 +261,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS // Get the build description (either a cached one or newly created). // Get the build description - let buildDescription = try getBuildDescription() + let buildDescription = try getBuildDescription(subset: subset) // Verify dependency imports on the described targets try verifyTargetImports(in: buildDescription) @@ -447,10 +447,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS } /// Create the build plan and return the build description. - private func plan() throws -> (description: BuildDescription, manifest: LLBuildManifest) { + private func plan(subset: BuildSubset? = nil) throws -> (description: BuildDescription, manifest: LLBuildManifest) { // Load the package graph. let graph = try getPackageGraph() - let buildToolPluginInvocationResults: [ResolvedTarget.ID: (target: ResolvedTarget, results: [BuildToolPluginInvocationResult])] let prebuildCommandResults: [ResolvedTarget.ID: [PrebuildCommandResult]] // Invoke any build tool plugins in the graph to generate prebuild commands and build commands. @@ -530,33 +529,43 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS } // Emit warnings about any unhandled files in authored packages. We do this after applying build tool plugins, once we know what files they handled. - for package in graph.rootPackages where package.manifest.toolsVersion >= .v5_3 { - for target in package.targets { - // Get the set of unhandled files in targets. - var unhandledFiles = Set(target.underlying.others) - if unhandledFiles.isEmpty { continue } - - // Subtract out any that were inputs to any commands generated by plugins. - if let result = buildToolPluginInvocationResults[target.id]?.results { - let handledFiles = result.flatMap { $0.buildCommands.flatMap { $0.inputFiles } } - unhandledFiles.subtract(handledFiles) - } - if unhandledFiles.isEmpty { continue } - - // Emit a diagnostic if any remain. This is kept the same as the previous message for now, but this could be improved. - let diagnosticsEmitter = self.observabilityScope.makeDiagnosticsEmitter { - var metadata = ObservabilityMetadata() - metadata.packageIdentity = package.identity - metadata.packageKind = package.manifest.packageKind - metadata.targetName = target.name - return metadata - } - var warning = "found \(unhandledFiles.count) file(s) which are unhandled; explicitly declare them as resources or exclude from the target\n" - for file in unhandledFiles { - warning += " " + file.pathString + "\n" - } - diagnosticsEmitter.emit(warning: warning) + // rdar://113256834 This fix works for the plugins that do not have PreBuildCommands. + let targetsToConsider: [ResolvedTarget] + if let subset = subset, let recursiveDependencies = try subset.recursiveDependencies(for: graph, observabilityScope: observabilityScope) { + targetsToConsider = recursiveDependencies + } else { + targetsToConsider = Array(graph.reachableTargets) + } + + for target in targetsToConsider { + guard let package = graph.package(for: target), package.manifest.toolsVersion >= .v5_3 else { + continue } + + // Get the set of unhandled files in targets. + var unhandledFiles = Set(target.underlying.others) + if unhandledFiles.isEmpty { continue } + + // Subtract out any that were inputs to any commands generated by plugins. + if let result = buildToolPluginInvocationResults[target.id]?.results { + let handledFiles = result.flatMap{ $0.buildCommands.flatMap{ $0.inputFiles } } + unhandledFiles.subtract(handledFiles) + } + if unhandledFiles.isEmpty { continue } + + // Emit a diagnostic if any remain. This is kept the same as the previous message for now, but this could be improved. + let diagnosticsEmitter = self.observabilityScope.makeDiagnosticsEmitter { + var metadata = ObservabilityMetadata() + metadata.packageIdentity = package.identity + metadata.packageKind = package.manifest.packageKind + metadata.targetName = target.name + return metadata + } + var warning = "found \(unhandledFiles.count) file(s) which are unhandled; explicitly declare them as resources or exclude from the target\n" + for file in unhandledFiles { + warning += " " + file.pathString + "\n" + } + diagnosticsEmitter.emit(warning: warning) } // Create the build plan based, on the graph and any information from plugins. @@ -782,6 +791,27 @@ extension BuildDescription { } extension BuildSubset { + func recursiveDependencies(for graph: PackageGraph, observabilityScope: ObservabilityScope) throws -> [ResolvedTarget]? { + switch self { + case .allIncludingTests: + return Array(graph.reachableTargets) + case .allExcludingTests: + return graph.reachableTargets.filter { $0.type != .test } + case .product(let productName): + guard let product = graph.allProducts.first(where: { $0.name == productName }) else { + observabilityScope.emit(error: "no product named '\(productName)'") + return nil + } + return try product.recursiveTargetDependencies() + case .target(let targetName): + guard let target = graph.allTargets.first(where: { $0.name == targetName }) else { + observabilityScope.emit(error: "no target named '\(targetName)'") + return nil + } + return try target.recursiveTargetDependencies() + } + } + /// Returns the name of the llbuild target that corresponds to the build subset. func llbuildTargetName(for graph: PackageGraph, config: String, observabilityScope: ObservabilityScope) -> String? diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index 20eb444400e..bfc557cdc49 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -34,6 +34,16 @@ class PluginTests: XCTestCase { } } + func testUseOfBuildToolPluginTargetNoPreBuildCommandsByExecutableInSamePackage() throws { + // Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require). + try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency") + + try fixture(name: "Miscellaneous/Plugins") { fixturePath in + let (_, stderr) = try executeSwiftBuild(fixturePath.appending("MySourceGenPluginNoPreBuildCommands"), configuration: .Debug, extraArgs: ["--product", "MyLocalTool"]) + XCTAssertFalse(stderr.contains("file(s) which are unhandled; explicitly declare them as resources or exclude from the target"), "unwanted warning") + } + } + func testUseOfBuildToolPluginProductByExecutableAcrossPackages() throws { // Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require). try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency") From 407a003a810f65dad2fdb3d937d6f9c771fa3765 Mon Sep 17 00:00:00 2001 From: kishore Date: Tue, 30 Jan 2024 11:14:09 -0800 Subject: [PATCH 08/12] fixing the test case --- .../MySourceGenPluginNoPreBuildCommandsTests.swift | 6 ++---- .../MySourceGenPluginNoPreBuildCommandsTests/lunch.txt | 1 + Tests/FunctionalTests/PluginTests.swift | 5 ++--- 3 files changed, 5 insertions(+), 7 deletions(-) create mode 100644 Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/lunch.txt diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift index fc5d18d5a1a..2e44779caee 100644 --- a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift @@ -15,8 +15,7 @@ final class SwiftyProtobufTests: XCTestCase { // Mac Catalyst won't have `Process`, but it is supported for executables. #if !targetEnvironment(macCatalyst) - - print("### ", productsDirectory) + let fooBinary = productsDirectory.appendingPathComponent("MyLocalTool") let process = Process() @@ -46,8 +45,7 @@ final class SwiftyProtobufTests: XCTestCase { // Mac Catalyst won't have `Process`, but it is supported for executables. #if !targetEnvironment(macCatalyst) - - print("### ", productsDirectory) + let fooBinary = productsDirectory.appendingPathComponent("MyOtherLocalTool") let process = Process() diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/lunch.txt b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/lunch.txt new file mode 100644 index 00000000000..32e1a738d68 --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/lunch.txt @@ -0,0 +1 @@ +coffee diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index bfc557cdc49..3129404410e 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -37,10 +37,9 @@ class PluginTests: XCTestCase { func testUseOfBuildToolPluginTargetNoPreBuildCommandsByExecutableInSamePackage() throws { // Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require). try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency") - try fixture(name: "Miscellaneous/Plugins") { fixturePath in - let (_, stderr) = try executeSwiftBuild(fixturePath.appending("MySourceGenPluginNoPreBuildCommands"), configuration: .Debug, extraArgs: ["--product", "MyLocalTool"]) - XCTAssertFalse(stderr.contains("file(s) which are unhandled; explicitly declare them as resources or exclude from the target"), "unwanted warning") + let (_, stderr) = try executeSwiftTest(fixturePath.appending("MySourceGenPluginNoPreBuildCommands")) + XCTAssertTrue(stderr.contains("file(s) which are unhandled; explicitly declare them as resources or exclude from the target"), "expected warning not emitted") } } From 1f7173d845cf1dfc3c168a9edd9152ec0f2f5108 Mon Sep 17 00:00:00 2001 From: kishore Date: Mon, 5 Feb 2024 09:53:42 -0800 Subject: [PATCH 09/12] fix code formatting --- Sources/Build/BuildOperation.swift | 3 ++- Tests/FunctionalTests/PluginTests.swift | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index 7fb9f23016b..3311c4e0f25 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -531,7 +531,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS // Emit warnings about any unhandled files in authored packages. We do this after applying build tool plugins, once we know what files they handled. // rdar://113256834 This fix works for the plugins that do not have PreBuildCommands. let targetsToConsider: [ResolvedTarget] - if let subset = subset, let recursiveDependencies = try subset.recursiveDependencies(for: graph, observabilityScope: observabilityScope) { + if let subset = subset, let recursiveDependencies = try + subset.recursiveDependencies(for: graph, observabilityScope: observabilityScope) { targetsToConsider = recursiveDependencies } else { targetsToConsider = Array(graph.reachableTargets) diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index 3129404410e..39b5a45d818 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -34,7 +34,7 @@ class PluginTests: XCTestCase { } } - func testUseOfBuildToolPluginTargetNoPreBuildCommandsByExecutableInSamePackage() throws { + func testUseOfBuildToolPluginTargetNoPreBuildCommands() throws { // Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require). try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency") try fixture(name: "Miscellaneous/Plugins") { fixturePath in From 6f81983bb86c705cc2341b8dc8e34bf42ea0bed0 Mon Sep 17 00:00:00 2001 From: kishore Date: Thu, 14 Mar 2024 19:26:47 -0700 Subject: [PATCH 10/12] rdar://112671586 Suppress redundant linkage warnings due to dependency linkage propagation --- .../ProductBuildDescription.swift | 6 ++++++ Sources/XCBuildSupport/PIFBuilder.swift | 5 +++++ Tests/BuildTests/BuildPlanTests.swift | 15 +++++++++++++++ Tests/BuildTests/BuildSystemDelegateTests.swift | 2 +- Tests/XCBuildSupportTests/PIFBuilderTests.swift | 6 ++++-- 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/Sources/Build/BuildDescription/ProductBuildDescription.swift b/Sources/Build/BuildDescription/ProductBuildDescription.swift index bb96cd383a2..7b7506306a5 100644 --- a/Sources/Build/BuildDescription/ProductBuildDescription.swift +++ b/Sources/Build/BuildDescription/ProductBuildDescription.swift @@ -189,6 +189,12 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription var isLinkingStaticStdlib = false let triple = self.buildParameters.triple + + // radar://112671586 supress unnecessary warnings + if triple.isMacOSX { + args += ["-Xlinker", "-no_warn_duplicate_libraries"] + } + switch derivedProductType { case .macro: throw InternalError("macro not supported") // should never be reached diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index 6907fb9b3bb..7d26627aec1 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -672,6 +672,11 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { impartedSettings[.OTHER_LDFLAGS, default: ["$(inherited)"]].append("-lc++") } + // radar://112671586 supress unnecessary warnings + #if os(macOS) + impartedSettings[.OTHER_LDFLAGS, default: ["$(inherited)"]].append("-Wl,-no_warn_duplicate_libraries") + #endif + addSources(target.sources, to: pifTarget) // Handle the target's dependencies (but don't link against them). diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 2055e433f4b..09636db39ee 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -773,6 +773,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", @@ -1186,6 +1187,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-dead_strip", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", @@ -1276,6 +1278,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", @@ -1435,6 +1438,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", @@ -1625,6 +1629,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", @@ -1779,6 +1784,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", @@ -2027,6 +2033,7 @@ final class BuildPlanTests: XCTestCase { buildPath.appending(components: "PkgPackageTests.xctest", "Contents", "MacOS", "PkgPackageTests") .pathString, "-module-name", "PkgPackageTests", + "-Xlinker", "-no_warn_duplicate_libraries", "-Xlinker", "-bundle", "-Xlinker", "-rpath", "-Xlinker", "@loader_path/../../../", "@\(buildPath.appending(components: "PkgPackageTests.product", "Objects.LinkFileList"))", @@ -2125,6 +2132,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-dead_strip", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", @@ -2489,6 +2497,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", @@ -2636,6 +2645,7 @@ final class BuildPlanTests: XCTestCase { "-o", buildPath.appending(components: "Foo").pathString, "-module-name", "Foo", "-lBar-Baz", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "Foo.product", "Objects.LinkFileList"))", @@ -2651,6 +2661,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "libBar-Baz.dylib").pathString, "-module-name", "Bar_Baz", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-library", "-Xlinker", "-install_name", "-Xlinker", "@rpath/libBar-Baz.dylib", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", @@ -2803,6 +2814,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "liblib.dylib").pathString, "-module-name", "lib", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-library", "-Xlinker", "-install_name", "-Xlinker", "@rpath/liblib.dylib", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", @@ -2942,6 +2954,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "liblib.dylib").pathString, "-module-name", "lib", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-library", "-Xlinker", "-install_name", "-Xlinker", "@rpath/liblib.dylib", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", @@ -2956,6 +2969,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", @@ -6158,6 +6172,7 @@ final class BuildPlanTests: XCTestCase { "-L", buildPath.pathString, "-o", buildPath.appending(components: "exe").pathString, "-module-name", "exe", + "-Xlinker", "-no_warn_duplicate_libraries", "-emit-executable", "@\(buildPath.appending(components: "exe.product", "Objects.LinkFileList"))", "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift-5.5/macosx", diff --git a/Tests/BuildTests/BuildSystemDelegateTests.swift b/Tests/BuildTests/BuildSystemDelegateTests.swift index 2251dde6d44..e8d50d1dd38 100644 --- a/Tests/BuildTests/BuildSystemDelegateTests.swift +++ b/Tests/BuildTests/BuildSystemDelegateTests.swift @@ -25,7 +25,7 @@ final class BuildSystemDelegateTests: XCTestCase { try XCTSkipIf(true, "test is only supported on macOS") #endif let (fullLog, _) = try executeSwiftBuild(fixturePath) - XCTAssertTrue(fullLog.contains("ld: warning: ignoring duplicate libraries: '-lz'"), "log didn't contain expected linker diagnostics") + XCTAssertFalse(fullLog.contains("ld: warning: ignoring duplicate libraries: '-lz'"), "log contains unnecessary linker warnings") } } diff --git a/Tests/XCBuildSupportTests/PIFBuilderTests.swift b/Tests/XCBuildSupportTests/PIFBuilderTests.swift index dcabed69073..f323036fa11 100644 --- a/Tests/XCBuildSupportTests/PIFBuilderTests.swift +++ b/Tests/XCBuildSupportTests/PIFBuilderTests.swift @@ -1312,6 +1312,7 @@ class PIFBuilderTests: XCTestCase { "-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/FooLib1.modulemap" ]) XCTAssertEqual(settings[.OTHER_LDRFLAGS], []) + XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-Wl,-no_warn_duplicate_libraries"]) } } @@ -1384,7 +1385,7 @@ class PIFBuilderTests: XCTestCase { "-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/FooLib2.modulemap" ]) XCTAssertEqual(settings[.OTHER_LDRFLAGS], []) - XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-lc++"]) + XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-lc++", "-Wl,-no_warn_duplicate_libraries"]) XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], [ "$(inherited)", "-Xcc", "-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/FooLib2.modulemap" @@ -1463,6 +1464,7 @@ class PIFBuilderTests: XCTestCase { "-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/BarLib.modulemap" ]) XCTAssertEqual(settings[.OTHER_LDRFLAGS], []) + XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-Wl,-no_warn_duplicate_libraries"]) XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], [ "$(inherited)", "-Xcc", "-fmodule-map-file=$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)/BarLib.modulemap" @@ -2393,7 +2395,7 @@ class PIFBuilderTests: XCTestCase { target.checkImpartedBuildSettings { settings in XCTAssertEqual(settings[.GCC_PREPROCESSOR_DEFINITIONS], nil) XCTAssertEqual(settings[.HEADER_SEARCH_PATHS], nil) - XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-lz"]) + XCTAssertEqual(settings[.OTHER_LDFLAGS], ["$(inherited)", "-Wl,-no_warn_duplicate_libraries", "-lz"]) XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], nil) } } From f7e6759c435bc3fb32fb2e58aba06f6ae35996a1 Mon Sep 17 00:00:00 2001 From: kishore Date: Tue, 19 Mar 2024 10:00:39 -0700 Subject: [PATCH 11/12] fixing test cases --- .../Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift | 3 +-- Sources/XCBuildSupport/PIFBuilder.swift | 4 +--- Tests/BuildTests/BuildSystemDelegateTests.swift | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift b/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift index 326e3041c52..765384e32b3 100644 --- a/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift +++ b/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift @@ -8,8 +8,7 @@ let package = Package( .executableTarget( name: "DoNotFilterLinkerDiagnostics", linkerSettings: [ - .linkedLibrary("z"), - .unsafeFlags(["-lz"]), + .unsafeFlags(["-Lfoobar"]), // should produce: ld: warning: ignoring duplicate libraries: '-lz' ] ), diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index 7d26627aec1..d019f651957 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -673,9 +673,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { } // radar://112671586 supress unnecessary warnings - #if os(macOS) - impartedSettings[.OTHER_LDFLAGS, default: ["$(inherited)"]].append("-Wl,-no_warn_duplicate_libraries") - #endif + impartedSettings[.OTHER_LDFLAGS, default: ["$(inherited)"]].append("-Wl,-no_warn_duplicate_libraries") addSources(target.sources, to: pifTarget) diff --git a/Tests/BuildTests/BuildSystemDelegateTests.swift b/Tests/BuildTests/BuildSystemDelegateTests.swift index e8d50d1dd38..4e44a42dafe 100644 --- a/Tests/BuildTests/BuildSystemDelegateTests.swift +++ b/Tests/BuildTests/BuildSystemDelegateTests.swift @@ -25,7 +25,7 @@ final class BuildSystemDelegateTests: XCTestCase { try XCTSkipIf(true, "test is only supported on macOS") #endif let (fullLog, _) = try executeSwiftBuild(fixturePath) - XCTAssertFalse(fullLog.contains("ld: warning: ignoring duplicate libraries: '-lz'"), "log contains unnecessary linker warnings") + XCTAssertTrue(fullLog.contains("ld: warning: search path 'foobar' not found"), "log didn't contain expected linker diagnostics") } } From 8c16a1b6114537aa119819bed713512e07328ad0 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 19 Mar 2024 17:03:43 +0000 Subject: [PATCH 12/12] Apply suggestions from code review --- .../Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift b/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift index 765384e32b3..9845aea76b5 100644 --- a/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift +++ b/Fixtures/Miscellaneous/DoNotFilterLinkerDiagnostics/Package.swift @@ -9,7 +9,6 @@ let package = Package( name: "DoNotFilterLinkerDiagnostics", linkerSettings: [ .unsafeFlags(["-Lfoobar"]), - // should produce: ld: warning: ignoring duplicate libraries: '-lz' ] ), ]