Skip to content

Allow swift-build to have async entrypoint #7369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 26, 2024
Merged

Allow swift-build to have async entrypoint #7369

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
443d884
Allow `swift-build` to have `async` entrypoint
MaxDesiatov Feb 24, 2024
b49a95e
Clean up `main` entrypoint of `swift-package-manager`
MaxDesiatov Feb 24, 2024
bd35a7a
Merge branch 'main' of github.com:apple/swift-package-manager into ma…
MaxDesiatov Feb 25, 2024
6fa3907
Address PR feedback
MaxDesiatov Feb 26, 2024
92f56c7
Clean up comment wording
MaxDesiatov Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,7 @@ package.targets.append(contentsOf: [
.testTarget(
name: "FunctionalPerformanceTests",
dependencies: [
"swift-build",
"swift-package",
"swift-test",
"swift-package-manager",
"SPMTestSupport"
]
),
Expand All @@ -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"
]
Expand All @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions Sources/Commands/SwiftBuildCommand.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ struct BuildCommandOptions: ParsableArguments {
}

/// swift-build command namespace
public struct SwiftBuildCommand: SwiftCommand {
public struct SwiftBuildCommand: AsyncSwiftCommand {
public static var configuration = CommandConfiguration(
commandName: "build",
_superCommandName: "swift",
Expand All @@ -110,7 +110,7 @@ public struct SwiftBuildCommand: SwiftCommand {
@OptionGroup()
var options: BuildCommandOptions

public func run(_ swiftCommandState: SwiftCommandState) throws {
public func run(_ swiftCommandState: SwiftCommandState) async throws {
if options.shouldPrintBinPath {
return try print(swiftCommandState.productsBuildParameters.buildPath.description)
}
Expand Down
11 changes: 6 additions & 5 deletions Sources/SPMTestSupport/SwiftPMProduct.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 4 additions & 1 deletion Sources/swift-build/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
7 changes: 6 additions & 1 deletion Sources/swift-build/main.swift → Sources/swift-build/Entrypoint.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,9 @@

import Commands

SwiftBuildCommand.main()
@main
struct Entrypoint {
static func main() async {
await SwiftBuildCommand.main()
}
}
18 changes: 16 additions & 2 deletions Sources/swift-package-manager/SwiftPM.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,29 @@ import PackageCollectionsCommand
import PackageRegistryCommand

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 {
// 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
private static func main(execName: String?) async -> Bool {
switch execName {
case "swift-package":
await SwiftPackageCommand.main()
case "swift-build":
SwiftBuildCommand.main()
await SwiftBuildCommand.main()
case "swift-experimental-sdk":
await SwiftSDKCommand.main()
case "swift-test":
Expand All @@ -41,5 +53,7 @@ struct SwiftPM {
default:
fatalError("swift-package-manager launched with unexpected name: \(execName ?? "(unknown)")")
}

return true
}
}
11 changes: 11 additions & 0 deletions Tests/CommandsTests/RunCommandTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import SPMTestSupport
import XCTest

import class TSCBasic.Process
import enum TSCBasic.ProcessEnv

final class RunCommandTests: CommandsTestCase {
private func execute(
Expand Down Expand Up @@ -102,6 +103,12 @@ final class RunCommandTests: 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?
Copy link
Member

@kateinoigakukun kateinoigakukun Feb 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this issue is not only a problem in tests but causes a real problem. I could reproduce this uninterruptable swift-run when invoking just built swift-package-manager through symlink named swift-run.

Based on my quick research, it seems like libdispatch blocks SIGINT by sigmask and spawns a dedicated thread to catch those signals. I think that's why we haven't seen this before with sync entrypoints.

So the problematic code can be minimized as follows.

import Dispatch
import Foundation

DispatchQueue.main.async {
  let program = "/usr/bin/sleep"
  let args = ["sleep", "10"]

  let cArgs = UnsafeMutablePointer<UnsafeMutablePointer<Int8>?>.allocate(capacity: args.count + 1)
  for (i, arg) in args.enumerated() {
    cArgs[i] = strdup(arg)
  }
  cArgs[args.count] = nil

  execv(program, cArgs)
  perror("execv")
}

dispatchMain()

I confirmed we can unblock signals as follows, but I'm not 100% sure it's a safe way.

var mask = sigset_t()
sigemptyset(&mask)
pthread_sigmask(SIG_SETMASK, &mask, nil)
execv(program, cArgs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC Dispatch blocks delivery of SIGINT whenever some code is executed in DispatchQueue.main.async, is that correct? Isn't it a bug in Dispatch then and could be fixed on their side?

Copy link
Member

@kateinoigakukun kateinoigakukun Feb 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, libdispatch blocks deliverry of SIGINT during the whole program execution, but it creates a dedicated thread to listen to signals before blocking the signals in the main thread. SIGINT is usually handled by the dedicated thread and I guess the actual signal handling works are scheduled on some dispatch queues. So this is not a bug in libdispatch at all, but they don't provide a graceful way to reset signal mask states. Therefore I suggested resetting masks manually just before execv in SwiftPM as one of the options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I've addressed it in a subsequent commit. The signals reset code had to be slightly different for it to work in our swift run use case and I'm also closing file descriptors as suggested by @weissi


throw XCTSkip("desired logic should be clarified")

try XCTSkipIfCI()
try fixture(name: "Miscellaneous/SwiftRun") { fixturePath in
let mainFilePath = fixturePath.appending("main.swift")
Expand All @@ -121,8 +128,12 @@ final class RunCommandTests: 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:))
)

Expand Down