Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Sep 8, 2023

It’s really quite unfortunate that we need to do all this handling ourselves…

@ahoppen ahoppen requested a review from bnbarham September 8, 2023 16:47
@ahoppen
Copy link
Member Author

ahoppen commented Sep 8, 2023

@swift-ci Please test

@ahoppen ahoppen force-pushed the ahoppen/cancellation branch from c2b2e7b to 2a361c9 Compare September 8, 2023 16:54
@ahoppen
Copy link
Member Author

ahoppen commented Sep 8, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Sep 8, 2023

@swift-ci Please test Windows

1 similar comment
@ahoppen
Copy link
Member Author

ahoppen commented Sep 8, 2023

@swift-ci Please test Windows

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

... unfortunate that this isn't handled by Process :(

try command.run()
} catch {
if !SigIntListener.hasReceivedSigInt {
// No point printing an error message if the user requested the termiation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// No point printing an error message if the user requested the termiation
// No point printing an error message if the user requested the termination

signal(SIGINT) { _ in
SigIntListener.hasReceivedSigInt = true
for process in SigIntListener.runningSubprocesses {
kill(process.processIdentifier, SIGINT)
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a Process.terminate and Process.interrupt you could use instead signalling directly

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, nice. I didn’t know about that.

… Ctrl-C

It’s really quite unfortunate that we need to do all this handling ourselves…
@ahoppen ahoppen force-pushed the ahoppen/cancellation branch from 2a361c9 to 04cf52d Compare September 8, 2023 18:46
@ahoppen
Copy link
Member Author

ahoppen commented Sep 9, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Sep 11, 2023

@swift-ci Please test Windows

@ahoppen ahoppen enabled auto-merge September 11, 2023 16:36
@ahoppen ahoppen merged commit b4e72de into swiftlang:main Sep 11, 2023
@ahoppen ahoppen deleted the ahoppen/cancellation branch August 4, 2025 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants