mirror of
https://github.com/l0ng-ai/openidea.git
synced 2026-11-06 08:07:49 +00:00
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>
This commit is contained in:
@@ -122,6 +122,7 @@ pub async fn sdk_python_pip(
|
||||
on_output: Channel<String>,
|
||||
) -> 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<LibraryGroup> {
|
||||
|
||||
/// Standard library and site-packages folders of a Python interpreter.
|
||||
pub fn python_libraries(python: &Path, root: Option<&Path>) -> Option<LibraryGroup> {
|
||||
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)))
|
||||
}
|
||||
|
||||
@@ -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<String> {
|
||||
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::TempDir> {
|
||||
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<Installer> {
|
||||
@@ -95,7 +113,8 @@ pub fn list_packages(python: &Path, cancel: &CancellationToken) -> IdeResult<Vec
|
||||
}
|
||||
let installer = installer(python)?;
|
||||
let (program, args) = pip_command(python, &installer, &["list", "--format", "json"]);
|
||||
let out = stream(&program, &args, None, &pip_env(python), cancel, &|_| {})?;
|
||||
let dir = neutral_dir();
|
||||
let out = stream(&program, &args, dir.as_ref().map(|d| d.path()), &pip_env(python), cancel, &|_| {})?;
|
||||
let mut packages = parse_pip_list(&out)?;
|
||||
packages.sort_by_key(|p| p.name.to_lowercase());
|
||||
Ok(packages)
|
||||
@@ -107,11 +126,23 @@ pub(crate) fn parse_pip_list(out: &str) -> IdeResult<Vec<PythonPackage>> {
|
||||
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<String> {
|
||||
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(|_| ())
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<std::ffi::OsStr>) -> 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<PathBuf> {
|
||||
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)
|
||||
};
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user