From e8d6e17bc57795991313c254a91d1ab513d3c7dc Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 10 Jun 2025 14:16:03 -0400 Subject: [PATCH 1/6] Prevent unnecessary rebuilds when modifying swiftc arguments Many arguments for the compiler are marked as `doesNotAffectIncrementalBuild` in `swift-driver`. Use the argument hashing in swift-driver to hash the swift compiler flags and incorporate that in to the build hash signature used to determine if SwiftPM should do a full rebuild. This prevents unnecssary rebuilds when adding or changing arguments like `-Xswiftc -diagnostic-style=llvm`. For a list of arguments that no longer affect rebuilds, search for `doesNotAffectIncrementalBuild` in https://github.com/swiftlang/swift-driver/blob/main/Sources/SwiftOptions/Options.swift. This builds on the work introduced in #8717 which avoided unnecessary rebuilds when SwiftPM arguments changed. --- Sources/Build/LLBuildCommands.swift | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Sources/Build/LLBuildCommands.swift b/Sources/Build/LLBuildCommands.swift index 0e127959402..67ca7945483 100644 --- a/Sources/Build/LLBuildCommands.swift +++ b/Sources/Build/LLBuildCommands.swift @@ -15,6 +15,10 @@ import Foundation import LLBuildManifest import SPMBuildCore import SPMLLBuild +import PackageModel + +import struct SwiftDriver.BuildRecordArguments +import struct SwiftOptions.OptionTable import class TSCBasic.LocalFileOutputByteStream @@ -426,13 +430,20 @@ final class PackageStructureCommand: CustomLLBuildCommand { /// For instance, building with or without `--verbose` should not cause a full rebuild. private func normalizeBuildParameters( _ buildParameters: BuildParameters - ) -> BuildParameters { + ) throws -> BuildParameters { var buildParameters = buildParameters buildParameters.outputParameters = BuildParameters.Output( isColorized: false, isVerbose: false ) buildParameters.workers = 1 + + let optionTable = OptionTable() + let parsedOptions = try optionTable.parse(Array(buildParameters.flags.swiftCompilerFlags), for: .batch) + let buildRecordInfoHash = BuildRecordArguments.computeHash(parsedOptions) + + buildParameters.flags.swiftCompilerFlags = [buildRecordInfoHash] + return buildParameters } } From 906e64921025cc4e5e8dc39aa657716cfb8fe506 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 13 Jun 2025 08:47:51 -0400 Subject: [PATCH 2/6] Respect impl only import flag --- Sources/Build/LLBuildCommands.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Sources/Build/LLBuildCommands.swift b/Sources/Build/LLBuildCommands.swift index 67ca7945483..aeb9442eb19 100644 --- a/Sources/Build/LLBuildCommands.swift +++ b/Sources/Build/LLBuildCommands.swift @@ -17,8 +17,13 @@ import SPMBuildCore import SPMLLBuild import PackageModel -import struct SwiftDriver.BuildRecordArguments -import struct SwiftOptions.OptionTable +#if USE_IMPL_ONLY_IMPORTS +@_implementationOnly import SwiftDriver +@_implementationOnly import SwiftOptions +#else +import SwiftDriver +import SwiftOptions +#endif import class TSCBasic.LocalFileOutputByteStream @@ -442,6 +447,8 @@ final class PackageStructureCommand: CustomLLBuildCommand { let parsedOptions = try optionTable.parse(Array(buildParameters.flags.swiftCompilerFlags), for: .batch) let buildRecordInfoHash = BuildRecordArguments.computeHash(parsedOptions) + // Replace the swiftCompilerFlags with a hash of themselves where arguments that + // do not affect incremental builds are removed. buildParameters.flags.swiftCompilerFlags = [buildRecordInfoHash] return buildParameters From 4f7a2b8e80bc24f8e13116f69b355010dfccc0f2 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 17 Jun 2025 11:11:09 -0400 Subject: [PATCH 3/6] Add tests for SwiftPM args and swiftc args that shouldn't cause a rebuild --- Tests/CommandsTests/BuildCommandTests.swift | 102 +++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/Tests/CommandsTests/BuildCommandTests.swift b/Tests/CommandsTests/BuildCommandTests.swift index 0218326f213..de531d2839d 100644 --- a/Tests/CommandsTests/BuildCommandTests.swift +++ b/Tests/CommandsTests/BuildCommandTests.swift @@ -1125,7 +1125,96 @@ struct BuildCommandTestCases { } } - + private static func buildSystemAndOutputLocation() throws -> [(BuildSystemProvider.Kind, Basics.RelativePath)] { + return try SupportedBuildSystemOnPlatform.map { buildSystem in + switch buildSystem { + case .xcode: + return ( + .xcode, + try RelativePath(validating: ".build") + .appending("apple") + .appending("Products") + .appending("Debug") + .appending("ExecutableNew") + ) + case .swiftbuild: + let triple = try UserToolchain.default.targetTriple.withoutVersion().tripleString + return ( + .swiftbuild, + try RelativePath(validating: ".build") + .appending(triple) + .appending("Products") + .appending("Debug") + .appending("ExecutableNew") + ) + case .native: + return ( + .native, + try RelativePath(validating: ".build") + .appending("debug") + .appending("ExecutableNew.build") + .appending("main.swift.o") + ) + } + } + } + + @Test(arguments: try buildSystemAndOutputLocation()) + func doesNotRebuildWithVerboseFlag( + buildSystem: BuildSystemProvider.Kind, + outputFile: Basics.RelativePath + ) async throws { + try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in + _ = try await self.build( + [], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) + + let mainOFile = fixturePath.appending(outputFile) + let initialMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + + _ = try await self.build( + ["--verbose"], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) + + let subsequentMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + #expect(initialMainOMtime == subsequentMainOMtime, "Expected no rebuild to occur when using the verbose flag, but the file was modified.") + } + } + + @Test(arguments: try buildSystemAndOutputLocation()) + func doesNotRebuildWithSwiftcArgsThatDontAffectIncrementalBuilds( + buildSystem: BuildSystemProvider.Kind, + outputFile: Basics.RelativePath + ) async throws { + try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in + _ = try await self.build( + [], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) + + let mainOFile = fixturePath.appending(outputFile) + let initialMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + + _ = try await self.build( + ["-Xswiftc", "-diagnostic-style=llvm"], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) + + let subsequentMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + #expect(initialMainOMtime == subsequentMainOMtime, "Expected no rebuild to occur when supplying -diagnostic-style, but the file was modified.") + } + } + @Test( .SWBINTTODO("Test failed because of missing plugin support in the PIF builder. This can be reinvestigated after the support is there."), .tags( @@ -1233,5 +1322,16 @@ struct BuildCommandTestCases { } +extension Triple { + func withoutVersion() throws -> Triple { + if isDarwin() { + let stringWithoutVersion = tripleString(forPlatformVersion: "") + return try Triple(stringWithoutVersion) + } else { + return self + } + } +} + From f59dcee99ab239666c9bec40a9b912bb2bf9d60c Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 17 Jun 2025 12:58:00 -0400 Subject: [PATCH 4/6] Add platform name to Debug output folder --- Tests/CommandsTests/BuildCommandTests.swift | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Tests/CommandsTests/BuildCommandTests.swift b/Tests/CommandsTests/BuildCommandTests.swift index de531d2839d..0d011a3b226 100644 --- a/Tests/CommandsTests/BuildCommandTests.swift +++ b/Tests/CommandsTests/BuildCommandTests.swift @@ -1127,6 +1127,8 @@ struct BuildCommandTestCases { private static func buildSystemAndOutputLocation() throws -> [(BuildSystemProvider.Kind, Basics.RelativePath)] { return try SupportedBuildSystemOnPlatform.map { buildSystem in + let triple = try UserToolchain.default.targetTriple.withoutVersion() + let debugFolder = triple.platformName() == "macosx" ? "Debug" : "Debug-\(triple.platformName() ?? "unknown")" switch buildSystem { case .xcode: return ( @@ -1134,18 +1136,21 @@ struct BuildCommandTestCases { try RelativePath(validating: ".build") .appending("apple") .appending("Products") - .appending("Debug") - .appending("ExecutableNew") + .appending(debugFolder) + .appending("ExecutableNew.swiftmodule") + .appending("Project") + .appending("\(triple).swiftsourceinfo") ) case .swiftbuild: - let triple = try UserToolchain.default.targetTriple.withoutVersion().tripleString return ( .swiftbuild, try RelativePath(validating: ".build") - .appending(triple) + .appending(triple.tripleString) .appending("Products") - .appending("Debug") - .appending("ExecutableNew") + .appending(debugFolder) + .appending("ExecutableNew.swiftmodule") + .appending("Project") + .appending("\(triple).swiftsourceinfo") ) case .native: return ( From 45e831eecd683aef0861b77c17438f241829d1a8 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 17 Jun 2025 14:47:20 -0400 Subject: [PATCH 5/6] Fixup test paths on macOS --- Tests/CommandsTests/BuildCommandTests.swift | 93 ++++++++++++--------- switch_branches.sh | 19 +++++ 2 files changed, 71 insertions(+), 41 deletions(-) create mode 100755 switch_branches.sh diff --git a/Tests/CommandsTests/BuildCommandTests.swift b/Tests/CommandsTests/BuildCommandTests.swift index 0d011a3b226..e9892847772 100644 --- a/Tests/CommandsTests/BuildCommandTests.swift +++ b/Tests/CommandsTests/BuildCommandTests.swift @@ -1129,25 +1129,28 @@ struct BuildCommandTestCases { return try SupportedBuildSystemOnPlatform.map { buildSystem in let triple = try UserToolchain.default.targetTriple.withoutVersion() let debugFolder = triple.platformName() == "macosx" ? "Debug" : "Debug-\(triple.platformName() ?? "unknown")" + let base = try RelativePath(validating: ".build") switch buildSystem { case .xcode: - return ( - .xcode, - try RelativePath(validating: ".build") + let path = base .appending("apple") .appending("Products") .appending(debugFolder) + return ( + .xcode, + triple.platformName() == "macosx" ? path.appending("ExecutableNew") : path .appending("ExecutableNew.swiftmodule") .appending("Project") .appending("\(triple).swiftsourceinfo") ) case .swiftbuild: - return ( - .swiftbuild, - try RelativePath(validating: ".build") + let path = base .appending(triple.tripleString) .appending("Products") .appending(debugFolder) + return ( + .swiftbuild, + triple.platformName() == "macosx" ? path.appending("ExecutableNew") : path .appending("ExecutableNew.swiftmodule") .appending("Project") .appending("\(triple).swiftsourceinfo") @@ -1155,7 +1158,7 @@ struct BuildCommandTestCases { case .native: return ( .native, - try RelativePath(validating: ".build") + base .appending("debug") .appending("ExecutableNew.build") .appending("main.swift.o") @@ -1169,26 +1172,30 @@ struct BuildCommandTestCases { buildSystem: BuildSystemProvider.Kind, outputFile: Basics.RelativePath ) async throws { - try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in - _ = try await self.build( - [], - packagePath: fixturePath, - cleanAfterward: false, - buildSystem: buildSystem, - ) + try await withKnownIssue("Sometimes failed to build due to a possible path issue", isIntermittent: true) { + try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in + _ = try await self.build( + [], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) - let mainOFile = fixturePath.appending(outputFile) - let initialMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + let mainOFile = fixturePath.appending(outputFile) + let initialMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date - _ = try await self.build( - ["--verbose"], - packagePath: fixturePath, - cleanAfterward: false, - buildSystem: buildSystem, - ) + _ = try await self.build( + ["--verbose"], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) - let subsequentMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date - #expect(initialMainOMtime == subsequentMainOMtime, "Expected no rebuild to occur when using the verbose flag, but the file was modified.") + let subsequentMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + #expect(initialMainOMtime == subsequentMainOMtime, "Expected no rebuild to occur when using the verbose flag, but the file was modified.") + } + } when: { + buildSystem == .swiftbuild && ProcessInfo.hostOperatingSystem == .windows } } @@ -1197,26 +1204,30 @@ struct BuildCommandTestCases { buildSystem: BuildSystemProvider.Kind, outputFile: Basics.RelativePath ) async throws { - try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in - _ = try await self.build( - [], - packagePath: fixturePath, - cleanAfterward: false, - buildSystem: buildSystem, - ) + try await withKnownIssue("Sometimes failed to build due to a possible path issue", isIntermittent: true) { + try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in + _ = try await self.build( + [], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) - let mainOFile = fixturePath.appending(outputFile) - let initialMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + let mainOFile = fixturePath.appending(outputFile) + let initialMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date - _ = try await self.build( - ["-Xswiftc", "-diagnostic-style=llvm"], - packagePath: fixturePath, - cleanAfterward: false, - buildSystem: buildSystem, - ) + _ = try await self.build( + ["-Xswiftc", "-diagnostic-style=llvm"], + packagePath: fixturePath, + cleanAfterward: false, + buildSystem: buildSystem, + ) - let subsequentMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date - #expect(initialMainOMtime == subsequentMainOMtime, "Expected no rebuild to occur when supplying -diagnostic-style, but the file was modified.") + let subsequentMainOMtime = try FileManager.default.attributesOfItem(atPath: mainOFile.pathString)[.modificationDate] as? Date + #expect(initialMainOMtime == subsequentMainOMtime, "Expected no rebuild to occur when supplying -diagnostic-style, but the file was modified.") + } + } when: { + buildSystem == .swiftbuild && ProcessInfo.hostOperatingSystem == .windows } } diff --git a/switch_branches.sh b/switch_branches.sh new file mode 100755 index 00000000000..a57f5f626fb --- /dev/null +++ b/switch_branches.sh @@ -0,0 +1,19 @@ +#!/bin/bash + +checkout() { + echo "Checking out $1" + git reset --hard $1 +} + +{ + export SLEEP_TIME=2 + + while : + do + checkout origin/main + sleep $SLEEP_TIME + + checkout f0a5106f3449c22ec27df3776b819d039de5877d + sleep $SLEEP_TIME + done +} From 06c31cf42297846cf62e2db8e6a72433985feba1 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 17 Jun 2025 20:17:50 -0400 Subject: [PATCH 6/6] Use buildSystem.binPathSuffixes in tests --- Tests/CommandsTests/BuildCommandTests.swift | 22 ++++++++------------- switch_branches.sh | 19 ------------------ 2 files changed, 8 insertions(+), 33 deletions(-) delete mode 100755 switch_branches.sh diff --git a/Tests/CommandsTests/BuildCommandTests.swift b/Tests/CommandsTests/BuildCommandTests.swift index e9892847772..f1cc0e3fa3f 100644 --- a/Tests/CommandsTests/BuildCommandTests.swift +++ b/Tests/CommandsTests/BuildCommandTests.swift @@ -1128,28 +1128,23 @@ struct BuildCommandTestCases { private static func buildSystemAndOutputLocation() throws -> [(BuildSystemProvider.Kind, Basics.RelativePath)] { return try SupportedBuildSystemOnPlatform.map { buildSystem in let triple = try UserToolchain.default.targetTriple.withoutVersion() - let debugFolder = triple.platformName() == "macosx" ? "Debug" : "Debug-\(triple.platformName() ?? "unknown")" let base = try RelativePath(validating: ".build") + let debugFolderComponents = buildSystem.binPathSuffixes(for: .debug) switch buildSystem { case .xcode: - let path = base - .appending("apple") - .appending("Products") - .appending(debugFolder) + let path = base.appending(components: debugFolderComponents) return ( - .xcode, + buildSystem, triple.platformName() == "macosx" ? path.appending("ExecutableNew") : path .appending("ExecutableNew.swiftmodule") .appending("Project") .appending("\(triple).swiftsourceinfo") ) case .swiftbuild: - let path = base - .appending(triple.tripleString) - .appending("Products") - .appending(debugFolder) + let path = base.appending(triple.tripleString) + .appending(components: debugFolderComponents) return ( - .swiftbuild, + buildSystem, triple.platformName() == "macosx" ? path.appending("ExecutableNew") : path .appending("ExecutableNew.swiftmodule") .appending("Project") @@ -1157,9 +1152,8 @@ struct BuildCommandTestCases { ) case .native: return ( - .native, - base - .appending("debug") + buildSystem, + base.appending(components: debugFolderComponents) .appending("ExecutableNew.build") .appending("main.swift.o") ) diff --git a/switch_branches.sh b/switch_branches.sh deleted file mode 100755 index a57f5f626fb..00000000000 --- a/switch_branches.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/bash - -checkout() { - echo "Checking out $1" - git reset --hard $1 -} - -{ - export SLEEP_TIME=2 - - while : - do - checkout origin/main - sleep $SLEEP_TIME - - checkout f0a5106f3449c22ec27df3776b819d039de5877d - sleep $SLEEP_TIME - done -}