diff --git a/crates/tower-runtime/src/local.rs b/crates/tower-runtime/src/local.rs index 745c4b57..e388fb04 100644 --- a/crates/tower-runtime/src/local.rs +++ b/crates/tower-runtime/src/local.rs @@ -256,33 +256,15 @@ async fn execute_local_app( } } } - Ok(mut child) => { - // Drain the logs to the output channel. - let stdout = child.stdout.take().expect("no stdout"); - tokio::spawn(drain_output( - FD::Stdout, - Channel::Setup, - opts.output_sender.clone(), - BufReader::new(stdout), - )); - - let stderr = child.stderr.take().expect("no stderr"); - tokio::spawn(drain_output( - FD::Stderr, - Channel::Setup, - opts.output_sender.clone(), - BufReader::new(stderr), - )); - - // Let's wait for the setup to finish. We don't care about the results. - let mut res = wait_for_process(ctx.clone(), &cancel_token, child).await; - - // If the requirements.txt install failed, retry with the legacy - // setuptools<82 pin. Some apps (those whose transitive deps rely on + Ok(child) => { + let mut res = run_setup_child(&ctx, &cancel_token, &opts.output_sender, child).await; + + // If the install failed, retry with the legacy setuptools<82 + // pin. Some apps (those whose transitive deps rely on // pkg_resources) need that pin to install successfully; we don't // apply it by default because it conflicts with apps whose deps // require setuptools>=82. - if res != 0 && uv.should_use_legacy_setuptools_pin(&working_dir) { + if res != 0 { let _ = opts.output_sender.send(Output { channel: Channel::Setup, fd: FD::Stdout, @@ -290,33 +272,10 @@ async fn execute_local_app( time: chrono::Utc::now(), }); - match uv + let retry_child = uv .sync_with_legacy_setuptools_pin(&working_dir, &env_vars) - .await - { - Err(e) => { - return Err(e.into()); - } - Ok(mut retry_child) => { - let stdout = retry_child.stdout.take().expect("no stdout"); - tokio::spawn(drain_output( - FD::Stdout, - Channel::Setup, - opts.output_sender.clone(), - BufReader::new(stdout), - )); - - let stderr = retry_child.stderr.take().expect("no stderr"); - tokio::spawn(drain_output( - FD::Stderr, - Channel::Setup, - opts.output_sender.clone(), - BufReader::new(stderr), - )); - - res = wait_for_process(ctx.clone(), &cancel_token, retry_child).await; - } - } + .await?; + res = run_setup_child(&ctx, &cancel_token, &opts.output_sender, retry_child).await; } if res != 0 { @@ -574,6 +533,31 @@ async fn kill_child_process(ctx: &tower_telemetry::Context, mut child: Child) { }; } +async fn run_setup_child( + ctx: &tower_telemetry::Context, + cancel_token: &CancellationToken, + output_sender: &OutputSender, + mut child: Child, +) -> i32 { + let stdout = child.stdout.take().expect("no stdout"); + tokio::spawn(drain_output( + FD::Stdout, + Channel::Setup, + output_sender.clone(), + BufReader::new(stdout), + )); + + let stderr = child.stderr.take().expect("no stderr"); + tokio::spawn(drain_output( + FD::Stderr, + Channel::Setup, + output_sender.clone(), + BufReader::new(stderr), + )); + + wait_for_process(ctx.clone(), cancel_token, child).await +} + async fn wait_for_process( ctx: tower_telemetry::Context, cancel_token: &CancellationToken, diff --git a/crates/tower-uv/src/lib.rs b/crates/tower-uv/src/lib.rs index a2e44bda..203fddab 100644 --- a/crates/tower-uv/src/lib.rs +++ b/crates/tower-uv/src/lib.rs @@ -357,29 +357,55 @@ impl Uv { &self.uv_path, cwd ); - self.spawn_requirements_install(cwd, env_vars, false).await + let mut cmd = self.pip_install(cwd); + cmd.arg("-r") + .arg(cwd.join("requirements.txt")) + .envs(env_vars); + + #[cfg(unix)] + { + cmd.process_group(0); + } + + if let Some(dir) = &self.cache_dir { + cmd.arg("--cache-dir").arg(dir); + } + + Ok(cmd.spawn()?) } else { // If there is no pyproject.toml or requirements.txt, then we can't sync. Err(Error::MissingPyprojectToml) } } - /// Returns whether a failed `sync()` for this directory is eligible for a - /// retry via [`sync_with_legacy_setuptools_pin`]. Only applies to projects - /// driven by `requirements.txt`; pyproject-based projects manage their own - /// setuptools dependency. - pub fn should_use_legacy_setuptools_pin(&self, cwd: &Path) -> bool { - cwd.join("requirements.txt").exists() + /// Builds a `uv pip install` command with our standard stdio/color setup. + /// Callers append source args (e.g. `-r requirements.txt` or `.`), any extra + /// packages, and `envs` before spawning. + fn pip_install(&self, cwd: &Path) -> Command { + let mut cmd = Command::new(&self.uv_path); + cmd.kill_on_drop(true) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .current_dir(cwd) + .arg("--color") + .arg("never") + .arg("pip") + .arg("install"); + cmd } - /// Re-runs the `requirements.txt` install with a `setuptools<82` pin appended. + /// Re-runs the install with a `setuptools<82` pin appended. /// /// setuptools 82 removed `pkg_resources`, but many legacy packages still import /// it without declaring the dependency. Pinning `setuptools<82` keeps it /// available. Some modern packages (e.g. dlt's transitive graph pinning /// `setuptools==82.0.1`) make this pin unsatisfiable, so it isn't applied up - /// front — callers should fall back to this only after a plain `sync()` - /// fails for a project using `requirements.txt`. + /// front — callers should fall back to this only after a plain `sync()` fails. + /// + /// Drops out of `uv sync` (which can't accept a CLI constraint) into `uv pip + /// install`, which can. The project source is `.` for a pyproject project or + /// `-r requirements.txt` otherwise. /// /// https://github.com/pypa/setuptools/issues/5174 pub async fn sync_with_legacy_setuptools_pin( @@ -387,44 +413,22 @@ impl Uv { cwd: &PathBuf, env_vars: &HashMap, ) -> Result { - if !cwd.join("requirements.txt").exists() { - return Err(Error::MissingPyprojectToml); - } - debug!( - "Retrying UV ({:?}) sync with setuptools<82 pin in {:?}", + "Retrying UV ({:?}) install with setuptools<82 pin in {:?}", &self.uv_path, cwd ); - self.spawn_requirements_install(cwd, env_vars, true).await - } - - async fn spawn_requirements_install( - &self, - cwd: &PathBuf, - env_vars: &HashMap, - pin_legacy_setuptools: bool, - ) -> Result { - let req_path = cwd.join("requirements.txt"); - - let mut cmd = Command::new(&self.uv_path); - cmd.kill_on_drop(true) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .current_dir(cwd) - .arg("--color") - .arg("never") - .arg("pip") - .arg("install") - .arg("-r") - .arg(&req_path); + let mut cmd = self.pip_install(cwd); - if pin_legacy_setuptools { - cmd.arg("setuptools<82"); + if cwd.join("pyproject.toml").exists() { + cmd.arg("."); + } else if cwd.join("requirements.txt").exists() { + cmd.arg("-r").arg(cwd.join("requirements.txt")); + } else { + return Err(Error::MissingPyprojectToml); } - cmd.envs(env_vars); + cmd.arg("setuptools<82").envs(env_vars); #[cfg(unix)] { diff --git a/crates/tower-uv/tests/sync_test.rs b/crates/tower-uv/tests/sync_test.rs index b2cc8387..df692164 100644 --- a/crates/tower-uv/tests/sync_test.rs +++ b/crates/tower-uv/tests/sync_test.rs @@ -116,7 +116,7 @@ async fn sync_with_legacy_setuptools_pin_fails_when_user_requires_modern_setupto } #[tokio::test] -async fn sync_with_legacy_setuptools_pin_errors_without_requirements_txt() { +async fn sync_with_legacy_setuptools_pin_errors_without_project_files() { let tmp = TempDir::new().expect("tempdir"); let cwd = tmp.path().to_path_buf(); @@ -125,6 +125,58 @@ async fn sync_with_legacy_setuptools_pin_errors_without_requirements_txt() { let result = uv.sync_with_legacy_setuptools_pin(&cwd, &env_vars).await; assert!( matches!(result, Err(tower_uv::Error::MissingPyprojectToml)), - "fallback should refuse to run without a requirements.txt" + "fallback should refuse to run without pyproject.toml or requirements.txt" + ); +} + +#[tokio::test] +async fn sync_with_legacy_setuptools_pin_fails_for_pyproject_requiring_modern_setuptools() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().to_path_buf(); + + // Mirrors the requirements.txt counterpart: when the project pins + // setuptools>=82, the pin must conflict — proving it's actually applied. + tokio::fs::write( + cwd.join("pyproject.toml"), + "[project]\nname = \"test-app\"\nversion = \"0.0.1\"\nrequires-python = \">=3.10\"\ndependencies = [\"setuptools>=82\"]\n", + ) + .await + .expect("write pyproject.toml"); + + let uv = make_uv_with_venv(&cwd).await; + let env_vars: HashMap = HashMap::new(); + let child = uv + .sync_with_legacy_setuptools_pin(&cwd, &env_vars) + .await + .expect("retry spawn failed"); + let code = wait(child).await; + assert_ne!( + code, 0, + "sync_with_legacy_setuptools_pin should fail when the pyproject project requires setuptools>=82" + ); +} + +#[tokio::test] +async fn sync_with_legacy_setuptools_pin_installs_for_pyproject() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().to_path_buf(); + + tokio::fs::write( + cwd.join("pyproject.toml"), + "[project]\nname = \"test-app\"\nversion = \"0.0.1\"\nrequires-python = \">=3.10\"\ndependencies = [\"six\"]\n", + ) + .await + .expect("write pyproject.toml"); + + let uv = make_uv_with_venv(&cwd).await; + let env_vars: HashMap = HashMap::new(); + let child = uv + .sync_with_legacy_setuptools_pin(&cwd, &env_vars) + .await + .expect("retry spawn failed"); + let code = wait(child).await; + assert_eq!( + code, 0, + "sync_with_legacy_setuptools_pin should succeed for a pyproject project" ); }