Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions crates/uv/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ url = { workspace = true }
walkdir = { workspace = true }
which = { workspace = true }
zip = { workspace = true }
cfg-if = "1.0"

[dev-dependencies]
assert_cmd = { version = "2.0.16" }
Expand All @@ -115,6 +116,9 @@ similar = { version = "2.6.0" }
tempfile = { workspace = true }
zip = { workspace = true }

[target.'cfg(unix)'.dependencies]
nix = "0.23"

[package.metadata.cargo-shear]
ignored = [
"flate2",
Expand Down
39 changes: 37 additions & 2 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use std::path::{Path, PathBuf};

use anstream::eprint;
use anyhow::{anyhow, bail, Context};
use cfg_if::cfg_if;
use futures::StreamExt;
use itertools::Itertools;
use owo_colors::OwoColorize;
use tokio::process::Command;
use tokio::process::{Child, Command};
use tracing::{debug, warn};
use url::Url;
use uv_cache::Cache;
Expand Down Expand Up @@ -996,9 +997,27 @@ pub(crate) async fn run(
// signal handlers after the command completes.
let _handler = tokio::spawn(async { while tokio::signal::ctrl_c().await.is_ok() {} });

let status = handle.wait().await.context("Child process disappeared")?;
#[cfg(unix)]
{
use tokio::select;
use tokio::signal::unix::{signal, SignalKind};
let mut term_signal = signal(SignalKind::terminate())?;
loop {
select! {
_ = handle.wait() => {
break
},

// `SIGTERM`
_ = term_signal.recv() => {
let _ = terminate_process(&mut handle);
}
};
}
}

// Exit based on the result of the command
let status = handle.wait().await?;
Copy link
Member

Choose a reason for hiding this comment

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

Is it "fine" / necessary for us to be calling .wait() twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a rust expert would appreciate your guidance here, from what I see inside of wait() it's not expensive to call it twice and it makes the logic in the select! block easier to read. If I do something like let status = select! {...} each brand needs to return the same type, but terminate_process doesn't actually return anything, so I'd need to fake a return which also seems unnecessary. wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like it is guaranteed to return the same result both times.

Copy link
Member

Choose a reason for hiding this comment

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

I went with returning the result directly on unix and only calling let status = handle.wait().await?; on other platforms, since it didn't seem too difficult and makes the control flow a bit clearer IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, agreed this looks better 👍

if let Some(code) = status.code() {
debug!("Command exited with code: {code}");
if let Ok(code) = u8::try_from(code) {
Expand All @@ -1017,6 +1036,22 @@ pub(crate) async fn run(
}
}

#[allow(clippy::items_after_statements, dead_code)]
fn terminate_process(_child: &mut Child) -> Result<(), anyhow::Error> {
cfg_if! {
if #[cfg(unix)] {
let pid = _child.id().context("Failed to get child process ID")?;
use nix::sys::signal::{self, Signal};
use nix::unistd::Pid;
signal::kill(Pid::from_raw(pid.try_into().unwrap()), Signal::SIGTERM)
.context("Failed to send SIGTERM")
} else if #[cfg(windows)] {
// On Windows, use winapi to terminate the process gracefully
todo!("Implement graceful termination on Windows");
}
}
}

/// Returns `true` if we can skip creating an additional ephemeral environment in `uv run`.
fn can_skip_ephemeral(
spec: Option<&RequirementsSpecification>,
Expand Down
39 changes: 38 additions & 1 deletion crates/uv/src/commands/tool/run.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use cfg_if::cfg_if;
use std::fmt::Display;
use std::fmt::Write;
use std::path::PathBuf;
Expand All @@ -7,6 +8,7 @@ use anstream::eprint;
use anyhow::{bail, Context};
use itertools::Itertools;
use owo_colors::OwoColorize;
use tokio::process::Child;
use tokio::process::Command;
use tracing::{debug, warn};

Expand Down Expand Up @@ -236,9 +238,27 @@ pub(crate) async fn run(
// signal handlers after the command completes.
let _handler = tokio::spawn(async { while tokio::signal::ctrl_c().await.is_ok() {} });

let status = handle.wait().await.context("Child process disappeared")?;
#[cfg(unix)]
{
use tokio::select;
use tokio::signal::unix::{signal, SignalKind};
let mut term_signal = signal(SignalKind::terminate())?;
loop {
select! {
_ = handle.wait() => {
break
},

// `SIGTERM`
_ = term_signal.recv() => {
let _ = terminate_process(&mut handle);
}
};
}
}

// Exit based on the result of the command
let status = handle.wait().await?;
if let Some(code) = status.code() {
debug!("Command exited with code: {code}");
if let Ok(code) = u8::try_from(code) {
Expand All @@ -257,6 +277,23 @@ pub(crate) async fn run(
}
}

#[allow(clippy::items_after_statements, dead_code)]
fn terminate_process(_child: &mut Child) -> Result<(), anyhow::Error> {
cfg_if! {
if #[cfg(unix)] {
let pid = _child.id().context("Failed to get child process ID")?;
// On Unix, send SIGTERM for a graceful shutdown
use nix::sys::signal::{self, Signal};
use nix::unistd::Pid;
signal::kill(Pid::from_raw(pid.try_into().unwrap()), Signal::SIGTERM)
.context("Failed to send SIGTERM")
} else if #[cfg(windows)] {
// On Windows, use winapi to terminate the process gracefully
todo!("Implement graceful termination on Windows");
}
}
}

/// Return the entry points for the specified package.
fn get_entrypoints(
from: &PackageName,
Expand Down
Loading