-
-
Notifications
You must be signed in to change notification settings - Fork 45
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.
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
Refactor utilities under Foundation.Process and CartonHelpers.Process and share implementations #477
Refactor utilities under Foundation.Process and CartonHelpers.Process and share implementations #477
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
struct CartonCoreError: Error & CustomStringConvertible { | ||
init(_ description: String) { | ||
self.description = description | ||
} | ||
var description: String | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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)") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ここですが、元コードだと stderr に出力して flush していたのですが、 |
||
} | ||
|
||
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 | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,7 +129,7 @@ struct CartonFrontendBundleCommand: AsyncParsableCommand { | |
topLevelResourcePaths: resources | ||
) | ||
|
||
terminal.write("Bundle generation finished successfully\n", inColor: .green, bold: true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. このメッセージをfrontendで出した後で、plugin側で改めてパスを出していたため、以下のようなメッセージになっていました。
これを統合し、frontend側で成功メッセージと配置したパスを表示するようにして、
ファイルパスはフルで出るようにしました。 この修正に伴い、didExitコールバックは消しました。 |
||
terminal.write("Bundle successfully generated at \(bundleDirectory)\n", inColor: .green, bold: true) | ||
} | ||
|
||
func optimize(_ inputPath: AbsolutePath, outputPath: AbsolutePath, terminal: InteractiveWriter) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
_ 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ここが元々 |
||
} | ||
signalSource.resume() | ||
} | ||
|
||
public func forwardTerminationSignals() { | ||
setSignalForwarding(SIGINT) | ||
setSignalForwarding(SIGTERM) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ここなんですが、すぐ上の |
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 最初の1回で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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. swiftpm pluginを正しく貫通する SIGINT を使います。 |
||
} | ||
|
||
let (response, data) = try await fetchDevServerWithRetry(at: try URL(string: url).unwrap("url")) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここが元々
interrupt
固定になっていました。