From 142a1fa712fa3770cecdc1cd92fb66f7365f6e3a Mon Sep 17 00:00:00 2001 From: Brad Heller Date: Mon, 27 Apr 2026 14:42:39 +0100 Subject: [PATCH 1/4] Only require setuptools when an initial probe fails --- Cargo.lock | 1 + crates/tower-uv/Cargo.toml | 3 ++ crates/tower-uv/src/lib.rs | 60 +++++++++++++++++---- crates/tower-uv/tests/sync_test.rs | 87 ++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 crates/tower-uv/tests/sync_test.rs diff --git a/Cargo.lock b/Cargo.lock index 210df352..ea2f1342 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3795,6 +3795,7 @@ dependencies = [ "regex", "reqwest", "seahash", + "tempfile", "tokio", "tokio-tar", "tower-telemetry", diff --git a/crates/tower-uv/Cargo.toml b/crates/tower-uv/Cargo.toml index 5c42f83e..328856f1 100644 --- a/crates/tower-uv/Cargo.toml +++ b/crates/tower-uv/Cargo.toml @@ -19,3 +19,6 @@ seahash = { workspace = true } tokio = { workspace = true } tokio-tar = { workspace = true } tower-telemetry = { workspace = true } + +[dev-dependencies] +tempfile = "3.12" diff --git a/crates/tower-uv/src/lib.rs b/crates/tower-uv/src/lib.rs index 94e9b6f9..ed5b1377 100644 --- a/crates/tower-uv/src/lib.rs +++ b/crates/tower-uv/src/lib.rs @@ -357,6 +357,50 @@ impl Uv { &self.uv_path, cwd ); + let req_path = cwd.join("requirements.txt"); + + // setuptools 82 removed pkg_resources, but many legacy packages still + // import it without declaring the dependency. Historically we always + // injected `setuptools<82` to keep pkg_resources available, but that + // pin makes resolution fail for apps using newer deps that require + // setuptools>=82. + // + // Try resolution *without* the pin first (the common, fast path). + // Only if resolution fails do we retry with `setuptools<82` — this + // covers cases like `dlt[motherduck,hub]==1.26.0a1` where the + // unconstrained resolution can't find a working solution but a + // bounded setuptools constraint nudges uv to one. + // https://github.com/pypa/setuptools/issues/5174 + let unpinned_resolves = { + let mut probe = Command::new(&self.uv_path); + probe + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .current_dir(cwd) + .arg("--color") + .arg("never") + .arg("pip") + .arg("install") + .arg("--dry-run") + .arg("-r") + .arg(&req_path) + .envs(env_vars); + + if let Some(dir) = &self.cache_dir { + probe.arg("--cache-dir").arg(dir); + } + + matches!(probe.status().await, Ok(s) if s.success()) + }; + + if !unpinned_resolves { + debug!( + "Falling back to setuptools<82 pin for {:?}: unpinned resolution failed", + cwd + ); + } + // If there is a requirements.txt, then we can use that to sync. let mut cmd = Command::new(&self.uv_path); cmd.kill_on_drop(true) @@ -369,15 +413,13 @@ impl Uv { .arg("pip") .arg("install") .arg("-r") - .arg(cwd.join("requirements.txt")) - // setuptools 82 removed pkg_resources, but many legacy packages - // still import it without declaring the dependency. Let's always install - // a version that includes pkg_resources for requirements.txt, on the - // basis that requirements.txt projects are probably not using the latest - // and greatest deps (then they'd likely be using pyproject.toml anyway) - // https://github.com/pypa/setuptools/issues/5174 - .arg("setuptools<82") - .envs(env_vars); + .arg(&req_path); + + if !unpinned_resolves { + cmd.arg("setuptools<82"); + } + + cmd.envs(env_vars); #[cfg(unix)] { diff --git a/crates/tower-uv/tests/sync_test.rs b/crates/tower-uv/tests/sync_test.rs new file mode 100644 index 00000000..02537889 --- /dev/null +++ b/crates/tower-uv/tests/sync_test.rs @@ -0,0 +1,87 @@ +//! Integration tests for `Uv::sync` requirements.txt handling. +//! +//! These tests focus on the `setuptools<82` injection logic. The default path +//! resolves *without* the pin (most apps work fine with a modern setuptools); +//! the pin is only applied as a fallback when unpinned resolution fails (e.g. +//! `dlt[motherduck,hub]==1.26.0a1`-style transitive constraints). +//! +//! The tests shell out to a real `uv` binary and hit pypi over the network, +//! mirroring the existing `install_test.rs`. + +use std::collections::HashMap; +use std::path::PathBuf; + +use tempfile::TempDir; +use tokio::process::Child; +use tower_uv::Uv; + +async fn wait(mut child: Child) -> i32 { + let status = child.wait().await.expect("wait failed"); + status.code().unwrap_or(-1) +} + +async fn make_uv_with_venv(cwd: &PathBuf) -> Uv { + let uv = Uv::new(None, false).await.expect("Uv::new failed"); + let env_vars: HashMap = HashMap::new(); + let venv_child = uv.venv(cwd, &env_vars).await.expect("venv spawn failed"); + let code = wait(venv_child).await; + assert_eq!(code, 0, "venv creation failed"); + uv +} + +#[tokio::test] +async fn sync_succeeds_without_pin_for_simple_requirements() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().to_path_buf(); + + // Trivial requirements.txt — unpinned resolution succeeds, so we install + // without the setuptools<82 pin. + tokio::fs::write(cwd.join("requirements.txt"), "six\n") + .await + .expect("write requirements.txt"); + + let uv = make_uv_with_venv(&cwd).await; + let env_vars: HashMap = HashMap::new(); + let child = uv.sync(&cwd, &env_vars).await.expect("sync spawn failed"); + let code = wait(child).await; + assert_eq!(code, 0, "sync should succeed for simple requirements.txt"); +} + +#[tokio::test] +async fn sync_succeeds_when_user_requires_modern_setuptools() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().to_path_buf(); + + // Regression test for the dlt scenario: an app that explicitly wants + // setuptools>=82 should resolve cleanly (no pin gets injected). + tokio::fs::write(cwd.join("requirements.txt"), "setuptools>=82\n") + .await + .expect("write requirements.txt"); + + let uv = make_uv_with_venv(&cwd).await; + let env_vars: HashMap = HashMap::new(); + let child = uv.sync(&cwd, &env_vars).await.expect("sync spawn failed"); + let code = wait(child).await; + assert_eq!( + code, 0, + "sync should succeed when the user requires setuptools>=82" + ); +} + +#[tokio::test] +async fn sync_succeeds_when_user_pins_legacy_setuptools() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().to_path_buf(); + + // Apps that need pkg_resources can pin setuptools<82 themselves; our logic + // shouldn't get in the way. + tokio::fs::write(cwd.join("requirements.txt"), "setuptools<82\n") + .await + .expect("write requirements.txt"); + + let uv = make_uv_with_venv(&cwd).await; + let env_vars: HashMap = HashMap::new(); + let child = uv.sync(&cwd, &env_vars).await.expect("sync spawn failed"); + let code = wait(child).await; + assert_eq!(code, 0, "sync should succeed when the user pins setuptools<82"); +} From f9b9132ac21d59a44f24a3db1a9eb933a9e2c846 Mon Sep 17 00:00:00 2001 From: Brad Heller Date: Mon, 27 Apr 2026 14:48:12 +0100 Subject: [PATCH 2/4] All dependencies come from workspace --- Cargo.toml | 1 + crates/tower-uv/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 297f27af..ebc29277 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,7 @@ sha2 = "0.10" snafu = "0.7" tar = "0.4" spinners = "4" +tempfile = "3.12" testutils = { path = "crates/testutils" } tmpdir = "1.0" tokio = { version = "1", features = ["full"] } diff --git a/crates/tower-uv/Cargo.toml b/crates/tower-uv/Cargo.toml index 328856f1..bd6f1381 100644 --- a/crates/tower-uv/Cargo.toml +++ b/crates/tower-uv/Cargo.toml @@ -21,4 +21,4 @@ tokio-tar = { workspace = true } tower-telemetry = { workspace = true } [dev-dependencies] -tempfile = "3.12" +tempfile = { workspace = true } From cbf3315e7308cc4a2cea1517fc55a63feef717c9 Mon Sep 17 00:00:00 2001 From: Brad Heller Date: Mon, 27 Apr 2026 14:50:36 +0100 Subject: [PATCH 3/4] Make commentary a little less specific --- crates/tower-uv/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/tower-uv/src/lib.rs b/crates/tower-uv/src/lib.rs index ed5b1377..1217a496 100644 --- a/crates/tower-uv/src/lib.rs +++ b/crates/tower-uv/src/lib.rs @@ -365,11 +365,6 @@ impl Uv { // pin makes resolution fail for apps using newer deps that require // setuptools>=82. // - // Try resolution *without* the pin first (the common, fast path). - // Only if resolution fails do we retry with `setuptools<82` — this - // covers cases like `dlt[motherduck,hub]==1.26.0a1` where the - // unconstrained resolution can't find a working solution but a - // bounded setuptools constraint nudges uv to one. // https://github.com/pypa/setuptools/issues/5174 let unpinned_resolves = { let mut probe = Command::new(&self.uv_path); From 961a7de5ef511f845127d1b2d977c843c5e15cf6 Mon Sep 17 00:00:00 2001 From: Brad Heller Date: Mon, 27 Apr 2026 15:42:04 +0100 Subject: [PATCH 4/4] Fall back to doing a pinned version install only if the first one fails --- crates/tower-runtime/src/local.rs | 44 ++++++++- crates/tower-uv/src/lib.rs | 138 +++++++++++++++-------------- crates/tower-uv/tests/sync_test.rs | 79 +++++++++++++---- 3 files changed, 175 insertions(+), 86 deletions(-) diff --git a/crates/tower-runtime/src/local.rs b/crates/tower-runtime/src/local.rs index 707d4dbb..745c4b57 100644 --- a/crates/tower-runtime/src/local.rs +++ b/crates/tower-runtime/src/local.rs @@ -275,7 +275,49 @@ async fn execute_local_app( )); // Let's wait for the setup to finish. We don't care about the results. - let res = wait_for_process(ctx.clone(), &cancel_token, child).await; + 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 + // 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) { + let _ = opts.output_sender.send(Output { + channel: Channel::Setup, + fd: FD::Stdout, + line: "tower: dependency install failed; retrying with setuptools<82 pin for pkg_resources compatibility".to_string(), + time: chrono::Utc::now(), + }); + + match 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; + } + } + } if res != 0 { // If the sync process failed, we want to return an error. diff --git a/crates/tower-uv/src/lib.rs b/crates/tower-uv/src/lib.rs index 1217a496..a2e44bda 100644 --- a/crates/tower-uv/src/lib.rs +++ b/crates/tower-uv/src/lib.rs @@ -357,81 +357,85 @@ impl Uv { &self.uv_path, cwd ); - let req_path = cwd.join("requirements.txt"); - - // setuptools 82 removed pkg_resources, but many legacy packages still - // import it without declaring the dependency. Historically we always - // injected `setuptools<82` to keep pkg_resources available, but that - // pin makes resolution fail for apps using newer deps that require - // setuptools>=82. - // - // https://github.com/pypa/setuptools/issues/5174 - let unpinned_resolves = { - let mut probe = Command::new(&self.uv_path); - probe - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .current_dir(cwd) - .arg("--color") - .arg("never") - .arg("pip") - .arg("install") - .arg("--dry-run") - .arg("-r") - .arg(&req_path) - .envs(env_vars); - - if let Some(dir) = &self.cache_dir { - probe.arg("--cache-dir").arg(dir); - } - - matches!(probe.status().await, Ok(s) if s.success()) - }; - - if !unpinned_resolves { - debug!( - "Falling back to setuptools<82 pin for {:?}: unpinned resolution failed", - cwd - ); - } + self.spawn_requirements_install(cwd, env_vars, false).await + } else { + // If there is no pyproject.toml or requirements.txt, then we can't sync. + Err(Error::MissingPyprojectToml) + } + } - // If there is a requirements.txt, then we can use that to sync. - 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); + /// 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() + } - if !unpinned_resolves { - cmd.arg("setuptools<82"); - } + /// Re-runs the `requirements.txt` 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`. + /// + /// https://github.com/pypa/setuptools/issues/5174 + pub async fn sync_with_legacy_setuptools_pin( + &self, + cwd: &PathBuf, + env_vars: &HashMap, + ) -> Result { + if !cwd.join("requirements.txt").exists() { + return Err(Error::MissingPyprojectToml); + } - cmd.envs(env_vars); + debug!( + "Retrying UV ({:?}) sync with setuptools<82 pin in {:?}", + &self.uv_path, cwd + ); - #[cfg(unix)] - { - cmd.process_group(0); - } + self.spawn_requirements_install(cwd, env_vars, true).await + } - if let Some(dir) = &self.cache_dir { - cmd.arg("--cache-dir").arg(dir); - } + async fn spawn_requirements_install( + &self, + cwd: &PathBuf, + env_vars: &HashMap, + pin_legacy_setuptools: bool, + ) -> Result { + let req_path = cwd.join("requirements.txt"); - let child = cmd.spawn()?; + 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); - Ok(child) - } else { - // If there is no pyproject.toml or requirements.txt, then we can't sync. - Err(Error::MissingPyprojectToml) + if pin_legacy_setuptools { + cmd.arg("setuptools<82"); } + + cmd.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()?) } pub async fn run( diff --git a/crates/tower-uv/tests/sync_test.rs b/crates/tower-uv/tests/sync_test.rs index 02537889..b2cc8387 100644 --- a/crates/tower-uv/tests/sync_test.rs +++ b/crates/tower-uv/tests/sync_test.rs @@ -1,12 +1,13 @@ //! Integration tests for `Uv::sync` requirements.txt handling. //! -//! These tests focus on the `setuptools<82` injection logic. The default path -//! resolves *without* the pin (most apps work fine with a modern setuptools); -//! the pin is only applied as a fallback when unpinned resolution fails (e.g. -//! `dlt[motherduck,hub]==1.26.0a1`-style transitive constraints). +//! `sync()` runs a plain `uv pip install -r requirements.txt` (no setuptools +//! pin). Callers that hit a resolution failure can retry via +//! `sync_with_legacy_setuptools_pin()` for the legacy `pkg_resources` +//! compatibility case. The retry orchestration lives in +//! `tower-runtime::local`. //! -//! The tests shell out to a real `uv` binary and hit pypi over the network, -//! mirroring the existing `install_test.rs`. +//! These tests shell out to a real `uv` binary and hit pypi, mirroring the +//! existing `install_test.rs`. use std::collections::HashMap; use std::path::PathBuf; @@ -30,12 +31,10 @@ async fn make_uv_with_venv(cwd: &PathBuf) -> Uv { } #[tokio::test] -async fn sync_succeeds_without_pin_for_simple_requirements() { +async fn sync_succeeds_for_simple_requirements() { let tmp = TempDir::new().expect("tempdir"); let cwd = tmp.path().to_path_buf(); - // Trivial requirements.txt — unpinned resolution succeeds, so we install - // without the setuptools<82 pin. tokio::fs::write(cwd.join("requirements.txt"), "six\n") .await .expect("write requirements.txt"); @@ -44,7 +43,7 @@ async fn sync_succeeds_without_pin_for_simple_requirements() { let env_vars: HashMap = HashMap::new(); let child = uv.sync(&cwd, &env_vars).await.expect("sync spawn failed"); let code = wait(child).await; - assert_eq!(code, 0, "sync should succeed for simple requirements.txt"); + assert_eq!(code, 0, "sync should succeed for a simple requirements.txt"); } #[tokio::test] @@ -52,8 +51,9 @@ async fn sync_succeeds_when_user_requires_modern_setuptools() { let tmp = TempDir::new().expect("tempdir"); let cwd = tmp.path().to_path_buf(); - // Regression test for the dlt scenario: an app that explicitly wants - // setuptools>=82 should resolve cleanly (no pin gets injected). + // Regression case: an app that requires setuptools>=82 used to fail + // because tower-uv unconditionally injected `setuptools<82`. Now the + // default sync path applies no pin, so resolution should succeed. tokio::fs::write(cwd.join("requirements.txt"), "setuptools>=82\n") .await .expect("write requirements.txt"); @@ -69,19 +69,62 @@ async fn sync_succeeds_when_user_requires_modern_setuptools() { } #[tokio::test] -async fn sync_succeeds_when_user_pins_legacy_setuptools() { +async fn sync_with_legacy_setuptools_pin_installs_legacy_setuptools() { let tmp = TempDir::new().expect("tempdir"); let cwd = tmp.path().to_path_buf(); - // Apps that need pkg_resources can pin setuptools<82 themselves; our logic - // shouldn't get in the way. - tokio::fs::write(cwd.join("requirements.txt"), "setuptools<82\n") + tokio::fs::write(cwd.join("requirements.txt"), "six\n") .await .expect("write requirements.txt"); let uv = make_uv_with_venv(&cwd).await; let env_vars: HashMap = HashMap::new(); - let child = uv.sync(&cwd, &env_vars).await.expect("sync spawn failed"); + 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 should succeed when the user pins setuptools<82"); + assert_eq!( + code, 0, + "sync_with_legacy_setuptools_pin should succeed when the pin is compatible" + ); +} + +#[tokio::test] +async fn sync_with_legacy_setuptools_pin_fails_when_user_requires_modern_setuptools() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().to_path_buf(); + + // The fallback method intentionally pins `setuptools<82`. When the user's + // requirements demand setuptools>=82, the resolver must report a conflict + // — this confirms the pin is actually being applied. + tokio::fs::write(cwd.join("requirements.txt"), "setuptools>=82\n") + .await + .expect("write requirements.txt"); + + 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 pin conflicts with user's requirements" + ); +} + +#[tokio::test] +async fn sync_with_legacy_setuptools_pin_errors_without_requirements_txt() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().to_path_buf(); + + let uv = Uv::new(None, false).await.expect("Uv::new failed"); + let env_vars: HashMap = HashMap::new(); + 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" + ); }