-
Notifications
You must be signed in to change notification settings - Fork 290
fix: forward kill signals from pixi to deno task shell commands #3837
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?
Conversation
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.
If this works that would be great! Lets test it throughly!
// fn reset_cursor() { | ||
// let term = console::Term::stdout(); | ||
// let _ = term.show_cursor(); | ||
// } |
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.
Isnt this still an issue?
@@ -0,0 +1,257 @@ | |||
// Copyright 2018-2025 the Deno authors. MIT license. |
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.
Is this used?
@@ -371,6 +370,73 @@ enum TaskExecutionError { | |||
UnsupportedPlatformError(#[from] UnsupportedPlatformError), | |||
} | |||
|
|||
/// Runs a deno task future forwarding any signals received |
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.
deno?
@@ -304,7 +303,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { | |||
} | |||
|
|||
// Handle CTRL-C ourselves again | |||
ctrlc_should_exit_process.store(true, Ordering::Relaxed); | |||
// ctrlc_should_exit_process.store(true, Ordering::Relaxed); |
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.
Please remove commented code.
Using the example in the issue this doesn't allow me to correctly stop the pixi process anymore. import signal
import time
def signal_handler(signum, frame):
print(f"Signal handler called with signal {signum}")
signal.signal(signal.SIGINT, signal_handler)
while True:
time.sleep(1) This will run indefinitely without me being able to stop it. While the old pixi allows me to send If I add
While the latest release gives me:
|
Thanks for trying. Indeed, I also added |
When I run this, I get:
|
The problem for me is that I can't properly quit it when the signal is ignored by the running executable. Pixi just sends it through and doesn't stop. |
We had some logic that ignored CTRL+C on the pixi level. However, my understanding is that it only made CTRL+C "transparent" so that it reached the right process underneath, but didn't properly set up forwarding for CTRL+C and other signals.
By using the
KillSignal
fromdeno_task_shell
we can properly forward any signal received inpixi
to the child processes.Should fix #3789
TODO: