From 1c0e03f807dd73a9d7c340be922d5d13bfb283f4 Mon Sep 17 00:00:00 2001 From: konsti Date: Thu, 23 Nov 2023 09:57:33 +0100 Subject: [PATCH] puffin_interpreter cleanup ahead of #235 (#492) Preparing for #235, some refactoring to `puffin_interpreter`. * Added a dedicated error type instead of anyhow * `InterpreterInfo` -> `Interpreter` * `detect_virtual_env` now returns an option so it can be chained for #235 --- crates/gourgeist/src/bare.rs | 29 +++--- crates/gourgeist/src/lib.rs | 11 ++- crates/gourgeist/src/main.rs | 4 +- crates/puffin-build/src/lib.rs | 6 +- crates/puffin-cli/src/commands/pip_compile.rs | 12 +-- crates/puffin-cli/src/commands/pip_sync.rs | 11 +-- crates/puffin-cli/src/commands/venv.rs | 11 ++- crates/puffin-dev/src/build.rs | 2 +- crates/puffin-dev/src/resolve_cli.rs | 8 +- crates/puffin-dev/src/resolve_many.rs | 2 +- crates/puffin-dispatch/src/lib.rs | 26 +++--- crates/puffin-installer/src/installer.rs | 2 +- .../{interpreter_info.rs => interpreter.rs} | 91 ++++++++----------- crates/puffin-interpreter/src/lib.rs | 42 ++++++++- crates/puffin-interpreter/src/virtual_env.rs | 77 +++++++++------- crates/puffin-resolver/src/finder.rs | 14 +-- crates/puffin-resolver/src/resolver.rs | 2 +- crates/puffin-resolver/tests/resolver.rs | 10 +- crates/puffin-traits/src/lib.rs | 4 +- 19 files changed, 201 insertions(+), 163 deletions(-) rename crates/puffin-interpreter/src/{interpreter_info.rs => interpreter.rs} (71%) diff --git a/crates/gourgeist/src/bare.rs b/crates/gourgeist/src/bare.rs index 2906fe4d6..5c4b0da7a 100644 --- a/crates/gourgeist/src/bare.rs +++ b/crates/gourgeist/src/bare.rs @@ -10,7 +10,7 @@ use fs_err::os::unix::fs::symlink; use fs_err::File; use tracing::info; -use puffin_interpreter::InterpreterInfo; +use puffin_interpreter::Interpreter; /// The bash activate scripts with the venv dependent paths patches out const ACTIVATE_TEMPLATES: &[(&str, &str)] = &[ @@ -56,8 +56,8 @@ pub struct VenvPaths { } /// Write all the files that belong to a venv without any packages installed. -pub fn create_bare_venv(location: &Utf8Path, info: &InterpreterInfo) -> io::Result { - let base_python: &Utf8Path = info +pub fn create_bare_venv(location: &Utf8Path, interpreter: &Interpreter) -> io::Result { + let base_python: &Utf8Path = interpreter .sys_executable() .try_into() .map_err(|err: FromPathError| err.into_io_error())?; @@ -113,14 +113,14 @@ pub fn create_bare_venv(location: &Utf8Path, info: &InterpreterInfo) -> io::Resu symlink(base_python, &venv_python)?; symlink( "python", - bin_dir.join(format!("python{}", info.simple_version().0)), + bin_dir.join(format!("python{}", interpreter.simple_version().0)), )?; symlink( "python", bin_dir.join(format!( "python{}.{}", - info.simple_version().0, - info.simple_version().1 + interpreter.simple_version().0, + interpreter.simple_version().1 )), )?; } @@ -133,8 +133,8 @@ pub fn create_bare_venv(location: &Utf8Path, info: &InterpreterInfo) -> io::Resu "{{ RELATIVE_SITE_PACKAGES }}", &format!( "../lib/python{}.{}/site-packages", - info.simple_version().0, - info.simple_version().1 + interpreter.simple_version().0, + interpreter.simple_version().1 ), ); fs::write(bin_dir.join(name), activator)?; @@ -153,17 +153,20 @@ pub fn create_bare_venv(location: &Utf8Path, info: &InterpreterInfo) -> io::Resu let pyvenv_cfg_data = &[ ("home", python_home), ("implementation", "CPython".to_string()), - ("version_info", info.markers().python_version.string.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", - info.base_prefix().to_string_lossy().to_string(), + interpreter.base_prefix().to_string_lossy().to_string(), ), ( "base-exec-prefix", - info.base_exec_prefix().to_string_lossy().to_string(), + interpreter.base_exec_prefix().to_string_lossy().to_string(), ), ("base-executable", base_python.to_string()), ]; @@ -176,8 +179,8 @@ pub fn create_bare_venv(location: &Utf8Path, info: &InterpreterInfo) -> io::Resu .join("lib") .join(format!( "python{}.{}", - info.simple_version().0, - info.simple_version().1 + interpreter.simple_version().0, + interpreter.simple_version().1 )) .join("site-packages"); fs::create_dir_all(&site_packages)?; diff --git a/crates/gourgeist/src/lib.rs b/crates/gourgeist/src/lib.rs index cb8e8c7f8..817a1d6ae 100644 --- a/crates/gourgeist/src/lib.rs +++ b/crates/gourgeist/src/lib.rs @@ -6,7 +6,7 @@ use thiserror::Error; pub use interpreter::parse_python_cli; use platform_host::PlatformError; -use puffin_interpreter::{InterpreterInfo, Virtualenv}; +use puffin_interpreter::{Interpreter, Virtualenv}; pub use crate::bare::create_bare_venv; @@ -24,10 +24,13 @@ pub enum Error { } /// Create a virtualenv. -pub fn create_venv(location: &Path, info: &InterpreterInfo) -> Result { +pub fn create_venv(location: &Path, interpreter: &Interpreter) -> Result { let location: &Utf8Path = location .try_into() .map_err(|err: FromPathError| err.into_io_error())?; - let paths = create_bare_venv(location, info)?; - Ok(Virtualenv::new_prefix(paths.root.as_std_path(), info)) + let paths = create_bare_venv(location, interpreter)?; + Ok(Virtualenv::new_prefix( + paths.root.as_std_path(), + interpreter, + )) } diff --git a/crates/gourgeist/src/main.rs b/crates/gourgeist/src/main.rs index 3022365f6..4205bcd6b 100644 --- a/crates/gourgeist/src/main.rs +++ b/crates/gourgeist/src/main.rs @@ -11,7 +11,7 @@ use tracing_subscriber::{fmt, EnvFilter}; use gourgeist::{create_bare_venv, parse_python_cli}; use platform_host::Platform; -use puffin_interpreter::InterpreterInfo; +use puffin_interpreter::Interpreter; #[derive(Parser, Debug)] struct Cli { @@ -25,7 +25,7 @@ fn run() -> Result<(), gourgeist::Error> { let location = cli.path.unwrap_or(Utf8PathBuf::from(".venv")); let python = parse_python_cli(cli.python)?; let platform = Platform::current()?; - let info = InterpreterInfo::query(python.as_std_path(), platform, None).unwrap(); + let info = Interpreter::query(python.as_std_path(), platform, None).unwrap(); create_bare_venv(&location, &info)?; Ok(()) } diff --git a/crates/puffin-build/src/lib.rs b/crates/puffin-build/src/lib.rs index 73eb52599..35eadb0cb 100644 --- a/crates/puffin-build/src/lib.rs +++ b/crates/puffin-build/src/lib.rs @@ -27,7 +27,7 @@ use tracing::{debug, instrument}; use zip::ZipArchive; use pep508_rs::Requirement; -use puffin_interpreter::{InterpreterInfo, Virtualenv}; +use puffin_interpreter::{Interpreter, Virtualenv}; use puffin_traits::BuildContext; /// e.g. `pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory` @@ -220,7 +220,7 @@ impl SourceBuild { pub async fn setup( source: &Path, subdirectory: Option<&Path>, - interpreter_info: &InterpreterInfo, + interpreter: &Interpreter, build_context: &impl BuildContext, source_build_context: SourceBuildContext, package_id: &str, @@ -250,7 +250,7 @@ impl SourceBuild { None }; - let venv = gourgeist::create_venv(&temp_dir.path().join(".venv"), interpreter_info)?; + let venv = gourgeist::create_venv(&temp_dir.path().join(".venv"), interpreter)?; // There are packages such as DTLSSocket 0.1.16 that say // ```toml diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index 399c7ba2c..49b7aa0d2 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -116,24 +116,24 @@ pub(crate) async fn pip_compile( debug!( "Using Python {} at {}", - venv.interpreter_info().markers().python_version, + venv.interpreter().markers().python_version, venv.python_executable().display() ); // Determine the compatible platform tags. let tags = Tags::from_env( - venv.interpreter_info().platform(), - venv.interpreter_info().simple_version(), + venv.interpreter().platform(), + venv.interpreter().simple_version(), )?; // Determine the markers to use for resolution. let markers = python_version.map_or_else( - || Cow::Borrowed(venv.interpreter_info().markers()), - |python_version| Cow::Owned(python_version.markers(venv.interpreter_info().markers())), + || Cow::Borrowed(venv.interpreter().markers()), + |python_version| Cow::Owned(python_version.markers(venv.interpreter().markers())), ); // Inject the fake python version if necessary let interpreter_info = venv - .interpreter_info() + .interpreter() .clone() .patch_markers(markers.clone().into_owned()); diff --git a/crates/puffin-cli/src/commands/pip_sync.rs b/crates/puffin-cli/src/commands/pip_sync.rs index 80fa18504..20edee451 100644 --- a/crates/puffin-cli/src/commands/pip_sync.rs +++ b/crates/puffin-cli/src/commands/pip_sync.rs @@ -105,8 +105,8 @@ pub(crate) async fn sync_requirements( // Determine the current environment markers. let tags = Tags::from_env( - venv.interpreter_info().platform(), - venv.interpreter_info().simple_version(), + venv.interpreter().platform(), + venv.interpreter().simple_version(), )?; // Instantiate a client. @@ -129,9 +129,8 @@ pub(crate) async fn sync_requirements( } else { let start = std::time::Instant::now(); - let wheel_finder = - puffin_resolver::DistFinder::new(&tags, &client, venv.interpreter_info()) - .with_reporter(FinderReporter::from(printer).with_length(remote.len() as u64)); + let wheel_finder = puffin_resolver::DistFinder::new(&tags, &client, venv.interpreter()) + .with_reporter(FinderReporter::from(printer).with_length(remote.len() as u64)); let resolution = wheel_finder.resolve(&remote).await?; let s = if resolution.len() == 1 { "" } else { "s" }; @@ -223,7 +222,7 @@ pub(crate) async fn sync_requirements( let build_dispatch = BuildDispatch::new( client, cache.to_path_buf(), - venv.interpreter_info().clone(), + venv.interpreter().clone(), fs::canonicalize(venv.python_executable())?, no_build, ); diff --git a/crates/puffin-cli/src/commands/venv.rs b/crates/puffin-cli/src/commands/venv.rs index 0c8647599..f77a382c8 100644 --- a/crates/puffin-cli/src/commands/venv.rs +++ b/crates/puffin-cli/src/commands/venv.rs @@ -5,10 +5,11 @@ use anyhow::Result; use colored::Colorize; use fs_err as fs; use miette::{Diagnostic, IntoDiagnostic}; -use platform_host::Platform; -use puffin_interpreter::InterpreterInfo; use thiserror::Error; +use platform_host::Platform; +use puffin_interpreter::Interpreter; + use crate::commands::ExitStatus; use crate::printer::Printer; @@ -43,7 +44,7 @@ enum VenvError { #[error("Failed to extract Python interpreter info")] #[diagnostic(code(puffin::venv::interpreter))] - InterpreterError(#[source] anyhow::Error), + InterpreterError(#[source] puffin_interpreter::Error), #[error("Failed to create virtual environment")] #[diagnostic(code(puffin::venv::creation))] @@ -73,8 +74,8 @@ fn venv_impl( }; let platform = Platform::current().into_diagnostic()?; - let interpreter_info = InterpreterInfo::query(&base_python, platform, None) - .map_err(VenvError::InterpreterError)?; + let interpreter_info = + Interpreter::query(&base_python, platform, None).map_err(VenvError::InterpreterError)?; writeln!( printer, diff --git a/crates/puffin-dev/src/build.rs b/crates/puffin-dev/src/build.rs index 8c86046c6..010c2c96e 100644 --- a/crates/puffin-dev/src/build.rs +++ b/crates/puffin-dev/src/build.rs @@ -46,7 +46,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result { let build_dispatch = BuildDispatch::new( RegistryClientBuilder::new(cache_dir.path().clone()).build(), cache_dir.path().clone(), - venv.interpreter_info().clone(), + venv.interpreter().clone(), fs::canonicalize(venv.python_executable())?, false, ); diff --git a/crates/puffin-dev/src/resolve_cli.rs b/crates/puffin-dev/src/resolve_cli.rs index 14ad97dcd..407ec04de 100644 --- a/crates/puffin-dev/src/resolve_cli.rs +++ b/crates/puffin-dev/src/resolve_cli.rs @@ -50,15 +50,15 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> { let build_dispatch = BuildDispatch::new( client.clone(), cache_dir.path().clone(), - venv.interpreter_info().clone(), + venv.interpreter().clone(), fs::canonicalize(venv.python_executable())?, args.no_build, ); // Copied from `BuildDispatch` let tags = Tags::from_env( - venv.interpreter_info().platform(), - venv.interpreter_info().simple_version(), + venv.interpreter().platform(), + venv.interpreter().simple_version(), )?; let resolver = Resolver::new( Manifest::new( @@ -68,7 +68,7 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> { None, ), ResolutionOptions::default(), - venv.interpreter_info().markers(), + venv.interpreter().markers(), &tags, &client, &build_dispatch, diff --git a/crates/puffin-dev/src/resolve_many.rs b/crates/puffin-dev/src/resolve_many.rs index f7d821e06..b1eb4283d 100644 --- a/crates/puffin-dev/src/resolve_many.rs +++ b/crates/puffin-dev/src/resolve_many.rs @@ -52,7 +52,7 @@ pub(crate) async fn resolve_many(args: ResolveManyArgs) -> Result<()> { let build_dispatch = BuildDispatch::new( RegistryClientBuilder::new(cache_dir.path().clone()).build(), cache_dir.path().clone(), - venv.interpreter_info().clone(), + venv.interpreter().clone(), fs::canonicalize(venv.python_executable())?, args.no_build, ); diff --git a/crates/puffin-dispatch/src/lib.rs b/crates/puffin-dispatch/src/lib.rs index 991cf83b4..342f4c796 100644 --- a/crates/puffin-dispatch/src/lib.rs +++ b/crates/puffin-dispatch/src/lib.rs @@ -17,7 +17,7 @@ use platform_tags::Tags; use puffin_build::{SourceBuild, SourceBuildContext}; use puffin_client::RegistryClient; use puffin_installer::{Builder, Downloader, InstallPlan, Installer, Unzipper}; -use puffin_interpreter::{InterpreterInfo, Virtualenv}; +use puffin_interpreter::{Interpreter, Virtualenv}; use puffin_resolver::{DistFinder, Manifest, ResolutionOptions, Resolver}; use puffin_traits::BuildContext; @@ -26,7 +26,7 @@ use puffin_traits::BuildContext; pub struct BuildDispatch { client: RegistryClient, cache: PathBuf, - interpreter_info: InterpreterInfo, + interpreter: Interpreter, base_python: PathBuf, no_build: bool, source_build_context: SourceBuildContext, @@ -37,14 +37,14 @@ impl BuildDispatch { pub fn new( client: RegistryClient, cache: PathBuf, - interpreter_info: InterpreterInfo, + interpreter: Interpreter, base_python: PathBuf, no_build: bool, ) -> Self { Self { client, cache, - interpreter_info, + interpreter, base_python, no_build, source_build_context: SourceBuildContext::default(), @@ -64,8 +64,8 @@ impl BuildContext for BuildDispatch { self.cache.as_path() } - fn interpreter_info(&self) -> &InterpreterInfo { - &self.interpreter_info + fn interpreter(&self) -> &Interpreter { + &self.interpreter } fn base_python(&self) -> &Path { @@ -83,13 +83,13 @@ impl BuildContext for BuildDispatch { ) -> Pin>> + Send + 'a>> { Box::pin(async { let tags = Tags::from_env( - self.interpreter_info.platform(), - self.interpreter_info.simple_version(), + self.interpreter.platform(), + self.interpreter.simple_version(), )?; let resolver = Resolver::new( Manifest::new(requirements.to_vec(), Vec::default(), Vec::default(), None), self.options, - self.interpreter_info.markers(), + self.interpreter.markers(), &tags, &self.client, self, @@ -124,8 +124,8 @@ impl BuildContext for BuildDispatch { } = InstallPlan::try_from_requirements(requirements, &self.cache, venv)?; let tags = Tags::from_env( - self.interpreter_info.platform(), - self.interpreter_info.simple_version(), + self.interpreter.platform(), + self.interpreter.simple_version(), )?; // Resolve the dependencies. @@ -137,7 +137,7 @@ impl BuildContext for BuildDispatch { if remote.len() == 1 { "" } else { "s" }, remote.iter().map(ToString::to_string).join(", ") ); - let resolution = DistFinder::new(&tags, &self.client, self.interpreter_info()) + let resolution = DistFinder::new(&tags, &self.client, self.interpreter()) .resolve(&remote) .await .context("Failed to resolve build dependencies")?; @@ -249,7 +249,7 @@ impl BuildContext for BuildDispatch { let builder = SourceBuild::setup( source, subdirectory, - &self.interpreter_info, + &self.interpreter, self, self.source_build_context.clone(), package_id, diff --git a/crates/puffin-installer/src/installer.rs b/crates/puffin-installer/src/installer.rs index 4c298a814..3ac49a0d1 100644 --- a/crates/puffin-installer/src/installer.rs +++ b/crates/puffin-installer/src/installer.rs @@ -41,7 +41,7 @@ impl<'a> Installer<'a> { wheels.par_iter().try_for_each(|wheel| { let location = install_wheel_rs::InstallLocation::new( self.venv.root(), - self.venv.interpreter_info().simple_version(), + self.venv.interpreter().simple_version(), ); install_wheel_rs::linker::install_wheel( diff --git a/crates/puffin-interpreter/src/interpreter_info.rs b/crates/puffin-interpreter/src/interpreter.rs similarity index 71% rename from crates/puffin-interpreter/src/interpreter_info.rs rename to crates/puffin-interpreter/src/interpreter.rs index 8b5b205f0..0f94427d9 100644 --- a/crates/puffin-interpreter/src/interpreter_info.rs +++ b/crates/puffin-interpreter/src/interpreter.rs @@ -1,10 +1,8 @@ -use std::io; use std::path::{Path, PathBuf}; use std::process::Command; +use std::time::UNIX_EPOCH; -use anyhow::Result; use serde::{Deserialize, Serialize}; -use thiserror::Error; use tracing::debug; use pep440_rs::Version; @@ -12,10 +10,11 @@ use pep508_rs::MarkerEnvironment; use platform_host::Platform; use crate::python_platform::PythonPlatform; +use crate::Error; /// A Python executable and its associated platform markers. #[derive(Debug, Clone)] -pub struct InterpreterInfo { +pub struct Interpreter { pub(crate) platform: PythonPlatform, pub(crate) markers: MarkerEnvironment, pub(crate) base_exec_prefix: PathBuf, @@ -23,9 +22,13 @@ pub struct InterpreterInfo { pub(crate) sys_executable: PathBuf, } -impl InterpreterInfo { +impl Interpreter { /// Detect the interpreter info for the given Python executable. - pub fn query(executable: &Path, platform: Platform, cache: Option<&Path>) -> Result { + pub fn query( + executable: &Path, + platform: Platform, + cache: Option<&Path>, + ) -> Result { let info = if let Some(cache) = cache { InterpreterQueryResult::query_cached(executable, cache)? } else { @@ -65,7 +68,7 @@ impl InterpreterInfo { } } -impl InterpreterInfo { +impl Interpreter { /// Returns the path to the Python virtual environment. pub fn platform(&self) -> &Platform { &self.platform @@ -106,18 +109,6 @@ impl InterpreterInfo { } } -#[derive(Debug, Error)] -pub(crate) enum InterpreterQueryError { - #[error(transparent)] - IO(#[from] io::Error), - #[error("Failed to query python interpreter at {interpreter}")] - PythonSubcommand { - interpreter: PathBuf, - #[source] - err: io::Error, - }, -} - #[derive(Deserialize, Serialize)] pub(crate) struct InterpreterQueryResult { pub(crate) markers: MarkerEnvironment, @@ -128,11 +119,11 @@ pub(crate) struct InterpreterQueryResult { impl InterpreterQueryResult { /// Return the resolved [`InterpreterQueryResult`] for the given Python executable. - pub(crate) fn query(interpreter: &Path) -> Result { + pub(crate) fn query(interpreter: &Path) -> Result { let output = Command::new(interpreter) .args(["-c", include_str!("get_interpreter_info.py")]) .output() - .map_err(|err| InterpreterQueryError::PythonSubcommand { + .map_err(|err| Error::PythonSubcommandLaunch { interpreter: interpreter.to_path_buf(), err, })?; @@ -140,35 +131,27 @@ impl InterpreterQueryResult { // stderr isn't technically a criterion for success, but i don't know of any cases where there // should be stderr output and if there is, we want to know if !output.status.success() || !output.stderr.is_empty() { - return Err(InterpreterQueryError::PythonSubcommand { - interpreter: interpreter.to_path_buf(), - err: io::Error::new( - io::ErrorKind::Other, - format!( - "Querying python at {} failed with status {}:\n--- stdout:\n{}\n--- stderr:\n{}", - interpreter.display(), - output.status, - String::from_utf8_lossy(&output.stdout).trim(), - String::from_utf8_lossy(&output.stderr).trim() - ), + return Err(Error::PythonSubcommandOutput { + message: format!( + "Querying python at {} failed with status {}", + interpreter.display(), + output.status, ), + stdout: String::from_utf8_lossy(&output.stdout).trim().to_string(), + stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(), }); } - let data = serde_json::from_slice::(&output.stdout).map_err(|err| - InterpreterQueryError::PythonSubcommand { - interpreter: interpreter.to_path_buf(), - err: io::Error::new( - io::ErrorKind::Other, - format!( - "Querying python at {} did not return the expected data ({}):\n--- stdout:\n{}\n--- stderr:\n{}", - interpreter.display(), - err, - String::from_utf8_lossy(&output.stdout).trim(), - String::from_utf8_lossy(&output.stderr).trim() - ), + let data = serde_json::from_slice::(&output.stdout).map_err(|err| { + Error::PythonSubcommandOutput { + message: format!( + "Querying python at {} did not return the expected data: {}", + interpreter.display(), + err, ), + stdout: String::from_utf8_lossy(&output.stdout).trim().to_string(), + stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(), } - )?; + })?; Ok(data) } @@ -178,7 +161,7 @@ impl InterpreterQueryResult { /// Running a Python script is (relatively) expensive, and the markers won't change /// unless the Python executable changes, so we use the executable's last modified /// time as a cache key. - pub(crate) fn query_cached(executable: &Path, cache: &Path) -> Result { + pub(crate) fn query_cached(executable: &Path, cache: &Path) -> Result { // Read from the cache. let key = if let Ok(key) = cache_key(executable) { if let Ok(data) = cacache::read_sync(cache, &key) { @@ -211,11 +194,15 @@ impl InterpreterQueryResult { /// Create a cache key for the Python executable, consisting of the executable's /// last modified time and the executable's path. -fn cache_key(executable: &Path) -> Result { - let modified = executable - .metadata()? +fn cache_key(executable: &Path) -> Result { + let modified = fs_err::metadata(executable)? + // Note: This is infallible on windows and unix (i.e. all platforms we support) .modified()? - .duration_since(std::time::UNIX_EPOCH)? - .as_millis(); - Ok(format!("puffin:v0:{}:{}", executable.display(), modified)) + .duration_since(UNIX_EPOCH) + .map_err(|err| Error::SystemTime(executable.to_path_buf(), err))?; + Ok(format!( + "puffin:v0:{}:{}", + executable.display(), + modified.as_millis() + )) } diff --git a/crates/puffin-interpreter/src/lib.rs b/crates/puffin-interpreter/src/lib.rs index 056fc9c34..4eca309be 100644 --- a/crates/puffin-interpreter/src/lib.rs +++ b/crates/puffin-interpreter/src/lib.rs @@ -1,6 +1,44 @@ -pub use crate::interpreter_info::InterpreterInfo; +use std::io; +use std::path::PathBuf; +use std::time::SystemTimeError; + +use thiserror::Error; + +pub use crate::interpreter::Interpreter; pub use crate::virtual_env::Virtualenv; -mod interpreter_info; +mod interpreter; mod python_platform; mod virtual_env; + +#[derive(Debug, Error)] +pub enum Error { + #[error("Expected {0} to be a virtual environment, but pyvenv.cfg is missing")] + MissingPyVenvCfg(PathBuf), + #[error("Your virtualenv at {0} is broken. It contains a pyvenv.cfg but no python at {1}")] + BrokenVenv(PathBuf, PathBuf), + #[error("Both VIRTUAL_ENV and CONDA_PREFIX are set. Please unset one of them.")] + Conflict, + #[error("Couldn't find a virtualenv or conda environment (Looked for VIRTUAL_ENV, CONDA_PREFIX and .venv)")] + NotFound, + #[error(transparent)] + Io(#[from] io::Error), + #[error("Invalid modified date on {0}")] + SystemTime(PathBuf, #[source] SystemTimeError), + #[error("Failed to query python interpreter at {interpreter}")] + PythonSubcommandLaunch { + interpreter: PathBuf, + #[source] + err: io::Error, + }, + #[error("{message}:\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")] + PythonSubcommandOutput { + message: String, + stdout: String, + stderr: String, + }, + #[error("Failed to write to cache")] + Cacache(#[from] cacache::Error), + #[error("Failed to write to cache")] + Serde(#[from] serde_json::Error), +} diff --git a/crates/puffin-interpreter/src/virtual_env.rs b/crates/puffin-interpreter/src/virtual_env.rs index 01fbee4bf..2d1857574 100644 --- a/crates/puffin-interpreter/src/virtual_env.rs +++ b/crates/puffin-interpreter/src/virtual_env.rs @@ -1,52 +1,58 @@ use std::env; use std::path::{Path, PathBuf}; -use crate::InterpreterInfo; -use anyhow::{bail, Result}; -use platform_host::Platform; use tracing::debug; +use platform_host::Platform; + use crate::python_platform::PythonPlatform; +use crate::{Error, Interpreter}; /// A Python executable and its associated platform markers. #[derive(Debug, Clone)] pub struct Virtualenv { root: PathBuf, - interpreter_info: InterpreterInfo, + interpreter: Interpreter, } impl Virtualenv { /// Venv the current Python executable from the host environment. - pub fn from_env(platform: Platform, cache: Option<&Path>) -> Result { + pub fn from_env(platform: Platform, cache: Option<&Path>) -> Result { let platform = PythonPlatform::from(platform); - let venv = detect_virtual_env(&platform)?; + let Some(venv) = detect_virtual_env(&platform)? else { + return Err(Error::NotFound); + }; let executable = platform.venv_python(&venv); - let interpreter_info = InterpreterInfo::query(&executable, platform.0, cache)?; + let interpreter = Interpreter::query(&executable, platform.0, cache)?; Ok(Self { root: venv, - interpreter_info, + interpreter, }) } - pub fn from_virtualenv(platform: Platform, root: &Path, cache: Option<&Path>) -> Result { + pub fn from_virtualenv( + platform: Platform, + root: &Path, + cache: Option<&Path>, + ) -> Result { let platform = PythonPlatform::from(platform); let executable = platform.venv_python(root); - let interpreter_info = InterpreterInfo::query(&executable, platform.0, cache)?; + let interpreter = Interpreter::query(&executable, platform.0, cache)?; Ok(Self { root: root.to_path_buf(), - interpreter_info, + interpreter, }) } /// Creating a new venv from a python interpreter changes this - pub fn new_prefix(venv: &Path, interpreter_info: &InterpreterInfo) -> Self { + pub fn new_prefix(venv: &Path, interpreter: &Interpreter) -> Self { Self { root: venv.to_path_buf(), - interpreter_info: InterpreterInfo { + interpreter: Interpreter { base_prefix: venv.to_path_buf(), - ..interpreter_info.clone() + ..interpreter.clone() }, } } @@ -74,25 +80,37 @@ impl Virtualenv { &self.root } - pub fn interpreter_info(&self) -> &InterpreterInfo { - &self.interpreter_info + pub fn interpreter(&self) -> &Interpreter { + &self.interpreter } /// Returns the path to the `site-packages` directory inside a virtual environment. pub fn site_packages(&self) -> PathBuf { - self.interpreter_info + self.interpreter .platform - .venv_site_packages(&self.root, self.interpreter_info().simple_version()) + .venv_site_packages(&self.root, self.interpreter().simple_version()) } } /// Locate the current virtual environment. -pub(crate) fn detect_virtual_env(target: &PythonPlatform) -> Result { +pub(crate) fn detect_virtual_env(target: &PythonPlatform) -> Result, Error> { match (env::var_os("VIRTUAL_ENV"), env::var_os("CONDA_PREFIX")) { - (Some(dir), None) => return Ok(PathBuf::from(dir)), - (None, Some(dir)) => return Ok(PathBuf::from(dir)), + (Some(dir), None) => { + debug!( + "Found a virtualenv through VIRTUAL_ENV at {}", + Path::new(&dir).display() + ); + return Ok(Some(PathBuf::from(dir))); + } + (None, Some(dir)) => { + debug!( + "Found a virtualenv through CONDA_PREFIX at {}", + Path::new(&dir).display() + ); + return Ok(Some(PathBuf::from(dir))); + } (Some(_), Some(_)) => { - bail!("Both VIRTUAL_ENV and CONDA_PREFIX are set. Please unset one of them.") + return Err(Error::Conflict); } (None, None) => { // No environment variables set. Try to find a virtualenv in the current directory. @@ -105,23 +123,16 @@ pub(crate) fn detect_virtual_env(target: &PythonPlatform) -> Result { let dot_venv = dir.join(".venv"); if dot_venv.is_dir() { if !dot_venv.join("pyvenv.cfg").is_file() { - bail!( - "Expected {} to be a virtual environment, but pyvenv.cfg is missing", - dot_venv.display() - ); + return Err(Error::MissingPyVenvCfg(dot_venv)); } let python = target.venv_python(&dot_venv); if !python.is_file() { - bail!( - "Your virtualenv at {} is broken. It contains a pyvenv.cfg but no python at {}", - dot_venv.display(), - python.display() - ); + return Err(Error::BrokenVenv(dot_venv, python)); } debug!("Found a virtualenv named .venv at {}", dot_venv.display()); - return Ok(dot_venv); + return Ok(Some(dot_venv)); } } - bail!("Couldn't find a virtualenv or conda environment.") + Ok(None) } diff --git a/crates/puffin-resolver/src/finder.rs b/crates/puffin-resolver/src/finder.rs index 7f3b5cd33..a918efea7 100644 --- a/crates/puffin-resolver/src/finder.rs +++ b/crates/puffin-resolver/src/finder.rs @@ -15,7 +15,7 @@ use pep440_rs::Version; use pep508_rs::{Requirement, VersionOrUrl}; use platform_tags::{TagPriority, Tags}; use puffin_client::RegistryClient; -use puffin_interpreter::InterpreterInfo; +use puffin_interpreter::Interpreter; use puffin_normalize::PackageName; use pypi_types::{File, IndexUrl, SimpleJson}; @@ -26,21 +26,17 @@ pub struct DistFinder<'a> { tags: &'a Tags, client: &'a RegistryClient, reporter: Option>, - interpreter_info: &'a InterpreterInfo, + interpreter: &'a Interpreter, } impl<'a> DistFinder<'a> { /// Initialize a new distribution finder. - pub fn new( - tags: &'a Tags, - client: &'a RegistryClient, - interpreter_info: &'a InterpreterInfo, - ) -> Self { + pub fn new(tags: &'a Tags, client: &'a RegistryClient, interpreter: &'a Interpreter) -> Self { Self { tags, client, reporter: None, - interpreter_info, + interpreter, } } @@ -157,7 +153,7 @@ impl<'a> DistFinder<'a> { .requires_python .as_ref() .map_or(true, |requires_python| { - requires_python.contains(self.interpreter_info.version()) + requires_python.contains(self.interpreter.version()) }) { continue; diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 8c70d67b1..c027356b0 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -552,7 +552,7 @@ impl<'a, Context: BuildContext + Send + Sync> Resolver<'a, Context> { metadata, &package_name, self.tags, - self.build_context.interpreter_info().version(), + self.build_context.interpreter().version(), self.exclude_newer.as_ref(), ); self.index diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index f20a4701d..43c411a1b 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -17,7 +17,7 @@ use pep508_rs::{MarkerEnvironment, Requirement, StringVersion}; use platform_host::{Arch, Os, Platform}; use platform_tags::Tags; use puffin_client::RegistryClientBuilder; -use puffin_interpreter::{InterpreterInfo, Virtualenv}; +use puffin_interpreter::{Interpreter, Virtualenv}; use puffin_resolver::{ Graph, Manifest, PreReleaseMode, ResolutionMode, ResolutionOptions, Resolver, }; @@ -31,7 +31,7 @@ static EXCLUDE_NEWER: Lazy> = Lazy::new(|| { }); struct DummyContext { - interpreter_info: InterpreterInfo, + interpreter: Interpreter, } impl BuildContext for DummyContext { @@ -39,8 +39,8 @@ impl BuildContext for DummyContext { panic!("The test should not need to build source distributions") } - fn interpreter_info(&self) -> &InterpreterInfo { - &self.interpreter_info + fn interpreter(&self) -> &Interpreter { + &self.interpreter } fn base_python(&self) -> &Path { @@ -82,7 +82,7 @@ async fn resolve( let temp_dir = tempdir()?; let client = RegistryClientBuilder::new(temp_dir.path()).build(); let build_context = DummyContext { - interpreter_info: InterpreterInfo::artificial( + interpreter: Interpreter::artificial( Platform::current()?, markers.clone(), PathBuf::from("/dev/null"), diff --git a/crates/puffin-traits/src/lib.rs b/crates/puffin-traits/src/lib.rs index 91308e357..92a0f5df2 100644 --- a/crates/puffin-traits/src/lib.rs +++ b/crates/puffin-traits/src/lib.rs @@ -7,7 +7,7 @@ use std::pin::Pin; use anyhow::Result; use pep508_rs::Requirement; -use puffin_interpreter::{InterpreterInfo, Virtualenv}; +use puffin_interpreter::{Interpreter, Virtualenv}; /// Avoid cyclic crate dependencies between resolver, installer and builder. /// @@ -54,7 +54,7 @@ pub trait BuildContext { /// All (potentially nested) source distribution builds use the same base python and can reuse /// it's metadata (e.g. wheel compatibility tags). - fn interpreter_info(&self) -> &InterpreterInfo; + fn interpreter(&self) -> &Interpreter; /// The system (or conda) python interpreter to create venvs. fn base_python(&self) -> &Path;