Refactor utilities under Foundation.Process and CartonHelpers.Process…

… and share implementations (#477) * organizer process utilities * resurrect ProcessResult.checkNonZeroExit * import Foundation * use SIGINT * remove didExit and tweak bundle completion message
swiftwasm · Jun 4, 2024 · 924e171 · 924e171
1 parent 5eebe1f
commit 924e171
Show file tree

Hide file tree

Showing 11 changed files with 139 additions and 114 deletions.
diff --git a/Plugins/CartonBundlePlugin/CartonBundlePluginCommand.swift b/Plugins/CartonBundlePlugin/CartonBundlePluginCommand.swift
@@ -85,12 +85,6 @@ struct CartonBundlePluginCommand: CommandPlugin {
         ["--resources", $0.string]
       } + extractor.remainingArguments
     let frontend = try makeCartonFrontendProcess(context: context, arguments: frontendArguments)
-    frontend.forwardTerminationSignals()
-    try frontend.run()
-    frontend.waitUntilExit()
-    if frontend.terminationStatus == 0 {
-      print("Bundle written in \(bundleDirectory)")
-    }
-    frontend.checkNonZeroExit()
+    try frontend.checkRun(printsLoadingMessage: false, forwardExit: true)
   }
 }
diff --git a/Plugins/CartonDevPlugin/CartonDevPluginCommand.swift b/Plugins/CartonDevPlugin/CartonDevPluginCommand.swift
@@ -114,7 +114,7 @@ struct CartonDevPluginCommand: CommandPlugin {
     }
 
     frontend.waitUntilExit()
-    frontend.checkNonZeroExit()
+    frontend.forwardExit()
   }
 
   private func defaultProduct(context: PluginContext) throws -> String {

diff --git a/Plugins/CartonPluginShared/PluginShared.swift b/Plugins/CartonPluginShared/PluginShared.swift
@@ -184,10 +184,7 @@ internal func checkHelpFlag(_ arguments: [String], subcommand: String, context:
   if arguments.contains("--help") || arguments.contains("-h") {
     let frontend = try makeCartonFrontendProcess(
       context: context, arguments: [subcommand, "--help"])
-    frontend.forwardTerminationSignals()
-    try frontend.run()
-    frontend.waitUntilExit()
-    exit(frontend.terminationStatus)
+    try frontend.checkRun(printsLoadingMessage: false, forwardExit: true)
   }
 }
 
@@ -203,36 +200,3 @@ internal func makeCartonFrontendProcess(context: PluginContext, arguments: [Stri
   process.arguments = arguments
   return process
 }
-
-internal func runCartonFrontend(context: PluginContext, arguments: [String]) throws -> Process {
-  let process = try makeCartonFrontendProcess(context: context, arguments: arguments)
-  try process.run()
-  return process
-}
-
-extension Process {
-  internal func forwardTerminationSignals() {
-    // Monitor termination/interrruption signals to forward them to child process
-    func setSignalForwarding(_ signalNo: Int32) {
-      signal(signalNo, SIG_IGN)
-      let signalSource = DispatchSource.makeSignalSource(signal: signalNo)
-      signalSource.setEventHandler {
-        signalSource.cancel()
-        self.interrupt()
-      }
-      signalSource.resume()
-    }
-    setSignalForwarding(SIGINT)
-    setSignalForwarding(SIGTERM)
-
-    self.terminationHandler = {
-      // Exit plugin process itself when child process exited
-      exit($0.terminationStatus)
-    }
-  }
-  internal func checkNonZeroExit() {
-    if terminationStatus != 0 {
-      exit(terminationStatus)
-    }
-  }
-}
diff --git a/Plugins/CartonTestPlugin/CartonTestPluginCommand.swift b/Plugins/CartonTestPlugin/CartonTestPluginCommand.swift
@@ -109,10 +109,7 @@ struct CartonTestPluginCommand: CommandPlugin {
         ["--resources", $0.string]
       } + extractor.remainingArguments
     let frontend = try makeCartonFrontendProcess(context: context, arguments: frontendArguments)
-    frontend.forwardTerminationSignals()
-    try frontend.run()
-    frontend.waitUntilExit()
-    frontend.checkNonZeroExit()
+    try frontend.checkRun(printsLoadingMessage: false, forwardExit: true)
   }
 
   private func buildDirectory(context: PluginContext) throws -> Path {

diff --git a/Sources/CartonCore/CartonCoreError.swift b/Sources/CartonCore/CartonCoreError.swift
@@ -0,0 +1,6 @@
+struct CartonCoreError: Error & CustomStringConvertible {
+  init(_ description: String) {
+    self.description = description
+  }
+  var description: String
+}
diff --git a/Sources/CartonCore/FoundationProcessEx.swift b/Sources/CartonCore/FoundationProcessEx.swift
@@ -0,0 +1,85 @@
+import Foundation
+
+extension Foundation.Process {
+  // Monitor termination/interrruption signals to forward them to child process
+  public func setSignalForwarding(_ signalNo: Int32) {
+    signal(signalNo, SIG_IGN)
+    let signalSource = DispatchSource.makeSignalSource(signal: signalNo)
+    signalSource.setEventHandler { [self] in
+      signalSource.cancel()
+      kill(processIdentifier, signalNo)
+    }
+    signalSource.resume()
+  }
+
+  public func forwardTerminationSignals() {
+    setSignalForwarding(SIGINT)
+    setSignalForwarding(SIGTERM)
+  }
+
+  public var commandLine: String {
+    get throws {
+      guard let executableURL else {
+        throw CartonCoreError("executableURL is none")
+      }
+
+      let commandLineArgs: [String] = [
+        executableURL.path
+      ] + (arguments ?? [])
+
+      let q = "\""
+      let commandLine: String = commandLineArgs
+        .map { "\(q)\($0)\(q)" }
+        .joined(separator: " ")
+
+      return commandLine
+    }
+  }
+
+  public func checkRun(
+    printsLoadingMessage: Bool,
+    forwardExit: Bool = false
+  ) throws {
+    if printsLoadingMessage {
+      print("Running \(try commandLine)")
+    }
+
+    try run()
+    forwardTerminationSignals()
+    waitUntilExit()
+
+    if forwardExit {
+      self.forwardExit()
+    }
+
+    try checkNonZeroExit()
+  }
+
+  public func forwardExit() {
+    exit(terminationStatus)
+  }
+
+  public func checkNonZeroExit() throws {
+    if terminationStatus != 0 {
+      throw CartonCoreError(
+        "Process failed with status \(terminationStatus).\n" +
+        "Command line: \(try commandLine)"
+      )
+    }
+  }
+
+  public static func checkRun(
+    _ executableURL: URL,
+    arguments: [String],
+    printsLoadingMessage: Bool = true,
+    forwardExit: Bool = false
+  ) throws {
+    let process = Foundation.Process()
+    process.executableURL = executableURL
+    process.arguments = arguments
+    try process.checkRun(
+      printsLoadingMessage: printsLoadingMessage,
+      forwardExit: forwardExit
+    )
+  }
+}
diff --git a/Sources/CartonDriver/CartonDriverCommand.swift b/Sources/CartonDriver/CartonDriverCommand.swift
@@ -30,6 +30,7 @@
 //
 // This executable should be eventually removed once SwiftPM provides a way to express those requirements.
 
+import CartonCore
 import CartonHelpers
 import Foundation
 import SwiftToolchain
@@ -41,49 +42,6 @@ struct CartonDriverError: Error & CustomStringConvertible {
   var description: String
 }
 
-extension Foundation.Process {
-  internal static func checkRun(
-    _ executableURL: URL, arguments: [String], forwardExit: Bool = false
-  ) throws {
-    let commandLine: String = ([executableURL.path] + arguments)
-      .map { "\"\($0)\"" }.joined(separator: " ")
-
-    fputs("Running \(commandLine)\n", stderr)
-    fflush(stderr)
-
-    let process = Foundation.Process()
-    process.executableURL = executableURL
-    process.arguments = arguments
-
-    // Monitor termination/interrruption signals to forward them to child process
-    func setSignalForwarding(_ signalNo: Int32) {
-      signal(signalNo, SIG_IGN)
-      let signalSource = DispatchSource.makeSignalSource(signal: signalNo)
-      signalSource.setEventHandler {
-        signalSource.cancel()
-        process.interrupt()
-      }
-      signalSource.resume()
-    }
-    setSignalForwarding(SIGINT)
-    setSignalForwarding(SIGTERM)
-
-    try process.run()
-    process.waitUntilExit()
-
-    if forwardExit {
-      exit(process.terminationStatus)
-    }
-
-    if process.terminationStatus != 0 {
-      throw CartonDriverError(
-        "Process failed with status \(process.terminationStatus).\n" +
-        "Command line: \(commandLine)"
-      )
-    }
-  }
-}
-
 func derivePackageCommandArguments(
   swiftExec: URL,
   subcommand: String,
@@ -184,7 +142,10 @@ func pluginSubcommand(subcommand: String, argv0: String, arguments: [String]) as
     extraArguments: extraArguments
   )
 
-  try Foundation.Process.checkRun(swiftExec, arguments: pluginArguments, forwardExit: true)
+  try Foundation.Process.checkRun(
+    swiftExec, arguments: pluginArguments,
+    forwardExit: true
+  )
 }
 
 public func main(arguments: [String]) async throws {
@@ -213,7 +174,8 @@ public func main(arguments: [String]) async throws {
     let (swiftPath, _) = try await toolchainSystem.inferSwiftPath(terminal)
     try Foundation.Process.checkRun(
       URL(fileURLWithPath: swiftPath.pathString),
-      arguments: ["package"] + arguments.dropFirst(), forwardExit: true
+      arguments: ["package"] + arguments.dropFirst(),
+      forwardExit: true
     )
   case "--version":
     print(cartonVersion)

diff --git a/Sources/CartonFrontend/Commands/CartonFrontendBundleCommand.swift b/Sources/CartonFrontend/Commands/CartonFrontendBundleCommand.swift
@@ -135,7 +135,7 @@ struct CartonFrontendBundleCommand: AsyncParsableCommand {
       topLevelResourcePaths: resources
     )
 
-    terminal.write("Bundle generation finished successfully\n", inColor: .green, bold: true)
+    terminal.write("Bundle successfully generated at \(bundleDirectory)\n", inColor: .green, bold: true)
   }
 
   func optimize(_ inputPath: AbsolutePath, outputPath: AbsolutePath, terminal: InteractiveWriter)

diff --git a/Sources/CartonHelpers/ProcessEx.swift b/Sources/CartonHelpers/ProcessEx.swift
@@ -1,3 +1,6 @@
+import Foundation
+import Dispatch
+
 extension ProcessResult {
   public mutating func setOutput(_ value: Result<[UInt8], any Swift.Error>) {
     self = ProcessResult(
@@ -17,3 +20,28 @@ extension ProcessResult {
     return try utf8Output()
   }
 }
+
+@discardableResult
+private func osSignal(
+  _ sig: Int32,
+  _ fn: (@convention(c) (Int32) -> Void)?
+) -> (@convention(c) (Int32) -> Void)? {
+  signal(sig, fn)
+}
+
+extension Process {
+  public func setSignalForwarding(_ signalNo: Int32) {
+    osSignal(signalNo, SIG_IGN)
+    let signalSource = DispatchSource.makeSignalSource(signal: signalNo)
+    signalSource.setEventHandler {
+      signalSource.cancel()
+      self.signal(signalNo)
+    }
+    signalSource.resume()
+  }
+
+  public func forwardTerminationSignals() {
+    setSignalForwarding(SIGINT)
+    setSignalForwarding(SIGTERM)
+  }
+}
diff --git a/Tests/CartonCommandTests/CommandTestHelper.swift b/Tests/CartonCommandTests/CommandTestHelper.swift
@@ -100,17 +100,7 @@ func swiftRunProcess(
 
   try process.launch()
 
-  func setSignalForwarding(_ signalNo: Int32) {
-    signal(signalNo, SIG_IGN)
-    let signalSource = DispatchSource.makeSignalSource(signal: signalNo)
-    signalSource.setEventHandler {
-      signalSource.cancel()
-      process.signal(SIGINT)
-    }
-    signalSource.resume()
-  }
-  setSignalForwarding(SIGINT)
-  setSignalForwarding(SIGTERM)
+  process.forwardTerminationSignals()
 
   return SwiftRunProcess(
     process: process,
@@ -137,7 +127,7 @@ func fetchWebContent(at url: URL, timeout: Duration) async throws -> (response:
   )
 
   let (body, response) = try await session.data(for: request)
-  
+
   guard let response = response as? HTTPURLResponse else {
     throw CommandTestError("Response from \(url.absoluteString) is not HTTPURLResponse")
   }

diff --git a/Tests/CartonCommandTests/DevCommandTests.swift b/Tests/CartonCommandTests/DevCommandTests.swift
@@ -43,24 +43,23 @@ final class DevCommandTests: XCTestCase {
       // FIXME: Don't assume a specific port is available since it can be used by others or tests
       try await withFixture("EchoExecutable") { packageDirectory in
         let process = try swiftRunProcess(
-          ["carton", "dev", "--verbose", "--port", "8081", "--skip-auto-open"],
+          ["carton", "dev", "--verbose", "--port", "8080", "--skip-auto-open"],
           packageDirectory: packageDirectory.asURL
         )
 
-        try await checkForExpectedContent(process: process, at: "http://127.0.0.1:8081")
+        try await checkForExpectedContent(process: process, at: "http://127.0.0.1:8080")
       }
     }
   #endif
 
   private func fetchDevServerWithRetry(at url: URL) async throws -> (response: HTTPURLResponse, body: Data) {
     // client time out for connecting and responding
-    let timeOut: Duration = .seconds(60)
+    let timeOut: Duration = .seconds(10)
 
     // client delay... let the server start up
-    let delay: Duration = .seconds(30)
+    let delay: Duration = .seconds(3)
 
-    // only try 5 times.
-    let count = 5
+    let count = 100
 
     do {
       return try await withRetry(maxAttempts: count, initialDelay: delay, retryInterval: delay) {
@@ -78,7 +77,7 @@ final class DevCommandTests: XCTestCase {
   func checkForExpectedContent(process: SwiftRunProcess, at url: String) async throws {
     defer {
       // end the process regardless of success
-      process.process.signal(SIGTERM)
+      process.process.signal(SIGINT)
     }
 
     let (response, data) = try await fetchDevServerWithRetry(at: try URL(string: url).unwrap("url"))