-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Display PID of other running SwiftPM processes #8575
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
base: main
Are you sure you want to change the base?
Display PID of other running SwiftPM processes #8575
Conversation
} catch ProcessLockError.unableToAquireLock(let errno) { | ||
if errno == EWOULDBLOCK { | ||
let lockingPID = try? String(contentsOfFile: self.scratchDirectory.appending(".lock").pathString, encoding: .utf8) | ||
let pidInfo = lockingPID.map { "(PID: \($0))" } ?? "" |
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.
I'd add a space after the closing parenthesis, like so:
let pidInfo = lockingPID.map { "(PID: \($0)) " } ?? ""
...and then correspondingly remove the space after the pifInfo
interpolation a few lines down, like so:
"Another instance of SwiftPM \(pidInfo)is already running using '\(self.scratchDirectory)', but this will be ignored since `--ignore-lock` has been passed"
...so that we don't end up with a double space between "SwiftPM" and "is" if we couldn't read the PID, since that would look out of place.
Thanks for your PR, this is a nice little enhancement! Just had some minor formatting related-feedback. |
@@ -1064,25 +1064,33 @@ public final class SwiftCommandState { | |||
// Try a non-blocking lock first so that we can inform the user about an already running SwiftPM. | |||
do { | |||
try workspaceLock.lock(type: .exclusive, blocking: false) | |||
let pid = ProcessInfo.processInfo.processIdentifier | |||
try String(pid).write(toFile: self.scratchDirectory.appending(".lock").pathString, atomically: true, encoding: .utf8) |
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.
nit: refactor out self.scratchDirectory.appending(".lock").pathString
in to a variable
Just did all the changes, one other thought: would it be better if we mark |
@louisunlimited I think that's a good idea. It's a non-essential call and worst case the message just falls back to what we have today. |
Cool, just pushed the changes, lmk if you need me for anything else |
@swift-ci test |
Great pr Louis! I was working on the same thing too: If you want to take any changes from it to enhance yours please do :) |
Displays the PID of the other SwiftPM instance currently holding the lock
Motivation:
When we try to run different SwiftPM processes we already see notes like
I'll be nice to also have info about PID so I can choose to terminate it or wait for it to finish, so it might display smt like:
Modifications:
My approach was trying to write process id from the process that acquires the lock to the lock file and then let other processes read from it and get the pid.
Result:
If another SwiftPM process is currently running, it now displays: