From 71dd9d120b9d19020cc58c32e7fa1be2e5607852 Mon Sep 17 00:00:00 2001 From: Piettro <2058578+pcastro93@users.noreply.github.com> Date: Tue, 3 Nov 2026 09:58:36 -0500 Subject: [PATCH] fix(python): keep modules in the working directory from shadowing the stdlib (#964) Python puts the working directory first on sys.path for -c / -m. venv creation, pip (list/install/uninstall), the library-path probe and the IDE-managed Python tool probes and installs inherited the IDE's own working directory, so a project's typing.py / logging.py there broke them ("partially initialized module 'typing'"). venv creation now runs python -I; pip, the library probe and the tool commands run in an empty private directory (keeping pip install --user packages visible), and relative requirements (., ./pkg, .[dev]) resolve against the project root. The window-label part of the original change already landed in #960. Refs #955 Co-authored-by: pcastro93 <2058578+pcastro93@users.noreply.github.com> --- crates/ide-host/src/ops/sdk.rs | 3 +- crates/ide-sdk/Cargo.toml | 4 +-- crates/ide-sdk/src/libraries.rs | 6 ++-- crates/ide-sdk/src/python.rs | 52 +++++++++++++++++++++++++++------ crates/ide-sdk/src/tests.rs | 43 +++++++++++++++++++++++++++ crates/ide-tools/src/lib.rs | 27 ++++++++++++----- crates/ide-tools/src/tests.rs | 14 +++++++++ 7 files changed, 127 insertions(+), 22 deletions(-) diff --git a/crates/ide-host/src/ops/sdk.rs b/crates/ide-host/src/ops/sdk.rs index 3e06c85c..2db3148d 100644 --- a/crates/ide-host/src/ops/sdk.rs +++ b/crates/ide-host/src/ops/sdk.rs @@ -122,6 +122,7 @@ pub async fn sdk_python_pip( on_output: Channel, ) -> IdeResult<()> { host.require_trust(window)?; + let project_root = host.project(window).ok().map(|p| p.root.clone()); let cancel = host.begin_project_request(window, request); let result = tokio::task::spawn_blocking(move || { let send = |line: String| { @@ -130,7 +131,7 @@ pub async fn sdk_python_pip( if uninstall { ide_sdk::python::uninstall_package(&python, &requirement, &cancel, &send) } else { - ide_sdk::python::install_package(&python, &requirement, upgrade, &cancel, &send) + ide_sdk::python::install_package(&python, &requirement, upgrade, project_root.as_deref(), &cancel, &send) } }) .await diff --git a/crates/ide-sdk/Cargo.toml b/crates/ide-sdk/Cargo.toml index 8d9a24e7..374bfee6 100644 --- a/crates/ide-sdk/Cargo.toml +++ b/crates/ide-sdk/Cargo.toml @@ -15,9 +15,7 @@ ide-vfs.workspace = true serde.workspace = true serde_json.workspace = true specta.workspace = true +tempfile.workspace = true toml.workspace = true tracing.workspace = true zip.workspace = true - -[dev-dependencies] -tempfile.workspace = true diff --git a/crates/ide-sdk/src/libraries.rs b/crates/ide-sdk/src/libraries.rs index 827e42ec..0871150d 100644 --- a/crates/ide-sdk/src/libraries.rs +++ b/crates/ide-sdk/src/libraries.rs @@ -11,7 +11,7 @@ //! JDK's `src.zip` and the Maven / Gradle dependency jars (source attachments resolved by JDT). use crate::exe; -use crate::exec::{query, query_in, which}; +use crate::exec::{query_in, which}; use ide_core::settings::Settings; use serde::{Deserialize, Serialize}; use specta::Type; @@ -157,7 +157,9 @@ fn python_groups(root: &Path, s: &Settings) -> Vec { /// Standard library and site-packages folders of a Python interpreter. pub fn python_libraries(python: &Path, root: Option<&Path>) -> Option { - let out = query(python, &["-c", PYTHON_PATHS], QUERY_TIMEOUT)?; + // In a neutral directory: the working directory comes first on `sys.path` for `-c`. + let dir = crate::python::neutral_dir(); + let out = query_in(python, &["-c", PYTHON_PATHS], dir.as_ref().map(|d| d.path()), &[], QUERY_TIMEOUT)?; let name = crate::discover::python_sdk(python, "system", root).map(|s| s.name).unwrap_or_else(|| "Python".into()); group(name, "python", library_roots(parse_python_paths(&out))) } diff --git a/crates/ide-sdk/src/python.rs b/crates/ide-sdk/src/python.rs index f57f4e72..dcaecebe 100644 --- a/crates/ide-sdk/src/python.rs +++ b/crates/ide-sdk/src/python.rs @@ -1,7 +1,7 @@ //! Python environments: creating virtualenvs and managing packages with pip //! (or `uv pip` for environments created by uv, which ship without pip). -use crate::exec::{query, stream, which}; +use crate::exec::{query_in, stream, which}; use crate::{Sdk, bin_dir, exe, venv_of}; use ide_core::{CancellationToken, IdeError, IdeResult}; use serde::{Deserialize, Serialize}; @@ -32,13 +32,29 @@ pub fn create_venv( if let Some(parent) = location.parent() { std::fs::create_dir_all(parent)?; } - let args = vec!["-m".to_owned(), "venv".to_owned(), location.to_string_lossy().into_owned()]; - stream(base, &args, None, &[], cancel, on_line)?; + stream(base, &venv_args(location), None, &[], cancel, on_line)?; let python = location.join(bin_dir()).join(exe("python")); crate::discover::python_sdk(&python, "venv", location.parent().and_then(Path::parent)) .ok_or_else(|| IdeError::internal(format!("{} was not created", python.display()))) } +/// Arguments of `base -m venv location`. `-I` (isolated mode) keeps the working +/// directory off `sys.path`: a project's `logging.py` or `typing.py` there would +/// otherwise shadow the standard library modules venv imports. (`-P` does just +/// that but needs Python 3.11.) +pub(crate) fn venv_args(location: &Path) -> Vec { + vec!["-I".to_owned(), "-m".to_owned(), "venv".to_owned(), location.to_string_lossy().into_owned()] +} + +/// A fresh, empty, private directory to run `python -m …` / `python -c …` in. Python +/// puts the working directory first on `sys.path` for those, and the IDE's own (or a +/// project's) could hold modules that shadow the standard library or pip. Not `-I`: +/// that also drops the user site, hiding `pip install --user` packages. `None` (the +/// inherited directory) if it cannot be created. +pub(crate) fn neutral_dir() -> Option { + tempfile::Builder::new().prefix("openidea-python-").tempdir().ok() +} + /// How packages of an interpreter are managed. enum Installer { Pip, @@ -47,7 +63,9 @@ enum Installer { } fn has_pip(python: &Path) -> bool { - query(python, &["-m", "pip", "--version"], Duration::from_secs(10)).is_some() + let dir = neutral_dir(); + query_in(python, &["-m", "pip", "--version"], dir.as_ref().map(|d| d.path()), &[], Duration::from_secs(10)) + .is_some() } fn installer(python: &Path) -> IdeResult { @@ -95,7 +113,8 @@ pub fn list_packages(python: &Path, cancel: &CancellationToken) -> IdeResult IdeResult> { serde_json::from_str(json).map_err(|e| IdeError::internal(format!("unexpected pip output: {e}"))) } -/// Installs (or upgrades, with `upgrade`) a requirement such as `requests` or `django==5.0`. +/// A requirement naming a local path (`.`, `./pkg`, `../lib[dev]`) made absolute +/// against `base` (the project root): pip runs in [`neutral_dir`], not the project. +pub(crate) fn resolve_requirement(requirement: &str, base: Option<&Path>) -> IdeResult { + if !requirement.starts_with('.') { + return Ok(requirement.to_owned()); + } + let base = base.ok_or_else(|| IdeError::invalid("a relative path needs an open project"))?; + Ok(base.join(requirement).to_string_lossy().into_owned()) +} + +/// Installs (or upgrades, with `upgrade`) a requirement such as `requests` or `django==5.0`; +/// relative paths (`./pkg`) are resolved against `project_root`. pub fn install_package( python: &Path, requirement: &str, upgrade: bool, + project_root: Option<&Path>, cancel: &CancellationToken, on_line: &dyn Fn(String), ) -> IdeResult<()> { @@ -119,14 +150,16 @@ pub fn install_package( if requirement.is_empty() || requirement.starts_with('-') { return Err(IdeError::invalid("enter a package name or requirement")); } + let requirement = resolve_requirement(requirement, project_root)?; let installer = installer(python)?; let mut sub = vec!["install"]; if upgrade { sub.push("--upgrade"); } - sub.push(requirement); + sub.push(&requirement); let (program, args) = pip_command(python, &installer, &sub); - stream(&program, &args, None, &pip_env(python), cancel, on_line).map(|_| ()) + let dir = neutral_dir(); + stream(&program, &args, dir.as_ref().map(|d| d.path()), &pip_env(python), cancel, on_line).map(|_| ()) } pub fn uninstall_package( @@ -141,5 +174,6 @@ pub fn uninstall_package( } let installer = installer(python)?; let (program, args) = pip_command(python, &installer, &["uninstall", "-y", name]); - stream(&program, &args, None, &pip_env(python), cancel, on_line).map(|_| ()) + let dir = neutral_dir(); + stream(&program, &args, dir.as_ref().map(|d| d.path()), &pip_env(python), cancel, on_line).map(|_| ()) } diff --git a/crates/ide-sdk/src/tests.rs b/crates/ide-sdk/src/tests.rs index 2385a404..75d5011c 100644 --- a/crates/ide-sdk/src/tests.rs +++ b/crates/ide-sdk/src/tests.rs @@ -411,6 +411,49 @@ fn pip_list_output_is_parsed() { assert_eq!(pkgs[0].version, "2.32.0"); } +/// A directory whose `typing.py` breaks any interpreter that imports it from there. +fn shadowing_dir() -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("typing.py"), "raise ImportError('shadowed stdlib')\n").unwrap(); + std::fs::write(dir.path().join("logging.py"), "raise ImportError('shadowed stdlib')\n").unwrap(); + dir +} + +#[test] +fn venv_creation_ignores_modules_in_the_working_directory() { + let Some(python) = crate::exec::which("python3") else { return }; + let shadow = shadowing_dir(); + // Precondition: without isolation the shadowing module is picked up. + let plain = + std::process::Command::new(&python).args(["-c", "import typing"]).current_dir(shadow.path()).output().unwrap(); + assert!(!plain.status.success()); + let target = tempfile::tempdir().unwrap(); + let location = target.path().join(".venv"); + // `--without-pip` keeps the test fast; the stdlib imports of `venv` still happen. + let mut args = crate::python::venv_args(&location); + args.push("--without-pip".into()); + crate::exec::stream(&python, &args, Some(shadow.path()), &[], &Default::default(), &|_| {}).unwrap(); + assert!(location.join("pyvenv.cfg").is_file()); +} + +#[test] +fn neutral_dir_is_fresh_and_empty() { + let a = crate::python::neutral_dir().unwrap(); + let b = crate::python::neutral_dir().unwrap(); + assert_ne!(a.path(), b.path()); + assert_eq!(std::fs::read_dir(a.path()).unwrap().count(), 0); +} + +#[test] +fn relative_requirements_resolve_against_the_project() { + use crate::python::resolve_requirement; + let root = Path::new("/work/app"); + assert_eq!(resolve_requirement("requests==2.0", Some(root)).unwrap(), "requests==2.0"); + assert_eq!(resolve_requirement("./pkg", Some(root)).unwrap(), root.join("./pkg").to_string_lossy()); + assert_eq!(resolve_requirement(".[dev]", Some(root)).unwrap(), root.join(".[dev]").to_string_lossy()); + assert!(resolve_requirement("../lib", None).is_err()); +} + #[test] fn excluded_folders_become_anchored_globs() { let mut s = Settings::default(); diff --git a/crates/ide-tools/src/lib.rs b/crates/ide-tools/src/lib.rs index af2daf93..749b18f3 100644 --- a/crates/ide-tools/src/lib.rs +++ b/crates/ide-tools/src/lib.rs @@ -472,6 +472,19 @@ impl Tools { cmd } + /// `python` started in an empty private directory: Python puts the working directory + /// first on `sys.path` for `-c` / `-m`, and the IDE's own (inherited from wherever it + /// was launched, e.g. a project) could hold modules that shadow the standard library, + /// pip or the tool being probed. Not `-I`: that also hides `pip install --user` packages. + fn python_command(&self, python: impl AsRef) -> Command { + let mut cmd = self.command(python); + let dir = self.root.join(".python-cwd"); + if std::fs::create_dir_all(&dir).is_ok() { + cmd.current_dir(dir); + } + cmd + } + fn python(&self) -> Option { self.which_any(&["python3", "python"]) } @@ -708,7 +721,7 @@ impl Tools { } // Importable by the default interpreter (pip install --user / a system package). let Some(python) = self.python() else { return (ToolStatus::Missing, None, None) }; - let mut c = self.command(&python); + let mut c = self.python_command(&python); c.args(["-c", &format!("import {module}; print({module}.__version__)")]); match run_quiet(c, Duration::from_secs(5)) { Some((true, out)) => (ToolStatus::OnPath, Some(python), parse_version(&out)), @@ -717,7 +730,7 @@ impl Tools { } Method::PipUser { module, .. } => { let Some(python) = self.python() else { return (ToolStatus::Missing, None, None) }; - let mut c = self.command(&python); + let mut c = self.python_command(&python); c.args(["-c", &format!("import {module}; print({module}.__version__)")]); match run_quiet(c, Duration::from_secs(5)) { Some((true, out)) => (ToolStatus::Installed, Some(python), parse_version(&out)), @@ -1010,10 +1023,10 @@ impl Tools { c.args(["pip", "install", "--python"]).arg(&python).arg(package); run_streamed(c, cancel, emit)?; } else { - let mut c = self.command(self.python().expect("checked")); + let mut c = self.python_command(self.python().expect("checked")); c.args(["-m", "venv"]).arg(&venv); run_streamed(c, cancel, emit)?; - let mut c = self.command(&python); + let mut c = self.python_command(&python); c.args(["-m", "pip", "install", "--upgrade", "--disable-pip-version-check", package]); run_streamed(c, cancel, emit)?; } @@ -1032,7 +1045,7 @@ impl Tools { fn install_pip_user(&self, package: &str, cancel: &CancellationToken, emit: Emit) -> IdeResult<()> { let python = self.python().expect("checked"); let pip = |extra: &[&str]| { - let mut c = self.command(&python); + let mut c = self.python_command(&python); c.args(["-m", "pip", "install", "--user", "--upgrade", "--disable-pip-version-check"]) .args(extra) .arg(package); @@ -1069,7 +1082,7 @@ impl Tools { let _ = std::fs::remove_dir_all(&dir); let lib = python_lib_dir(&self.root, spec.name); std::fs::create_dir_all(&lib)?; - let mut c = self.command(self.python().expect("checked")); + let mut c = self.python_command(self.python().expect("checked")); c.args(["-m", "pip", "install", "--disable-pip-version-check", "--no-deps", "--upgrade", "--target"]) .arg(&lib) .args(["--only-binary=:all:", "--platform", "any", "--implementation", "py"]) @@ -1229,7 +1242,7 @@ impl Tools { Method::PipUser { package, .. } => { let python = self.python().ok_or_else(|| IdeError::unavailable("python3 not found"))?; let pip = |extra: &[&str]| { - let mut c = self.command(&python); + let mut c = self.python_command(&python); c.args(["-m", "pip", "uninstall", "-y", "--disable-pip-version-check"]).args(extra).arg(package); run_collect(c, &cancel, &quiet) }; diff --git a/crates/ide-tools/src/tests.rs b/crates/ide-tools/src/tests.rs index 96617bf2..be6878be 100644 --- a/crates/ide-tools/src/tests.rs +++ b/crates/ide-tools/src/tests.rs @@ -699,3 +699,17 @@ fn rust_analyzer_managed_copy_survives_rustup_becoming_available() { env.tools().uninstall("rust-analyzer", &langs()).unwrap(); assert!(!env.root.join("rust-analyzer").exists()); } + +#[cfg(unix)] +#[test] +fn python_probes_run_in_an_empty_private_directory() { + let env = Env::new(); + // A stand-in interpreter that reports where it was started. + env.script(&env.sys, "python3", "pwd; ls -A"); + let tools = env.tools(); + let python = tools.python().unwrap(); + let out = tools.python_command(&python).output().unwrap(); + let out = String::from_utf8(out.stdout).unwrap(); + let dir = env.root.join(".python-cwd"); + assert_eq!(out.trim(), dir.canonicalize().unwrap().to_string_lossy()); +}