From c9ffe976f9716173f722de03fc38761f30833cf7 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 1 Mar 2024 10:52:48 -0500 Subject: [PATCH] Centralize virtualenv path construction (#2102) ## Summary Right now, we have virtualenv construction encoded in a few different places. Namely, it happens in both `gourgeist` and `virtualenv_layout.rs` -- _and_ `interpreter.rs` also encodes some knowledge about how they work, by way of reconstructing the `SysconfigPaths`. Instead, `gourgeist` now returns the complete layout, enumerating all the directories it created. So, rather than returning a root directory, and re-creating all those paths in `uv-interpreter`, we pass the data directly back to it. --- crates/gourgeist/src/bare.rs | 108 ++++++++++-------- crates/gourgeist/src/lib.rs | 12 +- crates/uv-interpreter/src/interpreter.rs | 69 +++-------- crates/uv-interpreter/src/lib.rs | 9 +- .../uv-interpreter/src/python_environment.rs | 50 +++++--- crates/uv-interpreter/src/sysconfig.rs | 18 +++ crates/uv-interpreter/src/virtualenv.rs | 17 +++ .../uv-interpreter/src/virtualenv_layout.rs | 83 -------------- 8 files changed, 159 insertions(+), 207 deletions(-) create mode 100644 crates/uv-interpreter/src/sysconfig.rs create mode 100644 crates/uv-interpreter/src/virtualenv.rs delete mode 100644 crates/uv-interpreter/src/virtualenv_layout.rs diff --git a/crates/gourgeist/src/bare.rs b/crates/gourgeist/src/bare.rs index 1e9b4ad2e..2617bb4d4 100644 --- a/crates/gourgeist/src/bare.rs +++ b/crates/gourgeist/src/bare.rs @@ -11,7 +11,7 @@ use fs_err::File; use tracing::info; use uv_fs::Simplified; -use uv_interpreter::Interpreter; +use uv_interpreter::{Interpreter, SysconfigPaths, Virtualenv}; use crate::{Error, Prompt}; @@ -40,27 +40,6 @@ fn write_cfg(f: &mut impl Write, data: &[(String, String)]) -> io::Result<()> { Ok(()) } -/// Absolute paths of the virtualenv -#[derive(Debug)] -pub struct VenvPaths { - /// The location of the virtualenv, e.g. `.venv` - #[allow(unused)] - pub root: Utf8PathBuf, - - /// The python interpreter.rs inside the virtualenv, on unix `.venv/bin/python` - #[allow(unused)] - pub interpreter: Utf8PathBuf, - - /// The directory with the scripts, on unix `.venv/bin` - #[allow(unused)] - pub bin: Utf8PathBuf, - - /// The site-packages directory where all the packages are installed to, on unix - /// and python 3.11 `.venv/lib/python3.11/site-packages` - #[allow(unused)] - pub site_packages: Utf8PathBuf, -} - /// Write all the files that belong to a venv without any packages installed. pub fn create_bare_venv( location: &Utf8Path, @@ -68,7 +47,7 @@ pub fn create_bare_venv( prompt: Prompt, system_site_packages: bool, extra_cfg: Vec<(String, String)>, -) -> Result { +) -> Result { // We have to canonicalize the interpreter path, otherwise the home is set to the venv dir instead of the real root. // This would make python-build-standalone fail with the encodings module not being found because its home is wrong. let base_python: Utf8PathBuf = fs_err::canonicalize(interpreter.sys_executable())? @@ -107,8 +86,8 @@ pub fn create_bare_venv( Err(err) => return Err(Error::IO(err)), } - // TODO(konstin): I bet on windows we'll have to strip the prefix again let location = location.canonicalize_utf8()?; + let bin_name = if cfg!(unix) { "bin" } else if cfg!(windows) { @@ -116,7 +95,7 @@ pub fn create_bare_venv( } else { unimplemented!("Only Windows and Unix are supported") }; - let bin_dir = location.join(bin_name); + let scripts = location.join(bin_name); let prompt = match prompt { Prompt::CurrentDirectoryName => env::current_dir()? .file_name() @@ -132,27 +111,29 @@ pub fn create_bare_venv( fs::write(location.join(".gitignore"), "*")?; // Different names for the python interpreter - fs::create_dir(&bin_dir)?; - let venv_python = bin_dir.join(format!("python{EXE_SUFFIX}")); + fs::create_dir(&scripts)?; + let executable = scripts.join(format!("python{EXE_SUFFIX}")); + // No symlinking on Windows, at least not on a regular non-dev non-admin Windows install. #[cfg(unix)] { use fs_err::os::unix::fs::symlink; - symlink(&base_python, &venv_python)?; + symlink(&base_python, &executable)?; symlink( "python", - bin_dir.join(format!("python{}", interpreter.python_major())), + scripts.join(format!("python{}", interpreter.python_major())), )?; symlink( "python", - bin_dir.join(format!( + scripts.join(format!( "python{}.{}", interpreter.python_major(), interpreter.python_minor(), )), )?; } + #[cfg(windows)] { // https://github.com/python/cpython/blob/d457345bbc6414db0443819290b04a9a4333313d/Lib/venv/__init__.py#L261-L267 @@ -166,7 +147,7 @@ pub fn create_bare_venv( .join("scripts") .join("nt") .join(python_exe); - fs_err::copy(shim, bin_dir.join(python_exe))?; + fs_err::copy(shim, scripts.join(python_exe))?; } } #[cfg(not(any(unix, windows)))] @@ -175,9 +156,19 @@ pub fn create_bare_venv( } // Add all the activate scripts for different shells - // TODO(konstin): RELATIVE_SITE_PACKAGES is currently only the unix path. We should ensure that all launchers work - // cross-platform. for (name, template) in ACTIVATE_TEMPLATES { + let relative_site_packages = if cfg!(unix) { + format!( + "../lib/{}{}.{}/site-packages", + interpreter.site_packages_python(), + interpreter.python_major(), + interpreter.python_minor(), + ) + } else if cfg!(windows) { + "../Lib/site-packages".to_string() + } else { + unimplemented!("Only Windows and Unix are supported") + }; let activator = template .replace( "{{ VIRTUAL_ENV_DIR }}", @@ -191,14 +182,9 @@ pub fn create_bare_venv( ) .replace( "{{ RELATIVE_SITE_PACKAGES }}", - &format!( - "../lib/{}{}.{}/site-packages", - interpreter.site_packages_python(), - interpreter.python_major(), - interpreter.python_minor(), - ), + relative_site_packages.as_str(), ); - fs::write(bin_dir.join(name), activator)?; + fs::write(scripts.join(name), activator)?; } // pyvenv.cfg @@ -280,6 +266,7 @@ pub fn create_bare_venv( write_cfg(&mut pyvenv_cfg, &pyvenv_cfg_data)?; drop(pyvenv_cfg); + // Construct the path to the `site-packages` directory. let site_packages = if cfg!(unix) { location .join("lib") @@ -295,17 +282,42 @@ pub fn create_bare_venv( } else { unimplemented!("Only Windows and Unix are supported") }; + + // Construct the path to the `platstdlib` directory. + let platstdlib = if cfg!(windows) { + location.join("Lib") + } else { + location + .join("lib") + .join(format!( + "{}{}.{}", + interpreter.site_packages_python(), + interpreter.python_major(), + interpreter.python_minor() + )) + .join("site-packages") + }; + + // Populate `site-packages` with a `_virtualenv.py` file. fs::create_dir_all(&site_packages)?; - // Install _virtualenv.py patch. - // Frankly no idea what that does, i just copied it from virtualenv knowing that - // distutils/setuptools will have their cursed reasons fs::write(site_packages.join("_virtualenv.py"), VIRTUALENV_PATCH)?; fs::write(site_packages.join("_virtualenv.pth"), "import _virtualenv")?; - Ok(VenvPaths { - root: location.to_path_buf(), - interpreter: venv_python, - bin: bin_dir, - site_packages, + Ok(Virtualenv { + root: location.clone().into_std_path_buf(), + executable: executable.into_std_path_buf(), + sysconfig_paths: SysconfigPaths { + // Paths that were already constructed above. + scripts: scripts.into_std_path_buf(), + platstdlib: platstdlib.into_std_path_buf(), + // Set `purelib` and `platlib` to the same value. + purelib: site_packages.clone().into_std_path_buf(), + platlib: site_packages.into_std_path_buf(), + // Inherited from the interpreter. + stdlib: interpreter.stdlib().to_path_buf(), + include: interpreter.include().to_path_buf(), + platinclude: interpreter.platinclude().to_path_buf(), + data: location.into_std_path_buf(), + }, }) } diff --git a/crates/gourgeist/src/lib.rs b/crates/gourgeist/src/lib.rs index 0e4523579..b6c1d42c7 100644 --- a/crates/gourgeist/src/lib.rs +++ b/crates/gourgeist/src/lib.rs @@ -54,18 +54,20 @@ pub fn create_venv( system_site_packages: bool, extra_cfg: Vec<(String, String)>, ) -> Result { + // Create the virtualenv at the given location. let location: &Utf8Path = location .try_into() .map_err(|err: FromPathError| err.into_io_error())?; - let paths = create_bare_venv( + let virtualenv = create_bare_venv( location, &interpreter, prompt, system_site_packages, extra_cfg, )?; - Ok(PythonEnvironment::from_interpreter( - interpreter, - paths.root.as_std_path(), - )) + + // Create the corresponding `PythonEnvironment`. + let interpreter = interpreter.with_virtualenv(virtualenv); + let root = interpreter.prefix().to_path_buf(); + Ok(PythonEnvironment::from_interpreter(interpreter, root)) } diff --git a/crates/uv-interpreter/src/interpreter.rs b/crates/uv-interpreter/src/interpreter.rs index 06b49e5d6..961978b08 100644 --- a/crates/uv-interpreter/src/interpreter.rs +++ b/crates/uv-interpreter/src/interpreter.rs @@ -18,10 +18,10 @@ use platform_tags::{Tags, TagsError}; use uv_cache::{Cache, CacheBucket, CachedByTimestamp, Freshness, Timestamp}; use uv_fs::write_atomic_sync; -use crate::python_environment::detect_virtual_env; +use crate::python_environment::{detect_python_executable, detect_virtual_env}; use crate::python_query::try_find_default_python; -use crate::virtualenv_layout::VirtualenvLayout; -use crate::{find_requested_python, Error, PythonVersion}; +use crate::sysconfig::SysconfigPaths; +use crate::{find_requested_python, Error, PythonVersion, Virtualenv}; /// A Python executable and its associated platform markers. #[derive(Debug, Clone)] @@ -84,36 +84,11 @@ impl Interpreter { /// Return a new [`Interpreter`] with the given virtual environment root. #[must_use] - pub(crate) fn with_venv_root(self, venv_root: PathBuf) -> Self { - let layout = VirtualenvLayout::from_platform(&self.platform); + pub fn with_virtualenv(self, virtualenv: Virtualenv) -> Self { Self { - // Given that we know `venv_root` is a virtualenv, and not an arbitrary Python - // interpreter, we can safely assume that the platform is the same as the host - // platform. Further, we can safely assume that the paths follow a predictable - // structure, which allows us to avoid querying the interpreter for the `sysconfig` - // paths. - sysconfig_paths: SysconfigPaths { - purelib: layout.site_packages( - &venv_root, - self.site_packages_python(), - self.python_tuple(), - ), - platlib: layout.site_packages( - &venv_root, - self.site_packages_python(), - self.python_tuple(), - ), - platstdlib: layout.platstdlib( - &venv_root, - self.site_packages_python(), - self.python_tuple(), - ), - scripts: layout.scripts(&venv_root), - data: layout.data(&venv_root), - ..self.sysconfig_paths - }, - sys_executable: layout.python_executable(&venv_root), - prefix: venv_root, + sysconfig_paths: virtualenv.sysconfig_paths, + sys_executable: virtualenv.executable, + prefix: virtualenv.root, ..self } } @@ -199,9 +174,8 @@ impl Interpreter { }; // Check if the venv Python matches. - let python_platform = VirtualenvLayout::from_platform(platform); - if let Some(venv) = detect_virtual_env(&python_platform)? { - let executable = python_platform.python_executable(venv); + if let Some(venv) = detect_virtual_env()? { + let executable = detect_python_executable(venv); let interpreter = Self::query(&executable, platform.clone(), cache)?; if version_matches(&interpreter) { @@ -376,6 +350,11 @@ impl Interpreter { &self.base_prefix } + /// Return the `sys.prefix` path for this Python interpreter. + pub fn prefix(&self) -> &Path { + &self.prefix + } + /// Return the `sys.executable` path for this Python interpreter. pub fn sys_executable(&self) -> &Path { &self.sys_executable @@ -406,6 +385,11 @@ impl Interpreter { &self.sysconfig_paths.include } + /// Return the `platinclude` path for this Python interpreter, as returned by `sysconfig.get_paths()`. + pub fn platinclude(&self) -> &Path { + &self.sysconfig_paths.platinclude + } + /// Return the `stdlib` path for this Python interpreter, as returned by `sysconfig.get_paths()`. pub fn stdlib(&self) -> &Path { &self.sysconfig_paths.stdlib @@ -464,21 +448,6 @@ impl ExternallyManaged { } } -/// The installation paths returned by `sysconfig.get_paths()`. -/// -/// See: -#[derive(Debug, Deserialize, Serialize, Clone)] -struct SysconfigPaths { - stdlib: PathBuf, - platstdlib: PathBuf, - purelib: PathBuf, - platlib: PathBuf, - include: PathBuf, - platinclude: PathBuf, - scripts: PathBuf, - data: PathBuf, -} - #[derive(Debug, Deserialize, Serialize, Clone)] struct InterpreterInfo { markers: MarkerEnvironment, diff --git a/crates/uv-interpreter/src/lib.rs b/crates/uv-interpreter/src/lib.rs index de859f3a2..cbd529171 100644 --- a/crates/uv-interpreter/src/lib.rs +++ b/crates/uv-interpreter/src/lib.rs @@ -9,20 +9,21 @@ pub use crate::interpreter::Interpreter; pub use crate::python_environment::PythonEnvironment; pub use crate::python_query::{find_default_python, find_requested_python}; pub use crate::python_version::PythonVersion; +pub use crate::sysconfig::SysconfigPaths; +pub use crate::virtualenv::Virtualenv; mod cfg; mod interpreter; mod python_environment; mod python_query; mod python_version; -mod virtualenv_layout; +mod sysconfig; +mod virtualenv; #[derive(Debug, Error)] pub enum Error { - #[error("Expected `{0}` to be a virtualenv, but pyvenv.cfg is missing")] + #[error("Expected `{0}` to be a virtualenv, but `pyvenv.cfg` is missing")] MissingPyVenvCfg(PathBuf), - #[error("Broken virtualenv `{0}`, it contains a pyvenv.cfg but no Python binary at `{1}`")] - BrokenVenv(PathBuf, PathBuf), #[error("Both VIRTUAL_ENV and CONDA_PREFIX are set. Please unset one of them.")] Conflict, #[error("No versions of Python could be found. Is Python installed?")] diff --git a/crates/uv-interpreter/src/python_environment.rs b/crates/uv-interpreter/src/python_environment.rs index fb688eadf..4e1227373 100644 --- a/crates/uv-interpreter/src/python_environment.rs +++ b/crates/uv-interpreter/src/python_environment.rs @@ -8,7 +8,6 @@ use uv_cache::Cache; use uv_fs::{LockedFile, Simplified}; use crate::cfg::PyVenvConfiguration; -use crate::virtualenv_layout::VirtualenvLayout; use crate::{find_default_python, find_requested_python, Error, Interpreter}; /// A Python environment, consisting of a Python [`Interpreter`] and a root directory. @@ -21,12 +20,11 @@ pub struct PythonEnvironment { impl PythonEnvironment { /// Create a [`PythonEnvironment`] for an existing virtual environment. pub fn from_virtualenv(platform: Platform, cache: &Cache) -> Result { - let layout = VirtualenvLayout::from_platform(&platform); - let Some(venv) = detect_virtual_env(&layout)? else { + let Some(venv) = detect_virtual_env()? else { return Err(Error::VenvNotFound); }; let venv = fs_err::canonicalize(venv)?; - let executable = layout.python_executable(&venv); + let executable = detect_python_executable(&venv); let interpreter = Interpreter::query(&executable, platform, cache)?; debug_assert!( @@ -42,14 +40,6 @@ impl PythonEnvironment { }) } - /// Create a [`PythonEnvironment`] for a new virtual environment, created with the given interpreter. - pub fn from_interpreter(interpreter: Interpreter, venv: &Path) -> Self { - Self { - interpreter: interpreter.with_venv_root(venv.to_path_buf()), - root: venv.to_path_buf(), - } - } - /// Create a [`PythonEnvironment`] for a Python interpreter specifier (e.g., a path or a binary name). pub fn from_requested_python( python: &str, @@ -74,6 +64,11 @@ impl PythonEnvironment { }) } + /// Create a [`PythonEnvironment`] from an existing [`Interpreter`] and root directory. + pub fn from_interpreter(interpreter: Interpreter, root: PathBuf) -> Self { + Self { root, interpreter } + } + /// Returns the location of the Python interpreter. pub fn root(&self) -> &Path { &self.root @@ -121,7 +116,7 @@ impl PythonEnvironment { } /// Locate the current virtual environment. -pub(crate) fn detect_virtual_env(layout: &VirtualenvLayout) -> Result, Error> { +pub(crate) fn detect_virtual_env() -> Result, Error> { match ( env::var_os("VIRTUAL_ENV").filter(|value| !value.is_empty()), env::var_os("CONDA_PREFIX").filter(|value| !value.is_empty()), @@ -157,10 +152,6 @@ pub(crate) fn detect_virtual_env(layout: &VirtualenvLayout) -> Result Result) -> PathBuf { + let venv = venv.as_ref(); + if cfg!(windows) { + // Search for `python.exe` in the `Scripts` directory. + let executable = venv.join("Scripts").join("python.exe"); + if executable.exists() { + return executable; + } + + // Apparently, Python installed via msys2 on Windows _might_ produce a POSIX-like layout. + // See: https://github.com/PyO3/maturin/issues/1108 + let executable = venv.join("bin").join("python.exe"); + if executable.exists() { + return executable; + } + + // Fallback for Conda environments. + venv.to_path_buf() + } else { + // Search for `python` in the `bin` directory. + venv.join("bin").join("python") + } +} diff --git a/crates/uv-interpreter/src/sysconfig.rs b/crates/uv-interpreter/src/sysconfig.rs new file mode 100644 index 000000000..64045c952 --- /dev/null +++ b/crates/uv-interpreter/src/sysconfig.rs @@ -0,0 +1,18 @@ +use std::path::PathBuf; + +use serde::{Deserialize, Serialize}; + +/// The installation paths returned by `sysconfig.get_paths()`. +/// +/// See: +#[derive(Debug, Deserialize, Serialize, Clone)] +pub struct SysconfigPaths { + pub stdlib: PathBuf, + pub platstdlib: PathBuf, + pub purelib: PathBuf, + pub platlib: PathBuf, + pub include: PathBuf, + pub platinclude: PathBuf, + pub scripts: PathBuf, + pub data: PathBuf, +} diff --git a/crates/uv-interpreter/src/virtualenv.rs b/crates/uv-interpreter/src/virtualenv.rs new file mode 100644 index 000000000..17c5efb09 --- /dev/null +++ b/crates/uv-interpreter/src/virtualenv.rs @@ -0,0 +1,17 @@ +use std::path::PathBuf; + +use crate::sysconfig::SysconfigPaths; + +/// The layout of a virtual environment. +#[derive(Debug)] +pub struct Virtualenv { + /// The absolute path to the root of the virtualenv, e.g., `/path/to/.venv`. + pub root: PathBuf, + + /// The path to the Python interpreter inside the virtualenv, e.g., `.venv/bin/python` + /// (Unix, Python 3.11). + pub executable: PathBuf, + + /// The `sysconfig` paths for the virtualenv, as returned by `sysconfig.get_paths()`. + pub sysconfig_paths: SysconfigPaths, +} diff --git a/crates/uv-interpreter/src/virtualenv_layout.rs b/crates/uv-interpreter/src/virtualenv_layout.rs deleted file mode 100644 index a05535de7..000000000 --- a/crates/uv-interpreter/src/virtualenv_layout.rs +++ /dev/null @@ -1,83 +0,0 @@ -use std::env::consts::EXE_SUFFIX; -use std::path::Path; -use std::path::PathBuf; - -use platform_host::{Os, Platform}; - -/// Construct paths to various locations inside a virtual environment based on the platform. -#[derive(Debug, Clone, Eq, PartialEq)] -pub(crate) struct VirtualenvLayout<'a>(&'a Platform); - -impl<'a> VirtualenvLayout<'a> { - /// Create a new [`VirtualenvLayout`] for the given platform. - pub(crate) fn from_platform(platform: &'a Platform) -> Self { - Self(platform) - } - - /// Returns the path to the `python` executable inside a virtual environment. - pub(crate) fn python_executable(&self, venv_root: impl AsRef) -> PathBuf { - self.scripts(venv_root).join(format!("python{EXE_SUFFIX}")) - } - - /// Returns the directory in which the binaries are stored inside a virtual environment. - pub(crate) fn scripts(&self, venv_root: impl AsRef) -> PathBuf { - let venv = venv_root.as_ref(); - if matches!(self.0.os(), Os::Windows) { - let bin_dir = venv.join("Scripts"); - if bin_dir.join("python.exe").exists() { - return bin_dir; - } - // Python installed via msys2 on Windows might produce a POSIX-like venv - // See https://github.com/PyO3/maturin/issues/1108 - let bin_dir = venv.join("bin"); - if bin_dir.join("python.exe").exists() { - return bin_dir; - } - // for conda environment - venv.to_path_buf() - } else { - venv.join("bin") - } - } - - /// Returns the path to the `site-packages` directory inside a virtual environment. - pub(crate) fn site_packages( - &self, - venv_root: impl AsRef, - site_packages_python: &str, - version: (u8, u8), - ) -> PathBuf { - let venv = venv_root.as_ref(); - if matches!(self.0.os(), Os::Windows) { - venv.join("Lib").join("site-packages") - } else { - venv.join("lib") - .join(format!("{site_packages_python}{}.{}", version.0, version.1)) - .join("site-packages") - } - } - - /// Returns the path to the `data` directory inside a virtual environment. - #[allow(clippy::unused_self)] - pub(crate) fn data(&self, venv_root: impl AsRef) -> PathBuf { - venv_root.as_ref().to_path_buf() - } - - /// Returns the path to the `platstdlib` directory inside a virtual environment. - #[allow(clippy::unused_self)] - pub(crate) fn platstdlib( - &self, - venv_root: impl AsRef, - site_packages_python: &str, - version: (u8, u8), - ) -> PathBuf { - let venv = venv_root.as_ref(); - if matches!(self.0.os(), Os::Windows) { - venv.join("Lib") - } else { - venv.join("lib") - .join(format!("{site_packages_python}{}.{}", version.0, version.1)) - .join("site-packages") - } - } -}