diff --git a/Cargo.lock b/Cargo.lock index ef34d6501..4506a5b04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -332,7 +332,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c48f0051a4b4c5e0b6d365cd04af53aeaa209e3cc15ec2cdb69e73cc87fbd0dc" dependencies = [ "memchr", - "regex-automata 0.4.3", + "regex-automata 0.4.4", "serde", ] @@ -1115,7 +1115,7 @@ dependencies = [ "aho-corasick", "bstr", "log", - "regex-automata 0.4.3", + "regex-automata 0.4.4", "regex-syntax 0.8.2", ] @@ -1378,7 +1378,7 @@ dependencies = [ "globset", "log", "memchr", - "regex-automata 0.4.3", + "regex-automata 0.4.4", "same-file", "walkdir", "winapi-util", @@ -2267,6 +2267,7 @@ dependencies = [ "clap", "distribution-filename", "distribution-types", + "filetime", "flate2", "fs-err", "futures", @@ -2300,6 +2301,7 @@ dependencies = [ "puffin-workspace", "pypi-types", "pyproject-toml", + "regex", "requirements-txt", "reqwest", "rustc-hash", @@ -2948,13 +2950,13 @@ dependencies = [ [[package]] name = "regex" -version = "1.10.2" +version = "1.10.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "380b951a9c5e80ddfd6136919eef32310721aa4aacd4889a8d39124b026ab343" +checksum = "b62dbe01f0b06f9d8dc7d49e05a0785f153b00b2c227856282f671e0318c9b15" dependencies = [ "aho-corasick", "memchr", - "regex-automata 0.4.3", + "regex-automata 0.4.4", "regex-syntax 0.8.2", ] @@ -2969,9 +2971,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.3" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f804c7828047e88b2d32e2d7fe5a105da8ee3264f01902f796c8e067dc2483f" +checksum = "3b7fa1134405e2ec9353fd416b17f8dacd46c473d7d3fd1cf202706a14eb792a" dependencies = [ "aho-corasick", "memchr", diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index c04eab4bd..376b02a65 100644 --- a/crates/distribution-types/src/lib.rs +++ b/crates/distribution-types/src/lib.rs @@ -864,8 +864,22 @@ mod test { /// Ensure that we don't accidentally grow the `Dist` sizes. #[test] fn dist_size() { - assert!(std::mem::size_of::() <= 240); - assert!(std::mem::size_of::() <= 240); - assert!(std::mem::size_of::() <= 168); + // At time of writing, Unix is at 240, Windows is at 248. + assert!( + std::mem::size_of::() <= 248, + "{}", + std::mem::size_of::() + ); + assert!( + std::mem::size_of::() <= 248, + "{}", + std::mem::size_of::() + ); + // At time of writing, unix is at 168, windows is at 176. + assert!( + std::mem::size_of::() <= 176, + "{}", + std::mem::size_of::() + ); } } diff --git a/crates/gourgeist/src/activator/activate.ps1 b/crates/gourgeist/src/activator/activate.ps1 index 47d185320..4a9ad02fd 100644 --- a/crates/gourgeist/src/activator/activate.ps1 +++ b/crates/gourgeist/src/activator/activate.ps1 @@ -46,7 +46,7 @@ else { New-Variable -Scope global -Name _OLD_VIRTUAL_PATH -Value $env:PATH -$env:PATH = "$env:VIRTUAL_ENV/bin:" + $env:PATH +$env:PATH = "$env:VIRTUAL_ENV/{{ BIN_NAME }};" + $env:PATH if (!$env:VIRTUAL_ENV_DISABLE_PROMPT) { function global:_old_virtual_prompt { "" diff --git a/crates/gourgeist/src/bare.rs b/crates/gourgeist/src/bare.rs index b814c0b2b..4f343972d 100644 --- a/crates/gourgeist/src/bare.rs +++ b/crates/gourgeist/src/bare.rs @@ -5,8 +5,6 @@ use std::io::{BufWriter, Write}; use camino::{FromPathBufError, Utf8Path, Utf8PathBuf}; use fs_err as fs; -#[cfg(unix)] -use fs_err::os::unix::fs::symlink; use fs_err::File; use tracing::info; @@ -76,41 +74,33 @@ pub fn create_bare_venv(location: &Utf8Path, interpreter: &Interpreter) -> io::R fs::create_dir_all(location)?; // TODO(konstin): I bet on windows we'll have to strip the prefix again let location = location.canonicalize_utf8()?; - let bin_dir = { - #[cfg(unix)] - { - location.join("bin") - } - #[cfg(windows)] - { - location.join("Scripts") - } - #[cfg(not(any(unix, windows)))] - { - compile_error!("only unix (like mac and linux) and windows are supported") - } + let bin_name = if cfg!(unix) { + "bin" + } else if cfg!(windows) { + "Scripts" + } else { + unimplemented!("Only Windows and Unix are supported") }; + let bin_dir = location.join(bin_name); fs::write(location.join(".gitignore"), "*")?; // Different names for the python interpreter fs::create_dir(&bin_dir)?; let venv_python = { - #[cfg(unix)] - { + if cfg!(unix) { bin_dir.join("python") - } - #[cfg(windows)] - { + } else if cfg!(windows) { bin_dir.join("python.exe") - } - #[cfg(not(any(unix, windows)))] - { - compile_error!("only unix (like mac and linux) and windows are supported") + } else { + unimplemented!("Only Windows and Unix are supported") } }; + // 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( "python", @@ -125,11 +115,29 @@ pub fn create_bare_venv(location: &Utf8Path, interpreter: &Interpreter) -> io::R )), )?; } + #[cfg(windows)] + { + // https://github.com/python/cpython/blob/d457345bbc6414db0443819290b04a9a4333313d/Lib/venv/__init__.py#L261-L267 + // https://github.com/pypa/virtualenv/blob/d9fdf48d69f0d0ca56140cf0381edbb5d6fe09f5/src/virtualenv/create/via_global_ref/builtin/cpython/cpython3.py#L78-L83 + let shim = interpreter + .stdlib() + .join("venv") + .join("scripts") + .join("nt") + .join("python.exe"); + fs_err::copy(shim, bin_dir.join("python.exe"))?; + } + #[cfg(not(any(unix, windows)))] + { + compile_error!("Only Windows and Unix are supported") + } // Add all the activate scripts for different shells + // TODO(konstin): That's unix! for (name, template) in ACTIVATE_TEMPLATES { let activator = template .replace("{{ VIRTUAL_ENV_DIR }}", location.as_str()) + .replace("{{ BIN_NAME }}", bin_name) .replace( "{{ RELATIVE_SITE_PACKAGES }}", &format!( @@ -142,15 +150,26 @@ pub fn create_bare_venv(location: &Utf8Path, interpreter: &Interpreter) -> io::R } // pyvenv.cfg - let python_home = base_python - .parent() - .ok_or_else(|| { - io::Error::new( - io::ErrorKind::NotFound, - "The python interpreter needs to have a parent directory", - ) - })? - .to_string(); + let python_home = if cfg!(unix) { + // On Linux and Mac, Python is symlinked so the base home is the parent of the resolved-by-canonicalize path. + base_python + .parent() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::NotFound, + "The python interpreter needs to have a parent directory", + ) + })? + .to_string() + } else if cfg!(windows) { + // `virtualenv` seems to rely on the undocumented, private `sys._base_executable`. When I tried, + // `sys.base_prefix` was the same as the parent of `sys._base_executable`, but a much simpler logic and + // documented. + // https://github.com/pypa/virtualenv/blob/d9fdf48d69f0d0ca56140cf0381edbb5d6fe09f5/src/virtualenv/discovery/py_info.py#L136-L156 + interpreter.base_prefix().display().to_string() + } else { + unimplemented!("Only Windows and Unix are supported") + }; let pyvenv_cfg_data = &[ ("home", python_home), ("implementation", "CPython".to_string()), @@ -175,15 +194,20 @@ pub fn create_bare_venv(location: &Utf8Path, interpreter: &Interpreter) -> io::R write_cfg(&mut pyvenv_cfg, pyvenv_cfg_data)?; drop(pyvenv_cfg); - // TODO: This is different on windows - let site_packages = location - .join("lib") - .join(format!( - "python{}.{}", - interpreter.python_major(), - interpreter.python_minor(), - )) - .join("site-packages"); + let site_packages = if cfg!(unix) { + location + .join("lib") + .join(format!( + "python{}.{}", + interpreter.python_major(), + interpreter.python_minor(), + )) + .join("site-packages") + } else if cfg!(windows) { + location.join("Lib").join("site-packages") + } else { + unimplemented!("Only Windows and Unix are supported") + }; fs::create_dir_all(&site_packages)?; // Install _virtualenv.py patch. // Frankly no idea what that does, i just copied it from virtualenv knowing that diff --git a/crates/install-wheel-rs/src/install_location.rs b/crates/install-wheel-rs/src/install_location.rs index e96448f39..b3590c8f6 100644 --- a/crates/install-wheel-rs/src/install_location.rs +++ b/crates/install-wheel-rs/src/install_location.rs @@ -89,11 +89,13 @@ impl> InstallLocation { /// Returns the location of the `python` interpreter. pub fn python(&self) -> PathBuf { - if cfg!(windows) { - self.venv_root.as_ref().join("Scripts").join("python.exe") - } else { + if cfg!(unix) { // canonicalize on python would resolve the symlink self.venv_root.as_ref().join("bin").join("python") + } else if cfg!(windows) { + self.venv_root.as_ref().join("Scripts").join("python.exe") + } else { + unimplemented!("Only Windows and Unix are supported") } } diff --git a/crates/pep508-rs/src/lib.rs b/crates/pep508-rs/src/lib.rs index 03f136410..59ef56f99 100644 --- a/crates/pep508-rs/src/lib.rs +++ b/crates/pep508-rs/src/lib.rs @@ -16,6 +16,7 @@ #![deny(missing_docs)] +use std::borrow::Cow; #[cfg(feature = "pyo3")] use std::collections::hash_map::DefaultHasher; use std::collections::HashSet; @@ -714,10 +715,32 @@ fn parse_url(cursor: &mut Cursor, working_dir: Option<&Path>) -> Result, + cursor: &Cursor, + start: usize, + len: usize, +) -> Result { let url = if let Some((scheme, path)) = split_scheme(url) { if scheme == "file" { if let Some(path) = path.strip_prefix("//") { + let path = if cfg!(windows) { + // Transform `/C:/Users/ferris/wheel-0.42.0.tar.gz` to `C:\Users\ferris\wheel-0.42.0.tar.gz` + Cow::Owned( + path.strip_prefix('/') + .unwrap_or(path) + .replace('/', std::path::MAIN_SEPARATOR_STR), + ) + } else { + Cow::Borrowed(path) + }; // Ex) `file:///home/ferris/project/scripts/...` if let Some(working_dir) = working_dir { VerbatimUrl::from_path(path, working_dir).with_given(url.to_string()) @@ -770,7 +793,6 @@ fn parse_url(cursor: &mut Cursor, working_dir: Option<&Path>) -> Result Result<(), Pep508Error> { - /// use puffin_normalize::ExtraName; /// let marker_tree = MarkerTree::from_str(r#"("linux" in sys_platform) and extra == 'day'"#)?; /// let versions: Vec = (8..12).map(|minor| Version::new([3, minor])).collect(); /// assert!(marker_tree.evaluate_extras_and_python_version(&[ExtraName::from_str("day").unwrap()].into(), &versions)); diff --git a/crates/puffin-interpreter/src/get_interpreter_info.py b/crates/puffin-interpreter/src/get_interpreter_info.py index c05494e4b..b192ea89c 100644 --- a/crates/puffin-interpreter/src/get_interpreter_info.py +++ b/crates/puffin-interpreter/src/get_interpreter_info.py @@ -2,6 +2,7 @@ import json import os import platform import sys +import sysconfig def format_full_version(info): @@ -35,6 +36,7 @@ interpreter_info = { "markers": markers, "base_prefix": sys.base_prefix, "base_exec_prefix": sys.base_exec_prefix, + "stdlib": sysconfig.get_path("stdlib"), "sys_executable": sys.executable, } print(json.dumps(interpreter_info)) diff --git a/crates/puffin-interpreter/src/interpreter.rs b/crates/puffin-interpreter/src/interpreter.rs index 9b5cf3ea4..84be2ee42 100644 --- a/crates/puffin-interpreter/src/interpreter.rs +++ b/crates/puffin-interpreter/src/interpreter.rs @@ -26,6 +26,7 @@ pub struct Interpreter { pub(crate) markers: MarkerEnvironment, pub(crate) base_exec_prefix: PathBuf, pub(crate) base_prefix: PathBuf, + pub(crate) stdlib: PathBuf, pub(crate) sys_executable: PathBuf, tags: OnceCell, } @@ -51,6 +52,7 @@ impl Interpreter { markers: info.markers, base_exec_prefix: info.base_exec_prefix, base_prefix: info.base_prefix, + stdlib: info.stdlib, sys_executable: info.sys_executable, tags: OnceCell::new(), }) @@ -63,12 +65,14 @@ impl Interpreter { base_exec_prefix: PathBuf, base_prefix: PathBuf, sys_executable: PathBuf, + stdlib: PathBuf, ) -> Self { Self { platform: PythonPlatform(platform), markers, base_exec_prefix, base_prefix, + stdlib, sys_executable, tags: OnceCell::new(), } @@ -280,6 +284,11 @@ impl Interpreter { pub fn base_prefix(&self) -> &Path { &self.base_prefix } + + /// `sysconfig.get_path("stdlib")` + pub fn stdlib(&self) -> &Path { + &self.stdlib + } pub fn sys_executable(&self) -> &Path { &self.sys_executable } @@ -290,6 +299,7 @@ pub(crate) struct InterpreterQueryResult { pub(crate) markers: MarkerEnvironment, pub(crate) base_exec_prefix: PathBuf, pub(crate) base_prefix: PathBuf, + pub(crate) stdlib: PathBuf, pub(crate) sys_executable: PathBuf, } @@ -443,6 +453,7 @@ impl Timestamp { } } +#[cfg(unix)] #[cfg(test)] mod tests { use std::str::FromStr; @@ -458,7 +469,6 @@ mod tests { use crate::Interpreter; #[test] - #[cfg(unix)] fn test_cache_invalidation() { let mock_dir = tempdir().unwrap(); let mocked_interpreter = mock_dir.path().join("python"); @@ -479,6 +489,7 @@ mod tests { }, "base_exec_prefix": "/home/ferris/.pyenv/versions/3.12.0", "base_prefix": "/home/ferris/.pyenv/versions/3.12.0", + "stdlib": "/usr/lib/python3.12", "sys_executable": "/home/ferris/projects/puffin/.venv/bin/python" } "##}; diff --git a/crates/puffin-interpreter/src/lib.rs b/crates/puffin-interpreter/src/lib.rs index 193110e40..72a9b6f1e 100644 --- a/crates/puffin-interpreter/src/lib.rs +++ b/crates/puffin-interpreter/src/lib.rs @@ -6,7 +6,7 @@ use thiserror::Error; pub use crate::cfg::Configuration; pub use crate::interpreter::Interpreter; -pub use crate::python_query::find_requested_python; +pub use crate::python_query::{find_default_python, find_requested_python}; pub use crate::python_version::PythonVersion; pub use crate::virtual_env::Virtualenv; @@ -41,10 +41,14 @@ pub enum Error { #[source] err: io::Error, }, - #[error("Failed to run `py --list-paths` to find Python installations")] + #[error("Failed to run `py --list-paths` to find Python installations. Do you need to install Python?")] PyList(#[source] io::Error), #[error("No Python {major}.{minor} found through `py --list-paths`")] NoSuchPython { major: u8, minor: u8 }, + #[error("Neither `python` nor `python3` are in `PATH`. Do you need to install Python?")] + NoPythonInstalledUnix, + #[error("Could not find `python.exe` in PATH and `py --list-paths` did not list any Python versions. Do you need to install Python?")] + NoPythonInstalledWindows, #[error("{message}:\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")] PythonSubcommandOutput { message: String, diff --git a/crates/puffin-interpreter/src/python_query.rs b/crates/puffin-interpreter/src/python_query.rs index c4a948063..7e7948ec9 100644 --- a/crates/puffin-interpreter/src/python_query.rs +++ b/crates/puffin-interpreter/src/python_query.rs @@ -5,7 +5,7 @@ use std::process::Command; use once_cell::sync::Lazy; use regex::Regex; -use tracing::info_span; +use tracing::{info_span, instrument}; use crate::Error; @@ -25,6 +25,7 @@ static PY_LIST_PATHS: Lazy = Lazy::new(|| { /// Specifying a patch version is not supported. /// * `-p python3.10` or `-p python.exe` looks for a binary in `PATH`. /// * `-p /home/ferris/.local/bin/python3.10` uses this exact Python. +#[instrument] pub fn find_requested_python(request: &str) -> Result { let major_minor = request .split_once('.') @@ -49,11 +50,37 @@ pub fn find_requested_python(request: &str) -> Result { } } +/// Pick a sensible default for the python a user wants when they didn't specify a version. +#[instrument] +pub fn find_default_python() -> Result { + let python = if cfg!(unix) { + which::which_global("python3") + .or_else(|_| which::which_global("python")) + .map_err(|_| Error::NoPythonInstalledUnix)? + } else if cfg!(windows) { + // TODO(konstin): Is that the right order, or should we look for `py --list-paths` first? With the current way + // it works even if the python launcher is not installed. + if let Ok(python) = which::which_global("python.exe") { + python + } else { + installed_pythons_windows()? + .into_iter() + .next() + .ok_or(Error::NoPythonInstalledWindows)? + .2 + } + } else { + unimplemented!("Only Windows and Unix are supported") + }; + return Ok(fs_err::canonicalize(python)?); +} + /// Run `py --list-paths` to find the installed pythons. /// /// The command takes 8ms on my machine. TODO(konstin): Implement to read python /// installations from the registry instead. fn installed_pythons_windows() -> Result, Error> { + // TODO(konstin): Special case the not found error let output = info_span!("py_list_paths") .in_scope(|| Command::new("py").arg("--list-paths").output()) .map_err(Error::PyList)?; @@ -77,7 +104,7 @@ fn installed_pythons_windows() -> Result, Error> { stdout: String::from_utf8_lossy(&output.stdout).trim().to_string(), stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(), })?; - Ok(PY_LIST_PATHS + let pythons = PY_LIST_PATHS .captures_iter(&stdout) .filter_map(|captures| { let (_, [major, minor, path]) = captures.extract(); @@ -87,7 +114,8 @@ fn installed_pythons_windows() -> Result, Error> { PathBuf::from(path), )) }) - .collect()) + .collect(); + Ok(pythons) } pub(crate) fn find_python_windows(major: u8, minor: u8) -> Result, Error> { @@ -138,6 +166,7 @@ mod tests { "###); } + #[cfg(unix)] #[test] fn no_such_python_path() { assert_display_snapshot!( @@ -146,4 +175,21 @@ mod tests { Caused by: No such file or directory (os error 2) "###); } + + #[cfg(windows)] + #[test] + fn no_such_python_path() { + insta::with_settings!({ + filters => vec![ + // The exact message is host language dependent + ("Caused by: .* (os error 3)", "Caused by: The system cannot find the path specified. (os error 3)") + ] + }, { + assert_display_snapshot!( + format_err(find_requested_python(r"C:\does\not\exists\python3.12")), @r###" + failed to canonicalize path `C:\does\not\exists\python3.12` + Caused by: The system cannot find the path specified. (os error 3) + "###); + }); + } } diff --git a/crates/puffin-interpreter/src/virtual_env.rs b/crates/puffin-interpreter/src/virtual_env.rs index 4d59eaa28..70f7e60e4 100644 --- a/crates/puffin-interpreter/src/virtual_env.rs +++ b/crates/puffin-interpreter/src/virtual_env.rs @@ -45,17 +45,12 @@ impl Virtualenv { /// Returns the location of the python interpreter pub fn python_executable(&self) -> PathBuf { - #[cfg(unix)] - { - self.root.join("bin").join("python") - } - #[cfg(windows)] - { - self.root.join("Scripts").join("python.exe") - } - #[cfg(not(any(unix, windows)))] - { - compile_error!("Only windows and unix (linux, mac os, etc.) are supported") + if cfg!(unix) { + self.bin_dir().join("python") + } else if cfg!(windows) { + self.bin_dir().join("python.exe") + } else { + unimplemented!("Only Windows and Unix are supported") } } @@ -82,17 +77,12 @@ impl Virtualenv { } pub fn bin_dir(&self) -> PathBuf { - #[cfg(unix)] - { + if cfg!(unix) { self.root().join("bin") - } - #[cfg(windows)] - { + } else if cfg!(windows) { self.root().join("Scripts") - } - #[cfg(not(any(unix, windows)))] - { - compile_error!("only unix (like mac and linux) and windows are supported") + } else { + unimplemented!("Only Windows and Unix are supported") } } diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index 8da1cbbb1..336739762 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -112,6 +112,7 @@ async fn resolve( PathBuf::from("/dev/null"), PathBuf::from("/dev/null"), PathBuf::from("/dev/null"), + PathBuf::from("/dev/null"), ); let build_context = DummyContext { cache: Cache::temp()?, diff --git a/crates/puffin/Cargo.toml b/crates/puffin/Cargo.toml index 3e7b4fcd2..c97565e26 100644 --- a/crates/puffin/Cargo.toml +++ b/crates/puffin/Cargo.toml @@ -77,10 +77,12 @@ tikv-jemallocator = "0.5.4" [dev-dependencies] assert_cmd = { version = "2.0.12" } assert_fs = { version = "1.1.0" } +filetime = { version = "0.2.23" } indoc = { version = "2.0.4" } -insta-cmd = { version = "0.4.0" } insta = { version = "1.34.0", features = ["filters"] } +insta-cmd = { version = "0.4.0" } predicates = { version = "3.0.4" } +regex = { version = "1.10.3" } reqwest = { version = "0.11.23", features = ["blocking", "rustls"], default-features = false } [features] diff --git a/crates/puffin/src/commands/venv.rs b/crates/puffin/src/commands/venv.rs index 67c8ae01a..d00279918 100644 --- a/crates/puffin/src/commands/venv.rs +++ b/crates/puffin/src/commands/venv.rs @@ -3,10 +3,8 @@ use std::path::Path; use std::str::FromStr; use anyhow::Result; -use fs_err as fs; use miette::{Diagnostic, IntoDiagnostic}; use owo_colors::OwoColorize; -use puffin_installer::NoBinary; use thiserror::Error; use distribution_types::{DistributionMetadata, IndexLocations, Name}; @@ -15,7 +13,8 @@ use platform_host::Platform; use puffin_cache::Cache; use puffin_client::{FlatIndex, FlatIndexClient, RegistryClientBuilder}; use puffin_dispatch::BuildDispatch; -use puffin_interpreter::{find_requested_python, Interpreter}; +use puffin_installer::NoBinary; +use puffin_interpreter::{find_default_python, find_requested_python, Interpreter}; use puffin_resolver::InMemoryIndex; use puffin_traits::{BuildContext, InFlight, SetupPyStrategy}; @@ -46,29 +45,25 @@ pub(crate) async fn venv( #[derive(Error, Debug, Diagnostic)] enum VenvError { - #[error("Unable to find a Python interpreter")] - #[diagnostic(code(puffin::venv::python_not_found))] - PythonNotFound, - #[error("Failed to extract Python interpreter info")] #[diagnostic(code(puffin::venv::interpreter))] - InterpreterError(#[source] puffin_interpreter::Error), + Interpreter(#[source] puffin_interpreter::Error), #[error("Failed to create virtual environment")] #[diagnostic(code(puffin::venv::creation))] - CreationError(#[source] gourgeist::Error), + Creation(#[source] gourgeist::Error), #[error("Failed to install seed packages")] #[diagnostic(code(puffin::venv::seed))] - SeedError(#[source] anyhow::Error), + Seed(#[source] anyhow::Error), #[error("Failed to extract interpreter tags")] #[diagnostic(code(puffin::venv::tags))] - TagsError(#[source] platform_tags::TagsError), + Tags(#[source] platform_tags::TagsError), #[error("Failed to resolve `--find-links` entry")] #[diagnostic(code(puffin::venv::flat_index))] - FlatIndexError(#[source] puffin_client::FlatIndexError), + FlatIndex(#[source] puffin_client::FlatIndexError), } /// Create a virtual environment. @@ -84,17 +79,12 @@ async fn venv_impl( let base_python = if let Some(python_request) = python_request { find_requested_python(python_request).into_diagnostic()? } else { - fs::canonicalize( - which::which_global("python3") - .or_else(|_| which::which_global("python")) - .map_err(|_| VenvError::PythonNotFound)?, - ) - .into_diagnostic()? + find_default_python().into_diagnostic()? }; let platform = Platform::current().into_diagnostic()?; let interpreter = - Interpreter::query(&base_python, &platform, cache).map_err(VenvError::InterpreterError)?; + Interpreter::query(&base_python, &platform, cache).map_err(VenvError::Interpreter)?; writeln!( printer, @@ -112,7 +102,7 @@ async fn venv_impl( .into_diagnostic()?; // Create the virtual environment. - let venv = gourgeist::create_venv(path, interpreter).map_err(VenvError::CreationError)?; + let venv = gourgeist::create_venv(path, interpreter).map_err(VenvError::Creation)?; // Install seed packages. if seed { @@ -124,12 +114,12 @@ async fn venv_impl( // Resolve the flat indexes from `--find-links`. let flat_index = { - let tags = interpreter.tags().map_err(VenvError::TagsError)?; + let tags = interpreter.tags().map_err(VenvError::Tags)?; let client = FlatIndexClient::new(&client, cache); let entries = client .fetch(index_locations.flat_indexes()) .await - .map_err(VenvError::FlatIndexError)?; + .map_err(VenvError::FlatIndex)?; FlatIndex::from_entries(entries, tags) }; @@ -162,13 +152,13 @@ async fn venv_impl( Requirement::from_str("setuptools").unwrap(), ]) .await - .map_err(VenvError::SeedError)?; + .map_err(VenvError::Seed)?; // Install into the environment. build_dispatch .install(&resolution, &venv) .await - .map_err(VenvError::SeedError)?; + .map_err(VenvError::Seed)?; for distribution in resolution.distributions() { writeln!( diff --git a/crates/puffin/tests/common/mod.rs b/crates/puffin/tests/common/mod.rs index a31ba4555..ca042f4d6 100644 --- a/crates/puffin/tests/common/mod.rs +++ b/crates/puffin/tests/common/mod.rs @@ -1,11 +1,12 @@ #![allow(dead_code)] +use std::path::{Path, PathBuf}; + use assert_cmd::Command; use assert_fs::assert::PathAssert; use assert_fs::fixture::PathChild; use assert_fs::TempDir; use insta_cmd::get_cargo_bin; -use std::path::PathBuf; pub(crate) const BIN_NAME: &str = "puffin"; @@ -13,11 +14,29 @@ pub(crate) const INSTA_FILTERS: &[(&str, &str)] = &[ (r"--cache-dir .*", "--cache-dir [CACHE_DIR]"), (r"(\d+\.)?\d+(ms|s)", "[TIME]"), (r"v\d+\.\d+\.\d+", "v[VERSION]"), + // Rewrite Windows output to Unix output + (r"\\([\w\d])", "/$1"), + (r"puffin.exe", "puffin"), + // The exact message is host language dependent + ( + r"Caused by: .* \(os error 2\)", + "Caused by: No such file or directory (os error 2)", + ), ]; +pub(crate) fn venv_to_interpreter(venv: &Path) -> PathBuf { + if cfg!(unix) { + venv.join("bin").join("python") + } else if cfg!(windows) { + venv.join("Scripts").join("python.exe") + } else { + unimplemented!("Only Windows and Unix are supported") + } +} + /// Create a virtual environment named `.venv` in a temporary directory. pub(crate) fn create_venv_py312(temp_dir: &TempDir, cache_dir: &TempDir) -> PathBuf { - create_venv(temp_dir, cache_dir, "python3.12") + create_venv(temp_dir, cache_dir, "3.12") } /// Create a virtual environment named `.venv` in a temporary directory with the given diff --git a/crates/puffin/tests/pip_compile.rs b/crates/puffin/tests/pip_compile.rs index 36dd1d098..fd98659ac 100644 --- a/crates/puffin/tests/pip_compile.rs +++ b/crates/puffin/tests/pip_compile.rs @@ -5,7 +5,6 @@ use std::process::Command; use std::{fs, iter}; use anyhow::{bail, Context, Result}; -use assert_cmd::prelude::*; use assert_fs::prelude::*; use assert_fs::TempDir; use indoc::indoc; @@ -13,7 +12,9 @@ use insta::assert_snapshot; use insta_cmd::_macro_support::insta; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; use itertools::Itertools; +use url::Url; +use crate::common::create_venv; use common::{create_venv_py312, BIN_NAME, INSTA_FILTERS}; mod common; @@ -69,13 +70,16 @@ fn missing_requirements_in() -> Result<()> { let cache_dir = TempDir::new()?; let requirements_in = temp_dir.child("requirements.in"); - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") - .arg("compile") - .arg("requirements.in") - .arg("--cache-dir") - .arg(cache_dir.path()) - .current_dir(&temp_dir), @r###" + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") + .arg("compile") + .arg("requirements.in") + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -84,6 +88,8 @@ fn missing_requirements_in() -> Result<()> { error: failed to open file `requirements.in` Caused by: No such file or directory (os error 2) "###); + } + ); requirements_in.assert(predicates::path::missing()); @@ -96,14 +102,17 @@ fn missing_venv() -> Result<()> { let cache_dir = TempDir::new()?; let venv = temp_dir.child(".venv"); - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") - .arg("compile") - .arg("requirements.in") - .arg("--cache-dir") - .arg(cache_dir.path()) - .env("VIRTUAL_ENV", venv.as_os_str()) - .current_dir(&temp_dir), @r###" + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") + .arg("compile") + .arg("requirements.in") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -112,6 +121,7 @@ fn missing_venv() -> Result<()> { error: failed to open file `requirements.in` Caused by: No such file or directory (os error 2) "###); + }); venv.assert(predicates::path::missing()); @@ -768,20 +778,7 @@ fn compile_python_dev_version() -> Result<()> { fn compile_numpy_py38() -> Result<()> { let temp_dir = TempDir::new()?; let cache_dir = TempDir::new()?; - let venv = temp_dir.child(".venv"); - - Command::new(get_cargo_bin(BIN_NAME)) - .arg("venv") - .arg(venv.as_os_str()) - .arg("--cache-dir") - .arg(cache_dir.path()) - .arg("--python") - .arg("python3.8") - .current_dir(&temp_dir) - .assert() - .success(); - venv.assert(predicates::path::is_dir()); - let venv = venv.to_path_buf(); + let venv = create_venv(&temp_dir, &cache_dir, "3.8"); let requirements_in = temp_dir.child("requirements.in"); requirements_in.write_str("numpy")?; @@ -2064,7 +2061,10 @@ fn compile_wheel_path_dependency() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut flask_wheel_file)?; let requirements_in = temp_dir.child("requirements.in"); - requirements_in.write_str(&format!("flask @ file://{}", flask_wheel.path().display()))?; + requirements_in.write_str(&format!( + "flask @ {}", + Url::from_file_path(flask_wheel.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -2256,7 +2256,10 @@ fn compile_source_distribution_path_dependency() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut flask_wheel_file)?; let requirements_in = temp_dir.child("requirements.in"); - requirements_in.write_str(&format!("flask @ file://{}", flask_wheel.path().display()))?; + requirements_in.write_str(&format!( + "flask @ {}", + Url::from_file_path(flask_wheel.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -2767,7 +2770,7 @@ fn compile_editable() -> Result<()> { " })?; - let filter_path = requirements_in.display().to_string(); + let filter_path = regex::escape(&requirements_in.display().to_string()); let filters: Vec<_> = iter::once((filter_path.as_str(), "requirements.in")) .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -3246,7 +3249,7 @@ fn find_links_directory() -> Result<()> { "})?; let project_root = fs_err::canonicalize(std::env::current_dir()?.join("../.."))?; - let project_root_string = project_root.display().to_string(); + let project_root_string = regex::escape(&project_root.display().to_string()); let filters: Vec<_> = iter::once((project_root_string.as_str(), "[PROJECT_ROOT]")) .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -3560,8 +3563,13 @@ fn missing_path_requirement() -> Result<()> { let requirements_in = temp_dir.child("requirements.in"); requirements_in.write_str("django @ file:///tmp/django-3.2.8.tar.gz")?; + let filters: Vec<_> = [(r"/[A-Z]:/", "/")] + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect(); + insta::with_settings!({ - filters => INSTA_FILTERS.to_vec() + filters => filters }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .arg("pip") @@ -3595,9 +3603,15 @@ fn missing_editable_requirement() -> Result<()> { let requirements_in = temp_dir.child("requirements.in"); requirements_in.write_str("-e ../tmp/django-3.2.8.tar.gz")?; - let filters: Vec<_> = iter::once((r"(file:/)?/.*/", "file://[TEMP_DIR]/")) - .chain(INSTA_FILTERS.to_vec()) - .collect::>(); + // File url, absolute Unix path or absolute Windows path + let filters: Vec<_> = [ + (r" file://.*/", " file://[TEMP_DIR]/"), + (r" /.*/", " /[TEMP_DIR]/"), + (r" [A-Z]:\\.*\\", " /[TEMP_DIR]/"), + ] + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect::>(); insta::with_settings!({ filters => filters @@ -3619,7 +3633,7 @@ fn missing_editable_requirement() -> Result<()> { ----- stderr ----- error: Failed to build editables Caused by: Failed to build editable: file://[TEMP_DIR]/django-3.2.8.tar.gz - Caused by: Source distribution not found at: file://[TEMP_DIR]/django-3.2.8.tar.gz + Caused by: Source distribution not found at: /[TEMP_DIR]/django-3.2.8.tar.gz "###); }); diff --git a/crates/puffin/tests/pip_install.rs b/crates/puffin/tests/pip_install.rs index a23053d4e..d74ec3cca 100644 --- a/crates/puffin/tests/pip_install.rs +++ b/crates/puffin/tests/pip_install.rs @@ -12,7 +12,7 @@ use indoc::indoc; use insta_cmd::_macro_support::insta; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; -use common::{create_venv_py312, BIN_NAME, INSTA_FILTERS}; +use common::{create_venv_py312, venv_to_interpreter, BIN_NAME, INSTA_FILTERS}; mod common; @@ -20,7 +20,9 @@ mod common; static EXCLUDE_NEWER: &str = "2023-11-18T12:00:00Z"; fn assert_command(venv: &Path, command: &str, temp_dir: &Path) -> Assert { - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(venv)) + // https://github.com/python/cpython/issues/75953 + .arg("-B") .arg("-c") .arg(command) .current_dir(temp_dir) @@ -33,15 +35,18 @@ fn missing_requirements_txt() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let requirements_txt = temp_dir.child("requirements.txt"); - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") - .arg("install") - .arg("-r") - .arg("requirements.txt") - .arg("--strict") - .arg("--cache-dir") - .arg(cache_dir.path()) - .current_dir(&temp_dir), @r###" + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") + .arg("install") + .arg("-r") + .arg("requirements.txt") + .arg("--strict") + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -50,6 +55,7 @@ fn missing_requirements_txt() -> Result<()> { error: failed to open file `requirements.txt` Caused by: No such file or directory (os error 2) "###); + }); requirements_txt.assert(predicates::path::missing()); @@ -458,9 +464,16 @@ fn install_editable() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); let current_dir = std::env::current_dir()?; - let workspace_dir = current_dir.join("..").join("..").canonicalize()?; + let workspace_dir = regex::escape( + current_dir + .join("..") + .join("..") + .canonicalize()? + .to_str() + .unwrap(), + ); - let filters = iter::once((workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]")) + let filters = iter::once((workspace_dir.as_str(), "[WORKSPACE_DIR]")) .chain(INSTA_FILTERS.to_vec()) .collect::>(); @@ -566,9 +579,16 @@ fn install_editable_and_registry() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); let current_dir = std::env::current_dir()?; - let workspace_dir = current_dir.join("..").join("..").canonicalize()?; + let workspace_dir = regex::escape( + current_dir + .join("..") + .join("..") + .canonicalize()? + .to_str() + .unwrap(), + ); - let filters: Vec<_> = iter::once((workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]")) + let filters: Vec<_> = iter::once((workspace_dir.as_str(), "[WORKSPACE_DIR]")) .chain(INSTA_FILTERS.to_vec()) .collect(); diff --git a/crates/puffin/tests/pip_install_scenarios.rs b/crates/puffin/tests/pip_install_scenarios.rs index d7bdfe459..b6084171a 100644 --- a/crates/puffin/tests/pip_install_scenarios.rs +++ b/crates/puffin/tests/pip_install_scenarios.rs @@ -14,12 +14,12 @@ use assert_cmd::prelude::*; use insta_cmd::_macro_support::insta; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; -use common::{create_venv, BIN_NAME, INSTA_FILTERS}; +use common::{create_venv, venv_to_interpreter, BIN_NAME, INSTA_FILTERS}; mod common; fn assert_command(venv: &Path, command: &str, temp_dir: &Path) -> Assert { - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(venv)) .arg("-c") .arg(command) .current_dir(temp_dir) diff --git a/crates/puffin/tests/pip_sync.rs b/crates/puffin/tests/pip_sync.rs index a7e39bede..2a44195ee 100644 --- a/crates/puffin/tests/pip_sync.rs +++ b/crates/puffin/tests/pip_sync.rs @@ -4,19 +4,20 @@ use std::path::Path; use std::process::Command; use std::{fs, iter}; -use anyhow::{Context, Result}; +use anyhow::Result; use assert_cmd::prelude::*; use assert_fs::prelude::*; use indoc::indoc; use insta_cmd::_macro_support::insta; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; +use url::Url; -use common::{create_venv_py312, BIN_NAME, INSTA_FILTERS}; +use common::{create_venv_py312, venv_to_interpreter, BIN_NAME, INSTA_FILTERS}; mod common; fn check_command(venv: &Path, command: &str, temp_dir: &Path) { - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(venv)) // https://github.com/python/cpython/issues/75953 .arg("-B") .arg("-c") @@ -32,14 +33,17 @@ fn missing_requirements_txt() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let requirements_txt = temp_dir.child("requirements.txt"); - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") - .arg("sync") - .arg("requirements.txt") - .arg("--strict") - .arg("--cache-dir") - .arg(cache_dir.path()) - .current_dir(&temp_dir), @r###" + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") + .arg("sync") + .arg("requirements.txt") + .arg("--strict") + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -48,6 +52,7 @@ fn missing_requirements_txt() -> Result<()> { error: failed to open file `requirements.txt` Caused by: No such file or directory (os error 2) "###); + }); requirements_txt.assert(predicates::path::missing()); @@ -60,15 +65,18 @@ fn missing_venv() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") - .arg("sync") - .arg("requirements.txt") - .arg("--strict") - .arg("--cache-dir") - .arg(cache_dir.path()) - .env("VIRTUAL_ENV", venv.as_os_str()) - .current_dir(&temp_dir), @r###" + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") + .arg("sync") + .arg("requirements.txt") + .arg("--strict") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -77,6 +85,7 @@ fn missing_venv() -> Result<()> { error: failed to open file `requirements.txt` Caused by: No such file or directory (os error 2) "###); + }); venv.assert(predicates::path::missing()); @@ -344,7 +353,7 @@ fn link() -> Result<()> { .arg("--cache-dir") .arg(cache_dir.path()) .arg("--python") - .arg("python3.12") + .arg("3.12") .current_dir(&temp_dir) .assert() .success(); @@ -433,7 +442,7 @@ fn add_remove() -> Result<()> { check_command(&venv, "import tomli", &temp_dir); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import markupsafe") .current_dir(&temp_dir) @@ -998,8 +1007,7 @@ fn install_numpy_py38() -> Result<()> { .arg("venv") .arg(venv.as_os_str()) .arg("--python") - // TODO(konstin): Mock the venv in the installer test so we don't need this anymore - .arg(which::which("python3.8").context("python3.8 must be installed")?) + .arg("3.8") .arg("--cache-dir") .arg(cache_dir.path()) .current_dir(&temp_dir) @@ -1135,7 +1143,10 @@ fn install_local_wheel() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut archive_file)?; let requirements_txt = temp_dir.child("requirements.txt"); - requirements_txt.write_str(&format!("tomli @ file://{}", archive.path().display()))?; + requirements_txt.write_str(&format!( + "tomli @ {}", + Url::from_file_path(archive.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -1200,8 +1211,8 @@ fn install_local_wheel() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); // "Modify" the wheel. - let archive_file = std::fs::File::open(&archive)?; - archive_file.set_modified(std::time::SystemTime::now())?; + // The `filetime` crate works on Windows unlike the std. + filetime::set_file_mtime(&archive, filetime::FileTime::now()).unwrap(); // Reinstall. The wheel should be "downloaded" again. insta::with_settings!({ @@ -1231,8 +1242,7 @@ fn install_local_wheel() -> Result<()> { check_command(&venv, "import tomli", &temp_dir); // "Modify" the wheel. - let archive_file = std::fs::File::open(&archive)?; - archive_file.set_modified(std::time::SystemTime::now())?; + filetime::set_file_mtime(&archive, filetime::FileTime::now()).unwrap(); // Reinstall into the same virtual environment. The wheel should be reinstalled. insta::with_settings!({ @@ -1280,7 +1290,10 @@ fn mismatched_version() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut archive_file)?; let requirements_txt = temp_dir.child("requirements.txt"); - requirements_txt.write_str(&format!("tomli @ file://{}", archive.path().display()))?; + requirements_txt.write_str(&format!( + "tomli @ {}", + Url::from_file_path(archive.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -1328,7 +1341,10 @@ fn mismatched_name() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut archive_file)?; let requirements_txt = temp_dir.child("requirements.txt"); - requirements_txt.write_str(&format!("tomli @ file://{}", archive.path().display()))?; + requirements_txt.write_str(&format!( + "tomli @ {}", + Url::from_file_path(archive.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -1376,7 +1392,10 @@ fn install_local_source_distribution() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut archive_file)?; let requirements_txt = temp_dir.child("requirements.txt"); - requirements_txt.write_str(&format!("wheel @ file://{}", archive.path().display()))?; + requirements_txt.write_str(&format!( + "wheel @ {}", + Url::from_file_path(archive.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -1877,7 +1896,10 @@ fn install_path_source_dist_cached() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut archive_file)?; let requirements_txt = temp_dir.child("requirements.txt"); - requirements_txt.write_str(&format!("wheel @ file://{}", archive.path().display()))?; + requirements_txt.write_str(&format!( + "wheel @ {}", + Url::from_file_path(archive.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -2004,7 +2026,10 @@ fn install_path_built_dist_cached() -> Result<()> { std::io::copy(&mut response.bytes()?.as_ref(), &mut archive_file)?; let requirements_txt = temp_dir.child("requirements.txt"); - requirements_txt.write_str(&format!("tomli @ file://{}", archive.path().display()))?; + requirements_txt.write_str(&format!( + "tomli @ {}", + Url::from_file_path(archive.path()).unwrap() + ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) @@ -2684,7 +2709,14 @@ fn sync_editable() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); let current_dir = std::env::current_dir()?; - let workspace_dir = current_dir.join("..").join("..").canonicalize()?; + let workspace_dir = regex::escape( + current_dir + .join("..") + .join("..") + .canonicalize()? + .to_str() + .unwrap(), + ); let requirements_txt = temp_dir.child("requirements.txt"); requirements_txt.write_str(&indoc::formatdoc! {r" @@ -2697,7 +2729,7 @@ fn sync_editable() -> Result<()> { current_dir = current_dir.display(), })?; - let filter_path = requirements_txt.display().to_string(); + let filter_path = regex::escape(&requirements_txt.display().to_string()); let filters = INSTA_FILTERS .iter() .chain(&[ @@ -2706,7 +2738,7 @@ fn sync_editable() -> Result<()> { r"file://.*/../../scripts/editable-installs/poetry_editable", "file://[TEMP_DIR]/../../scripts/editable-installs/poetry_editable", ), - (workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]"), + (&workspace_dir, "[WORKSPACE_DIR]"), ]) .copied() .collect::>(); @@ -2837,7 +2869,14 @@ fn sync_editable_and_registry() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); let current_dir = std::env::current_dir()?; - let workspace_dir = current_dir.join("..").join("..").canonicalize()?; + let workspace_dir = regex::escape( + current_dir + .join("..") + .join("..") + .canonicalize()? + .to_str() + .unwrap(), + ); // Install the registry-based version of Black. let requirements_txt = temp_dir.child("requirements.txt"); @@ -2846,12 +2885,12 @@ fn sync_editable_and_registry() -> Result<()> { " })?; - let filter_path = requirements_txt.display().to_string(); + let filter_path = regex::escape(&requirements_txt.display().to_string()); let filters = INSTA_FILTERS .iter() .chain(&[ (filter_path.as_str(), "requirements.txt"), - (workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]"), + (workspace_dir.as_str(), "[WORKSPACE_DIR]"), ]) .copied() .collect::>(); @@ -2898,7 +2937,7 @@ fn sync_editable_and_registry() -> Result<()> { .iter() .chain(&[ (filter_path.as_str(), "requirements.txt"), - (workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]"), + (workspace_dir.as_str(), "[WORKSPACE_DIR]"), ]) .copied() .collect::>(); @@ -2940,7 +2979,7 @@ fn sync_editable_and_registry() -> Result<()> { .iter() .chain(&[ (filter_path.as_str(), "requirements.txt"), - (workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]"), + (workspace_dir.as_str(), "[WORKSPACE_DIR]"), ]) .copied() .collect::>(); @@ -2977,7 +3016,7 @@ fn sync_editable_and_registry() -> Result<()> { .iter() .chain(&[ (filter_path.as_str(), "requirements.txt"), - (workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]"), + (workspace_dir.as_str(), "[WORKSPACE_DIR]"), ]) .copied() .collect::>(); @@ -3026,9 +3065,12 @@ fn incompatible_wheel() -> Result<()> { wheel.touch()?; let requirements_txt = temp_dir.child("requirements.txt"); - requirements_txt.write_str(&format!("foo @ file://{}", wheel.path().display()))?; + requirements_txt.write_str(&format!( + "foo @ {}", + Url::from_file_path(wheel.path()).unwrap() + ))?; - let wheel_dir = wheel_dir.path().canonicalize()?.display().to_string(); + let wheel_dir = regex::escape(&wheel_dir.path().canonicalize()?.display().to_string()); let filters: Vec<_> = iter::once((wheel_dir.as_str(), "[TEMP_DIR]")) .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -3147,7 +3189,7 @@ fn find_links() -> Result<()> { "})?; let project_root = fs_err::canonicalize(std::env::current_dir()?.join("../.."))?; - let project_root_string = project_root.display().to_string(); + let project_root_string = regex::escape(&project_root.display().to_string()); let filters: Vec<_> = iter::once((project_root_string.as_str(), "[PROJECT_ROOT]")) .chain(INSTA_FILTERS.to_vec()) .collect(); diff --git a/crates/puffin/tests/pip_uninstall.rs b/crates/puffin/tests/pip_uninstall.rs index a9dcb66e2..792f0442d 100644 --- a/crates/puffin/tests/pip_uninstall.rs +++ b/crates/puffin/tests/pip_uninstall.rs @@ -5,10 +5,11 @@ use anyhow::Result; use assert_cmd::prelude::*; use assert_fs::prelude::*; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; +use url::Url; use common::{BIN_NAME, INSTA_FILTERS}; -use crate::common::create_venv_py312; +use crate::common::{create_venv_py312, venv_to_interpreter}; mod common; @@ -16,10 +17,13 @@ mod common; fn no_arguments() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?; - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") .arg("uninstall") - .current_dir(&temp_dir), @r###" + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -32,6 +36,7 @@ fn no_arguments() -> Result<()> { For more information, try '--help'. "###); + }); Ok(()) } @@ -63,12 +68,15 @@ fn invalid_requirement() -> Result<()> { fn missing_requirements_txt() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?; - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") .arg("uninstall") - .arg("-r") - .arg("requirements.txt") - .current_dir(&temp_dir), @r###" + .arg("-r") + .arg("requirements.txt") + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -77,6 +85,7 @@ fn missing_requirements_txt() -> Result<()> { error: failed to open file `requirements.txt` Caused by: No such file or directory (os error 2) "###); + }); Ok(()) } @@ -112,12 +121,15 @@ fn invalid_requirements_txt_requirement() -> Result<()> { fn missing_pyproject_toml() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?; - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .arg("pip") + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") .arg("uninstall") - .arg("-r") - .arg("pyproject.toml") - .current_dir(&temp_dir), @r###" + .arg("-r") + .arg("pyproject.toml") + .current_dir(&temp_dir), @r###" success: false exit_code: 2 ----- stdout ----- @@ -126,6 +138,7 @@ fn missing_pyproject_toml() -> Result<()> { error: failed to open file `pyproject.toml` Caused by: No such file or directory (os error 2) "###); + }); Ok(()) } @@ -248,7 +261,7 @@ fn uninstall() -> Result<()> { .assert() .success(); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import markupsafe") .current_dir(&temp_dir) @@ -276,7 +289,7 @@ fn uninstall() -> Result<()> { "###); }); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import markupsafe") .current_dir(&temp_dir) @@ -307,7 +320,7 @@ fn missing_record() -> Result<()> { .assert() .success(); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import markupsafe") .current_dir(&temp_dir) @@ -315,24 +328,38 @@ fn missing_record() -> Result<()> { .success(); // Delete the RECORD file. - let dist_info = venv - .join("lib") - .join("python3.12") - .join("site-packages") - .join("MarkupSafe-2.1.3.dist-info"); + let dist_info = fs_err::canonicalize(if cfg!(unix) { + venv.join("lib") + .join("python3.12") + .join("site-packages") + .join("MarkupSafe-2.1.3.dist-info") + } else if cfg!(windows) { + venv.join("Lib") + .join("site-packages") + .join("MarkupSafe-2.1.3.dist-info") + } else { + unimplemented!("Only Windows and Unix are supported") + }) + .unwrap(); std::fs::remove_file(dist_info.join("RECORD"))?; + let dist_info_str = regex::escape(&format!( + "RECORD file not found at: {}", + dist_info.display() + )); let filters: Vec<_> = iter::once(( - "RECORD file not found at: .*/.venv", - "RECORD file not found at: [VENV_PATH]", + dist_info_str.as_str(), + "RECORD file not found at: [DIST_INFO]", )) .chain(INSTA_FILTERS.to_vec()) .collect(); - insta::with_settings!({ - filters => filters, - }, { - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + insta::with_settings!( + { + filters => filters, + }, + { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .arg("pip") .arg("uninstall") .arg("MarkupSafe") @@ -345,9 +372,10 @@ fn missing_record() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: Cannot uninstall package; RECORD file not found at: [VENV_PATH]/lib/python3.12/site-packages/MarkupSafe-2.1.3.dist-info/RECORD + error: Cannot uninstall package; RECORD file not found at: [DIST_INFO]/RECORD "###); - }); + } + ); Ok(()) } @@ -359,9 +387,13 @@ fn uninstall_editable_by_name() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); let current_dir = std::env::current_dir()?; - let workspace_dir = current_dir.join("..").join("..").canonicalize()?; + let workspace_dir = regex::escape( + Url::from_directory_path(current_dir.join("..").join("..").canonicalize()?) + .unwrap() + .as_str(), + ); - let filters: Vec<_> = iter::once((workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]")) + let filters: Vec<_> = iter::once((workspace_dir.as_str(), "file://[WORKSPACE_DIR]/")) .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -379,7 +411,7 @@ fn uninstall_editable_by_name() -> Result<()> { .assert() .success(); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import poetry_editable") .assert() @@ -407,7 +439,7 @@ fn uninstall_editable_by_name() -> Result<()> { "###); }); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import poetry_editable") .assert() @@ -423,9 +455,13 @@ fn uninstall_editable_by_path() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); let current_dir = std::env::current_dir()?; - let workspace_dir = current_dir.join("..").join("..").canonicalize()?; + let workspace_dir = regex::escape( + Url::from_directory_path(current_dir.join("..").join("..").canonicalize()?) + .unwrap() + .as_str(), + ); - let filters: Vec<_> = iter::once((workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]")) + let filters: Vec<_> = iter::once((workspace_dir.as_str(), "file://[WORKSPACE_DIR]/")) .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -443,7 +479,7 @@ fn uninstall_editable_by_path() -> Result<()> { .assert() .success(); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import poetry_editable") .assert() @@ -471,7 +507,7 @@ fn uninstall_editable_by_path() -> Result<()> { "###); }); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import poetry_editable") .assert() @@ -487,9 +523,13 @@ fn uninstall_duplicate_editable() -> Result<()> { let venv = create_venv_py312(&temp_dir, &cache_dir); let current_dir = std::env::current_dir()?; - let workspace_dir = current_dir.join("..").join("..").canonicalize()?; + let workspace_dir = regex::escape( + Url::from_directory_path(current_dir.join("..").join("..").canonicalize()?) + .unwrap() + .as_str(), + ); - let filters: Vec<_> = iter::once((workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]")) + let filters: Vec<_> = iter::once((workspace_dir.as_str(), "file://[WORKSPACE_DIR]/")) .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -507,7 +547,7 @@ fn uninstall_duplicate_editable() -> Result<()> { .assert() .success(); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import poetry_editable") .assert() @@ -536,7 +576,7 @@ fn uninstall_duplicate_editable() -> Result<()> { "###); }); - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(&venv)) .arg("-c") .arg("import poetry_editable") .assert() diff --git a/crates/puffin/tests/venv.rs b/crates/puffin/tests/venv.rs index d5090a21a..d22529e00 100644 --- a/crates/puffin/tests/venv.rs +++ b/crates/puffin/tests/venv.rs @@ -16,17 +16,18 @@ fn create_venv() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); + let filter_venv = regex::escape(&venv.display().to_string()); insta::with_settings!({ filters => vec![ (r"Using Python 3\.\d+\.\d+ interpreter at .+", "Using Python [VERSION] interpreter at [PATH]"), - (temp_dir.to_str().unwrap(), "/home/ferris/project"), + (&filter_venv, "/home/ferris/project/.venv"), ] }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .arg("venv") .arg(venv.as_os_str()) .arg("--python") - .arg("python3.12") + .arg("3.12") .current_dir(&temp_dir), @r###" success: true exit_code: 0 @@ -48,16 +49,17 @@ fn create_venv_defaults_to_cwd() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); + let filter_venv = regex::escape(&venv.display().to_string()); insta::with_settings!({ filters => vec![ (r"Using Python 3\.\d+\.\d+ interpreter at .+", "Using Python [VERSION] interpreter at [PATH]"), - (temp_dir.to_str().unwrap(), "/home/ferris/project"), + (&filter_venv, "/home/ferris/project/.venv"), ] }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .arg("venv") .arg("--python") - .arg("python3.12") + .arg("3.12") .current_dir(&temp_dir), @r###" success: true exit_code: 0 @@ -79,10 +81,11 @@ fn seed() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); + let filter_venv = regex::escape(&venv.display().to_string()); insta::with_settings!({ filters => vec![ (r"Using Python 3\.\d+\.\d+ interpreter at .+", "Using Python [VERSION] interpreter at [PATH]"), - (temp_dir.to_str().unwrap(), "/home/ferris/project"), + (&filter_venv, "/home/ferris/project/.venv"), ] }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) @@ -90,7 +93,7 @@ fn seed() -> Result<()> { .arg(venv.as_os_str()) .arg("--seed") .arg("--python") - .arg("python3.12") + .arg("3.12") .current_dir(&temp_dir), @r###" success: true exit_code: 0 diff --git a/crates/requirements-txt/src/lib.rs b/crates/requirements-txt/src/lib.rs index 6afbd1b33..12e33410f 100644 --- a/crates/requirements-txt/src/lib.rs +++ b/crates/requirements-txt/src/lib.rs @@ -744,7 +744,8 @@ mod test { let err = RequirementsTxt::parse(basic, &working_dir).unwrap_err(); let errors = anyhow::Error::new(err) .chain() - .map(ToString::to_string) + // Windows support + .map(|err| err.to_string().replace('\\', "/")) .collect::>(); let expected = &[ "Unsupported URL (expected a `file://` scheme) in `./test-data/requirements-txt/unsupported-editable.txt`: http://localhost:8080/".to_string() diff --git a/scripts/scenarios/templates/install.mustache b/scripts/scenarios/templates/install.mustache index b51ca02b2..6217d8eec 100644 --- a/scripts/scenarios/templates/install.mustache +++ b/scripts/scenarios/templates/install.mustache @@ -14,30 +14,22 @@ use assert_cmd::prelude::*; use insta_cmd::_macro_support::insta; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; -use common::{create_venv, BIN_NAME, INSTA_FILTERS}; +use common::{create_venv, BIN_NAME, INSTA_FILTERS, venv_to_interpreter}; mod common; fn assert_command(venv: &Path, command: &str, temp_dir: &Path) -> Assert { - Command::new(venv.join("bin").join("python")) + Command::new(venv_to_interpreter(venv)) .arg("-c") .arg(command) .current_dir(temp_dir) .assert() } -fn assert_installed( - venv: &Path, - package: &'static str, - version: &'static str, - temp_dir: &Path, -) { +fn assert_installed(venv: &Path, package: &'static str, version: &'static str, temp_dir: &Path) { assert_command( venv, - format!( - "import {package} as package; print(package.__version__, end='')" - ) - .as_str(), + format!("import {package} as package; print(package.__version__, end='')").as_str(), temp_dir, ) .success()