-
Notifications
You must be signed in to change notification settings - Fork 3
v0.3.61 release #271
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
v0.3.61 release #271
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -357,44 +357,85 @@ impl Uv { | |||||||||||||||||||||||||||||
| &self.uv_path, 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) | ||||||||||||||||||||||||||||||
| .stdin(Stdio::null()) | ||||||||||||||||||||||||||||||
| .stdout(Stdio::piped()) | ||||||||||||||||||||||||||||||
| .stderr(Stdio::piped()) | ||||||||||||||||||||||||||||||
| .current_dir(cwd) | ||||||||||||||||||||||||||||||
| .arg("--color") | ||||||||||||||||||||||||||||||
| .arg("never") | ||||||||||||||||||||||||||||||
| .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); | ||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| #[cfg(unix)] | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| cmd.process_group(0); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| /// 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() | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+367
to
+373
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Match the retry gate to
Suggested fix pub fn should_use_legacy_setuptools_pin(&self, cwd: &Path) -> bool {
- cwd.join("requirements.txt").exists()
+ cwd.join("requirements.txt").exists() && !cwd.join("pyproject.toml").exists()
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if let Some(dir) = &self.cache_dir { | ||||||||||||||||||||||||||||||
| cmd.arg("--cache-dir").arg(dir); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| /// 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<String, String>, | ||||||||||||||||||||||||||||||
| ) -> Result<Child, Error> { | ||||||||||||||||||||||||||||||
| if !cwd.join("requirements.txt").exists() { | ||||||||||||||||||||||||||||||
| return Err(Error::MissingPyprojectToml); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| let child = cmd.spawn()?; | ||||||||||||||||||||||||||||||
| debug!( | ||||||||||||||||||||||||||||||
| "Retrying UV ({:?}) sync with setuptools<82 pin in {:?}", | ||||||||||||||||||||||||||||||
| &self.uv_path, cwd | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Ok(child) | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| // If there is no pyproject.toml or requirements.txt, then we can't sync. | ||||||||||||||||||||||||||||||
| Err(Error::MissingPyprojectToml) | ||||||||||||||||||||||||||||||
| self.spawn_requirements_install(cwd, env_vars, true).await | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| async fn spawn_requirements_install( | ||||||||||||||||||||||||||||||
| &self, | ||||||||||||||||||||||||||||||
| cwd: &PathBuf, | ||||||||||||||||||||||||||||||
| env_vars: &HashMap<String, String>, | ||||||||||||||||||||||||||||||
| pin_legacy_setuptools: bool, | ||||||||||||||||||||||||||||||
| ) -> Result<Child, Error> { | ||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| 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( | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
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.
Skip the fallback when cancellation already happened.
If the first
syncexits with-1becausecancel_tokenfired, Line 285 still retries and spawns a second install. That makesterminate()restart work instead of stopping it.Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents