From 443d884aec6cb32ba9d7d25989eb8d973a28c5a6 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 24 Feb 2024 19:53:34 +0000 Subject: [PATCH 1/4] Allow `swift-build` to have `async` entrypoint Due to a bug in Swift 5.9, we can't have multiple executables with `async` entrypoints linked into test products. This was the reason why we weren't able to make `swift-build` entrypoint `async`. Since we have a single `swift-package-manager` executable anyway, we can link that one into tests instead, and pass "simulated" executable name via an environment variable. This allows us to work around the bug and start using structured concurrency from `swift-build` entrypoint. --- Package.swift | 13 +++---------- Sources/Commands/SwiftBuildTool.swift | 4 ++-- Sources/SPMTestSupport/SwiftPMProduct.swift | 11 ++++++----- Sources/swift-build/CMakeLists.txt | 5 ++++- .../{main.swift => Entrypoint.swift} | 7 ++++++- Sources/swift-package-manager/SwiftPM.swift | 18 +++++++++++++++--- 6 files changed, 36 insertions(+), 22 deletions(-) rename Sources/swift-build/{main.swift => Entrypoint.swift} (83%) diff --git a/Package.swift b/Package.swift index 1b67941a013..30e19e82cf0 100644 --- a/Package.swift +++ b/Package.swift @@ -681,9 +681,7 @@ package.targets.append(contentsOf: [ .testTarget( name: "FunctionalPerformanceTests", dependencies: [ - "swift-build", - "swift-package", - "swift-test", + "swift-package-manager", "SPMTestSupport" ] ), @@ -695,9 +693,7 @@ if ProcessInfo.processInfo.environment["SWIFTCI_DISABLE_SDK_DEPENDENT_TESTS"] == .testTarget( name: "FunctionalTests", dependencies: [ - "swift-build", - "swift-package", - "swift-test", + "swift-package-manager", "PackageModel", "SPMTestSupport" ] @@ -713,10 +709,7 @@ if ProcessInfo.processInfo.environment["SWIFTCI_DISABLE_SDK_DEPENDENT_TESTS"] == .testTarget( name: "CommandsTests", dependencies: [ - "swift-build", - "swift-package", - "swift-test", - "swift-run", + "swift-package-manager", "Basics", "Build", "Commands", diff --git a/Sources/Commands/SwiftBuildTool.swift b/Sources/Commands/SwiftBuildTool.swift index ed01e55084b..5833e76cc6e 100644 --- a/Sources/Commands/SwiftBuildTool.swift +++ b/Sources/Commands/SwiftBuildTool.swift @@ -95,7 +95,7 @@ struct BuildToolOptions: ParsableArguments { } /// swift-build tool namespace -public struct SwiftBuildTool: SwiftCommand { +public struct SwiftBuildTool: AsyncSwiftCommand { public static var configuration = CommandConfiguration( commandName: "build", _superCommandName: "swift", @@ -110,7 +110,7 @@ public struct SwiftBuildTool: SwiftCommand { @OptionGroup() var options: BuildToolOptions - public func run(_ swiftTool: SwiftTool) throws { + public func run(_ swiftTool: SwiftTool) async throws { if options.shouldPrintBinPath { return try print(swiftTool.productsBuildParameters.buildPath.description) } diff --git a/Sources/SPMTestSupport/SwiftPMProduct.swift b/Sources/SPMTestSupport/SwiftPMProduct.swift index 1c10b1fda4e..4522060b83e 100644 --- a/Sources/SPMTestSupport/SwiftPMProduct.swift +++ b/Sources/SPMTestSupport/SwiftPMProduct.swift @@ -29,7 +29,7 @@ public enum SwiftPM { extension SwiftPM { /// Executable name. - private var executableName: RelativePath { + private var executableName: String { switch self { case .Build: return "swift-build" @@ -45,7 +45,7 @@ extension SwiftPM { } public var xctestBinaryPath: AbsolutePath { - Self.xctestBinaryPath(for: executableName) + Self.xctestBinaryPath(for: RelativePath("swift-package-manager")) } public static func xctestBinaryPath(for executableName: RelativePath) -> AbsolutePath { @@ -114,11 +114,12 @@ extension SwiftPM { #endif // FIXME: We use this private environment variable hack to be able to // create special conditions in swift-build for swiftpm tests. - environment["SWIFTPM_TESTS_MODULECACHE"] = xctestBinaryPath.parentDirectory.pathString - + environment["SWIFTPM_TESTS_MODULECACHE"] = self.xctestBinaryPath.parentDirectory.pathString + // Unset the internal env variable that allows skipping certain tests. environment["_SWIFTPM_SKIP_TESTS_LIST"] = nil - + environment["SWIFTPM_EXEC_NAME"] = self.executableName + for (key, value) in env ?? [:] { environment[key] = value } diff --git a/Sources/swift-build/CMakeLists.txt b/Sources/swift-build/CMakeLists.txt index 4488799b1f5..936298a6fad 100644 --- a/Sources/swift-build/CMakeLists.txt +++ b/Sources/swift-build/CMakeLists.txt @@ -7,9 +7,12 @@ # See http://swift.org/CONTRIBUTORS.txt for Swift project authors add_executable(swift-build - main.swift) + Entrypoint.swift) target_link_libraries(swift-build PRIVATE Commands) +target_compile_options(swift-build PRIVATE + -parse-as-library) + install(TARGETS swift-build DESTINATION bin) diff --git a/Sources/swift-build/main.swift b/Sources/swift-build/Entrypoint.swift similarity index 83% rename from Sources/swift-build/main.swift rename to Sources/swift-build/Entrypoint.swift index dbf30ff272b..6d5204e7daf 100644 --- a/Sources/swift-build/main.swift +++ b/Sources/swift-build/Entrypoint.swift @@ -12,4 +12,9 @@ import Commands -SwiftBuildTool.main() +@main +struct Entrypoint { + static func main() async { + await SwiftBuildTool.main() + } +} diff --git a/Sources/swift-package-manager/SwiftPM.swift b/Sources/swift-package-manager/SwiftPM.swift index e441a621873..e1bc0ab06fb 100644 --- a/Sources/swift-package-manager/SwiftPM.swift +++ b/Sources/swift-package-manager/SwiftPM.swift @@ -17,17 +17,22 @@ import PackageCollectionsTool import PackageRegistryTool let firstArg = CommandLine.arguments[0] -let execName = (try? AbsolutePath(validating: firstArg).basenameWithoutExt) ?? +let baseNameWithoutExtension = (try? AbsolutePath(validating: firstArg).basenameWithoutExt) ?? (try? RelativePath(validating: firstArg).basenameWithoutExt) @main struct SwiftPM { static func main() async { + await main(execName: baseNameWithoutExtension) + } + + @discardableResult + private static func main(execName: String?) async -> Bool { switch execName { case "swift-package": await SwiftPackageTool.main() case "swift-build": - SwiftBuildTool.main() + await SwiftBuildTool.main() case "swift-experimental-sdk": await SwiftSDKTool.main() case "swift-test": @@ -39,7 +44,14 @@ struct SwiftPM { case "swift-package-registry": await SwiftPackageRegistryTool.main() default: - fatalError("swift-package-manager launched with unexpected name: \(execName ?? "(unknown)")") + // Workaround a bug in Swift 5.9, where multiple executables with an `async` main entrypoint can't be linked + // into the same test bundle. We're then linking single `swift-package-manager` binary instead and passing + // executable name via `SWIFTPM_EXEC_NAME`. + if await !main(execName: EnvironmentVariables.process()["SWIFTPM_EXEC_NAME"]) { + fatalError("swift-package-manager launched with unexpected name: \(execName ?? "(unknown)")") + } } + + return true } } From b49a95e794911573d7527f836f99f6617b9c348a Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 24 Feb 2024 21:51:23 +0000 Subject: [PATCH 2/4] Clean up `main` entrypoint of `swift-package-manager` --- Sources/swift-package-manager/SwiftPM.swift | 16 +++++++++------- Tests/CommandsTests/RunToolTests.swift | 11 +++++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/Sources/swift-package-manager/SwiftPM.swift b/Sources/swift-package-manager/SwiftPM.swift index e1bc0ab06fb..aba32f39f83 100644 --- a/Sources/swift-package-manager/SwiftPM.swift +++ b/Sources/swift-package-manager/SwiftPM.swift @@ -23,7 +23,14 @@ let baseNameWithoutExtension = (try? AbsolutePath(validating: firstArg).basename @main struct SwiftPM { static func main() async { - await main(execName: baseNameWithoutExtension) + // Workaround a bug in Swift 5.9, where multiple executables with an `async` main entrypoint can't be linked + // into the same test bundle. We're then linking single `swift-package-manager` binary instead and passing + // executable name via `SWIFTPM_EXEC_NAME`. + if baseNameWithoutExtension == "swift-package-manager" { + await main(execName: EnvironmentVariables.process()["SWIFTPM_EXEC_NAME"]) + } else { + await main(execName: baseNameWithoutExtension) + } } @discardableResult @@ -44,12 +51,7 @@ struct SwiftPM { case "swift-package-registry": await SwiftPackageRegistryTool.main() default: - // Workaround a bug in Swift 5.9, where multiple executables with an `async` main entrypoint can't be linked - // into the same test bundle. We're then linking single `swift-package-manager` binary instead and passing - // executable name via `SWIFTPM_EXEC_NAME`. - if await !main(execName: EnvironmentVariables.process()["SWIFTPM_EXEC_NAME"]) { - fatalError("swift-package-manager launched with unexpected name: \(execName ?? "(unknown)")") - } + fatalError("swift-package-manager launched with unexpected name: \(execName ?? "(unknown)")") } return true diff --git a/Tests/CommandsTests/RunToolTests.swift b/Tests/CommandsTests/RunToolTests.swift index d2089e023c2..963f3a0cb48 100644 --- a/Tests/CommandsTests/RunToolTests.swift +++ b/Tests/CommandsTests/RunToolTests.swift @@ -16,6 +16,7 @@ import SPMTestSupport import XCTest import class TSCBasic.Process +import enum TSCBasic.ProcessEnv final class RunToolTests: CommandsTestCase { @@ -103,6 +104,12 @@ final class RunToolTests: CommandsTestCase { } func testSwiftRunSIGINT() throws { + // FIXME: not sure how this was supposed to work previously, since `swift-run` uses `execv` that doesn't inherit + // signal handlers, and the spawned process doesn't install signal handlers on its own. `async` effect on + // `@main` arguably makes it behave correctly? + + throw XCTSkip("desired logic should be clarified") + try XCTSkipIfCI() try fixture(name: "Miscellaneous/SwiftRun") { fixturePath in let mainFilePath = fixturePath.appending("main.swift") @@ -122,8 +129,12 @@ final class RunToolTests: CommandsTestCase { let sync = DispatchGroup() let outputHandler = OutputHandler(sync: sync) + + var environmentBlock = ProcessEnv.block + environmentBlock["SWIFTPM_EXEC_NAME"] = "swift-run" let process = Process( arguments: [SwiftPM.Run.xctestBinaryPath.pathString, "--package-path", fixturePath.pathString], + environmentBlock: environmentBlock, outputRedirection: .stream(stdout: outputHandler.handle(bytes:), stderr: outputHandler.handle(bytes:)) ) From 6fa390701a605b3cd1070964c73bd3bab0ce0efc Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Mon, 26 Feb 2024 13:10:50 +0000 Subject: [PATCH 3/4] Address PR feedback --- Sources/Commands/SwiftRunCommand.swift | 21 ++++++++++++++----- Sources/swift-package-manager/SwiftPM.swift | 2 +- Sources/swift-run/CMakeLists.txt | 5 ++++- .../{main.swift => Entrypoint.swift} | 7 ++++++- Tests/CommandsTests/RunCommandTests.swift | 6 ------ 5 files changed, 27 insertions(+), 14 deletions(-) rename Sources/swift-run/{main.swift => Entrypoint.swift} (83%) diff --git a/Sources/Commands/SwiftRunCommand.swift b/Sources/Commands/SwiftRunCommand.swift index bbf480ab23e..6e2ad06bdf1 100644 --- a/Sources/Commands/SwiftRunCommand.swift +++ b/Sources/Commands/SwiftRunCommand.swift @@ -90,7 +90,7 @@ struct RunCommandOptions: ParsableArguments { } /// swift-run command namespace -public struct SwiftRunCommand: SwiftCommand { +public struct SwiftRunCommand: AsyncSwiftCommand { public static var configuration = CommandConfiguration( commandName: "run", _superCommandName: "swift", @@ -109,7 +109,7 @@ public struct SwiftRunCommand: SwiftCommand { return .init(wantsREPLProduct: options.mode == .repl) } - public func run(_ swiftCommandState: SwiftCommandState) throws { + public func run(_ swiftCommandState: SwiftCommandState) async throws { if options.shouldBuildTests && options.shouldSkipBuild { swiftCommandState.observabilityScope.emit( .mutuallyExclusiveArgumentsError(arguments: ["--build-tests", "--skip-build"]) @@ -284,9 +284,20 @@ public struct SwiftRunCommand: SwiftCommand { /// A safe wrapper of TSCBasic.exec. private func execute(path: String, args: [String]) throws -> Never { #if !os(Windows) - // On platforms other than Windows, signal(SIGINT, SIG_IGN) is used for handling SIGINT by DispatchSourceSignal, - // but this process is about to be replaced by exec, so SIG_IGN must be returned to default. - signal(SIGINT, SIG_DFL) + // Dispatch will disable almost all asynchronous signals on its worker threads, and this is called from `async` + // context. To correctly `exec` a process, we will need to: + // 1. reset the signal masks + for i in 1.. Date: Mon, 26 Feb 2024 13:12:10 +0000 Subject: [PATCH 4/4] Clean up comment wording --- Sources/Commands/SwiftRunCommand.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Commands/SwiftRunCommand.swift b/Sources/Commands/SwiftRunCommand.swift index 6e2ad06bdf1..de7e9077090 100644 --- a/Sources/Commands/SwiftRunCommand.swift +++ b/Sources/Commands/SwiftRunCommand.swift @@ -285,7 +285,7 @@ public struct SwiftRunCommand: AsyncSwiftCommand { private func execute(path: String, args: [String]) throws -> Never { #if !os(Windows) // Dispatch will disable almost all asynchronous signals on its worker threads, and this is called from `async` - // context. To correctly `exec` a process, we will need to: + // context. To correctly `exec` a freshly built binary, we will need to: // 1. reset the signal masks for i in 1..