From 1f95a1705d4df4b58267d4583e65a1d7a74c9a9d Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Wed, 21 Feb 2024 22:34:30 -0500 Subject: [PATCH 1/4] feat: allow passing extra config k,v pairs for pyvenv.cfg when creating venv --- crates/gourgeist/src/bare.rs | 86 ++++++++++++++++++++++++---------- crates/gourgeist/src/lib.rs | 3 +- crates/gourgeist/src/main.rs | 2 +- crates/uv-build/src/lib.rs | 7 +++ crates/uv/src/commands/venv.rs | 6 ++- crates/uv/tests/venv.rs | 21 ++++++++- 6 files changed, 97 insertions(+), 28 deletions(-) diff --git a/crates/gourgeist/src/bare.rs b/crates/gourgeist/src/bare.rs index aff617b70c27..376706670b11 100644 --- a/crates/gourgeist/src/bare.rs +++ b/crates/gourgeist/src/bare.rs @@ -35,7 +35,7 @@ const VIRTUALENV_PATCH: &str = include_str!("_virtualenv.py"); /// Very basic `.cfg` file format writer. fn write_cfg( f: &mut impl Write, - data: &[(&str, String); 8], + data: &[(String, String)], prompt: Option, ) -> io::Result<()> { for (key, value) in data { @@ -73,6 +73,7 @@ pub fn create_bare_venv( location: &Utf8Path, interpreter: &Interpreter, prompt: Prompt, + extra_cfg: Vec<(String, String)>, ) -> io::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. @@ -226,31 +227,68 @@ pub fn create_bare_venv( } else { unimplemented!("Only Windows and Unix are supported") }; - let pyvenv_cfg_data = &[ - ("home", python_home), - ( - "implementation", - interpreter.markers().platform_python_implementation.clone(), - ), - ( - "version_info", - interpreter.markers().python_version.string.clone(), - ), - ("gourgeist", env!("CARGO_PKG_VERSION").to_string()), - // I wouldn't allow this option anyway - ("include-system-site-packages", "false".to_string()), - ( - "base-prefix", - interpreter.base_prefix().to_string_lossy().to_string(), - ), - ( - "base-exec-prefix", - interpreter.base_exec_prefix().to_string_lossy().to_string(), - ), - ("base-executable", base_python.to_string()), + + // Validate extra_cfg + let reserved_keys = [ + "home", + "implementation", + "version_info", + "gourgeist", + "include-system-site-packages", + "base-prefix", + "base-exec-prefix", + "base-executable", + "prompt", ]; + for (key, _) in &extra_cfg { + if reserved_keys.contains(&key.as_str()) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Reserved key found in extra_cfg: {key}"), + )); + } + } + + let pyvenv_cfg_data: Vec<(String, String)> = [ + vec![ + ("home".to_string(), python_home), + ( + "implementation".to_string(), + interpreter.markers().platform_python_implementation.clone(), + ), + ( + "version_info".to_string(), + interpreter.markers().python_version.string.clone(), + ), + ( + "gourgeist".to_string(), + env!("CARGO_PKG_VERSION").to_string(), + ), + ], + // Put custom cfg pairs after "gourgeist" + extra_cfg, + vec![ + ( + "include-system-site-packages".to_string(), + "false".to_string(), + ), + ( + "base-prefix".to_string(), + interpreter.base_prefix().to_string_lossy().to_string(), + ), + ( + "base-exec-prefix".to_string(), + interpreter.base_exec_prefix().to_string_lossy().to_string(), + ), + ("base-executable".to_string(), base_python.to_string()), + ], + ] + .into_iter() + .flatten() + .collect(); + let mut pyvenv_cfg = BufWriter::new(File::create(location.join("pyvenv.cfg"))?); - write_cfg(&mut pyvenv_cfg, pyvenv_cfg_data, prompt)?; + write_cfg(&mut pyvenv_cfg, &pyvenv_cfg_data, prompt)?; drop(pyvenv_cfg); let site_packages = if cfg!(unix) { diff --git a/crates/gourgeist/src/lib.rs b/crates/gourgeist/src/lib.rs index 9545253023ff..57ae639afb3d 100644 --- a/crates/gourgeist/src/lib.rs +++ b/crates/gourgeist/src/lib.rs @@ -51,11 +51,12 @@ pub fn create_venv( location: &Path, interpreter: Interpreter, prompt: Prompt, + extra_cfg: Vec<(String, String)>, ) -> Result { let location: &Utf8Path = location .try_into() .map_err(|err: FromPathError| err.into_io_error())?; - let paths = create_bare_venv(location, &interpreter, prompt)?; + let paths = create_bare_venv(location, &interpreter, prompt, extra_cfg)?; Ok(Virtualenv::from_interpreter( interpreter, paths.root.as_std_path(), diff --git a/crates/gourgeist/src/main.rs b/crates/gourgeist/src/main.rs index a6718fef24c2..13691a8985ec 100644 --- a/crates/gourgeist/src/main.rs +++ b/crates/gourgeist/src/main.rs @@ -36,7 +36,7 @@ fn run() -> Result<(), gourgeist::Error> { Cache::from_path(".gourgeist_cache")? }; let info = Interpreter::query(python.as_std_path(), &platform, &cache).unwrap(); - create_bare_venv(&location, &info, Prompt::from_args(cli.prompt))?; + create_bare_venv(&location, &info, Prompt::from_args(cli.prompt), Vec::new())?; Ok(()) } diff --git a/crates/uv-build/src/lib.rs b/crates/uv-build/src/lib.rs index 2708b194a5fb..ae13bb354739 100644 --- a/crates/uv-build/src/lib.rs +++ b/crates/uv-build/src/lib.rs @@ -325,10 +325,17 @@ impl SourceBuild { let pep517_backend = Self::get_pep517_backend(setup_py, &source_tree, &default_backend) .map_err(|err| *err)?; + // Extra cfg for pyvenv.cfg to specify build backend + let extra_cfg = vec![( + "uv-build".to_string(), + env!("CARGO_PKG_VERSION").to_string(), + )]; + let venv = gourgeist::create_venv( &temp_dir.path().join(".venv"), interpreter.clone(), gourgeist::Prompt::None, + extra_cfg, )?; // Setup the build environment. diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 72cbfc800463..ba38edd36382 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -119,8 +119,12 @@ async fn venv_impl( ) .into_diagnostic()?; + // Extra cfg for pyvenv.cfg to specify uv version + let extra_cfg = vec![("uv".to_string(), env!("CARGO_PKG_VERSION").to_string())]; + // Create the virtual environment. - let venv = gourgeist::create_venv(path, interpreter, prompt).map_err(VenvError::Creation)?; + let venv = gourgeist::create_venv(path, interpreter, prompt, extra_cfg) + .map_err(VenvError::Creation)?; // Install seed packages. if seed { diff --git a/crates/uv/tests/venv.rs b/crates/uv/tests/venv.rs index f2d9585f943e..5cc52d65bead 100644 --- a/crates/uv/tests/venv.rs +++ b/crates/uv/tests/venv.rs @@ -7,7 +7,9 @@ use assert_fs::prelude::*; use uv_fs::Normalized; -use crate::common::{create_bin_with_executables, get_bin, uv_snapshot, EXCLUDE_NEWER}; +use crate::common::{ + create_bin_with_executables, get_bin, uv_snapshot, TestContext, EXCLUDE_NEWER, +}; mod common; @@ -575,3 +577,20 @@ fn virtualenv_compatibility() -> Result<()> { Ok(()) } + +#[test] +fn verify_pyvenv_cfg() { + let context = TestContext::new("3.12"); + let venv = context.temp_dir.child(".venv"); + let pyvenv_cfg = venv.child("pyvenv.cfg"); + + venv.assert(predicates::path::is_dir()); + + // Check pyvenv.cfg exists + pyvenv_cfg.assert(predicates::path::is_file()); + + // Check if "uv = version" is present in the file + let version = env!("CARGO_PKG_VERSION").to_string(); + let search_string = format!("uv = {version}"); + pyvenv_cfg.assert(predicates::str::contains(search_string)); +} From 258e4034a7aec9768ce0cee8121fafa3f44bf6b5 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 22 Feb 2024 08:12:47 -0500 Subject: [PATCH 2/4] fixup: implement suggestions --- crates/gourgeist/src/bare.rs | 74 +++++++++++++++--------------------- 1 file changed, 30 insertions(+), 44 deletions(-) diff --git a/crates/gourgeist/src/bare.rs b/crates/gourgeist/src/bare.rs index 376706670b11..f0f9a0a4b426 100644 --- a/crates/gourgeist/src/bare.rs +++ b/crates/gourgeist/src/bare.rs @@ -33,17 +33,10 @@ const ACTIVATE_TEMPLATES: &[(&str, &str)] = &[ const VIRTUALENV_PATCH: &str = include_str!("_virtualenv.py"); /// Very basic `.cfg` file format writer. -fn write_cfg( - f: &mut impl Write, - data: &[(String, String)], - prompt: Option, -) -> io::Result<()> { +fn write_cfg(f: &mut impl Write, data: &[(String, String)]) -> io::Result<()> { for (key, value) in data { writeln!(f, "{key} = {value}")?; } - if let Some(prompt) = prompt { - writeln!(f, "prompt = {prompt}")?; - } Ok(()) } @@ -233,7 +226,6 @@ pub fn create_bare_venv( "home", "implementation", "version_info", - "gourgeist", "include-system-site-packages", "base-prefix", "base-exec-prefix", @@ -249,46 +241,40 @@ pub fn create_bare_venv( } } - let pyvenv_cfg_data: Vec<(String, String)> = [ - vec![ - ("home".to_string(), python_home), - ( - "implementation".to_string(), - interpreter.markers().platform_python_implementation.clone(), - ), - ( - "version_info".to_string(), - interpreter.markers().python_version.string.clone(), - ), - ( - "gourgeist".to_string(), - env!("CARGO_PKG_VERSION").to_string(), - ), - ], - // Put custom cfg pairs after "gourgeist" - extra_cfg, - vec![ - ( - "include-system-site-packages".to_string(), - "false".to_string(), - ), - ( - "base-prefix".to_string(), - interpreter.base_prefix().to_string_lossy().to_string(), - ), - ( - "base-exec-prefix".to_string(), - interpreter.base_exec_prefix().to_string_lossy().to_string(), - ), - ("base-executable".to_string(), base_python.to_string()), - ], + let mut pyvenv_cfg_data: Vec<(String, String)> = vec![ + ("home".to_string(), python_home), + ( + "implementation".to_string(), + interpreter.markers().platform_python_implementation.clone(), + ), + ( + "version_info".to_string(), + interpreter.markers().python_version.string.clone(), + ), + ( + "include-system-site-packages".to_string(), + "false".to_string(), + ), + ( + "base-prefix".to_string(), + interpreter.base_prefix().to_string_lossy().to_string(), + ), + ( + "base-exec-prefix".to_string(), + interpreter.base_exec_prefix().to_string_lossy().to_string(), + ), + ("base-executable".to_string(), base_python.to_string()), ] .into_iter() - .flatten() + .chain(extra_cfg) .collect(); + if let Some(prompt) = prompt { + pyvenv_cfg_data.push(("prompt".to_string(), prompt)); + } + let mut pyvenv_cfg = BufWriter::new(File::create(location.join("pyvenv.cfg"))?); - write_cfg(&mut pyvenv_cfg, &pyvenv_cfg_data, prompt)?; + write_cfg(&mut pyvenv_cfg, &pyvenv_cfg_data)?; drop(pyvenv_cfg); let site_packages = if cfg!(unix) { From 54f6891a7d99554622c5dfb937c22ee6e9b300bb Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 22 Feb 2024 08:30:33 -0500 Subject: [PATCH 3/4] Remove uv-build --- crates/uv-build/src/lib.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/uv-build/src/lib.rs b/crates/uv-build/src/lib.rs index ae13bb354739..d63ba89f6749 100644 --- a/crates/uv-build/src/lib.rs +++ b/crates/uv-build/src/lib.rs @@ -325,17 +325,11 @@ impl SourceBuild { let pep517_backend = Self::get_pep517_backend(setup_py, &source_tree, &default_backend) .map_err(|err| *err)?; - // Extra cfg for pyvenv.cfg to specify build backend - let extra_cfg = vec![( - "uv-build".to_string(), - env!("CARGO_PKG_VERSION").to_string(), - )]; - let venv = gourgeist::create_venv( &temp_dir.path().join(".venv"), interpreter.clone(), gourgeist::Prompt::None, - extra_cfg, + Vec::new(), )?; // Setup the build environment. From 64031502338b101f3993e17475c00853a9575562 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 22 Feb 2024 11:15:17 -0500 Subject: [PATCH 4/4] suggestion: move `create_bare_venv` to return `Result` --- crates/gourgeist/src/bare.rs | 19 ++++++++----------- crates/gourgeist/src/lib.rs | 2 ++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/gourgeist/src/bare.rs b/crates/gourgeist/src/bare.rs index f0f9a0a4b426..da9a658377c4 100644 --- a/crates/gourgeist/src/bare.rs +++ b/crates/gourgeist/src/bare.rs @@ -13,7 +13,7 @@ use uv_fs::Normalized; use uv_interpreter::Interpreter; -use crate::Prompt; +use crate::{Error, Prompt}; /// The bash activate scripts with the venv dependent paths patches out const ACTIVATE_TEMPLATES: &[(&str, &str)] = &[ @@ -67,7 +67,7 @@ pub fn create_bare_venv( interpreter: &Interpreter, prompt: Prompt, extra_cfg: Vec<(String, String)>, -) -> io::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())? @@ -78,10 +78,10 @@ pub fn create_bare_venv( match location.metadata() { Ok(metadata) => { if metadata.is_file() { - return Err(io::Error::new( + return Err(Error::IO(io::Error::new( io::ErrorKind::AlreadyExists, format!("File exists at `{location}`"), - )); + ))); } else if metadata.is_dir() { if location.join("pyvenv.cfg").is_file() { info!("Removing existing directory"); @@ -93,17 +93,17 @@ pub fn create_bare_venv( { info!("Ignoring empty directory"); } else { - return Err(io::Error::new( + return Err(Error::IO(io::Error::new( io::ErrorKind::AlreadyExists, format!("The directory `{location}` exists, but it's not a virtualenv"), - )); + ))); } } } Err(err) if err.kind() == io::ErrorKind::NotFound => { fs::create_dir_all(location)?; } - Err(err) => return Err(err), + Err(err) => return Err(Error::IO(err)), } // TODO(konstin): I bet on windows we'll have to strip the prefix again @@ -234,10 +234,7 @@ pub fn create_bare_venv( ]; for (key, _) in &extra_cfg { if reserved_keys.contains(&key.as_str()) { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("Reserved key found in extra_cfg: {key}"), - )); + return Err(Error::ReservedConfigKey(key.to_string())); } } diff --git a/crates/gourgeist/src/lib.rs b/crates/gourgeist/src/lib.rs index 57ae639afb3d..c4c2ec83c0e1 100644 --- a/crates/gourgeist/src/lib.rs +++ b/crates/gourgeist/src/lib.rs @@ -21,6 +21,8 @@ pub enum Error { InvalidPythonInterpreter(#[source] Box), #[error(transparent)] Platform(#[from] PlatformError), + #[error("Reserved key used for pyvenv.cfg: {0}")] + ReservedConfigKey(String), } /// The value to use for the shell prompt when inside a virtual environment.