From 6b08aaecadd71cbc16deef62a1fa672877df4423 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 19 Sep 2024 06:49:35 -0500 Subject: [PATCH] Avoid warning about bad Python interpreter links for empty project environment directories (#7527) Someone reported this a while back (will try to find the issue), and I ran into it working on #7522 --- crates/uv-python/src/environment.rs | 34 +++++++++++++++++++++++++++ crates/uv-python/src/lib.rs | 3 +++ crates/uv/src/commands/project/mod.rs | 28 ++++++++-------------- crates/uv/tests/sync.rs | 4 +--- 4 files changed, 48 insertions(+), 21 deletions(-) diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index a6dcfd614..e254e3095 100644 --- a/crates/uv-python/src/environment.rs +++ b/crates/uv-python/src/environment.rs @@ -34,6 +34,12 @@ pub struct EnvironmentNotFound { preference: EnvironmentPreference, } +#[derive(Clone, Debug, Error)] +pub struct InvalidEnvironment { + path: PathBuf, + reason: String, +} + impl From for EnvironmentNotFound { fn from(value: PythonNotFound) -> Self { Self { @@ -98,6 +104,17 @@ impl fmt::Display for EnvironmentNotFound { } } +impl std::fmt::Display for InvalidEnvironment { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!( + f, + "Invalid environment at `{}`: {}", + self.path.user_display(), + self.reason + ) + } +} + impl PythonEnvironment { /// Find a [`PythonEnvironment`] matching the given request and preference. /// @@ -133,6 +150,23 @@ impl PythonEnvironment { } Err(err) => return Err(Error::Discovery(err.into())), }; + + if venv.is_file() { + return Err(InvalidEnvironment { + path: venv, + reason: "expected directory but found a file".to_string(), + } + .into()); + } + + if !venv.join("pyvenv.cfg").is_file() { + return Err(InvalidEnvironment { + path: venv, + reason: "missing a `pyvenv.cfg` marker".to_string(), + } + .into()); + } + let executable = virtualenv_python_executable(venv); let interpreter = Interpreter::query(executable, cache)?; diff --git a/crates/uv-python/src/lib.rs b/crates/uv-python/src/lib.rs index 5917671db..5cdb3a4a4 100644 --- a/crates/uv-python/src/lib.rs +++ b/crates/uv-python/src/lib.rs @@ -78,6 +78,9 @@ pub enum Error { #[error(transparent)] MissingEnvironment(#[from] environment::EnvironmentNotFound), + + #[error(transparent)] + InvalidEnvironment(#[from] environment::InvalidEnvironment), } // The mock interpreters are not valid on Windows so we don't have unit test coverage there diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 06823f552..fb9c6a6e9 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -120,7 +120,7 @@ pub(crate) enum ProjectError { #[error("Environment marker is empty")] EmptyEnvironment, - #[error("Project virtual environment directory `{0}` cannot be used because it has existing, non-virtual environment content")] + #[error("Project virtual environment directory `{0}` cannot be used because it is not a virtual environment and is non-empty")] InvalidProjectEnvironmentDir(PathBuf), #[error("Failed to parse `pyproject.toml`")] @@ -275,14 +275,6 @@ pub(crate) fn validate_requires_python( } } -/// Find the virtual environment for the current project. -fn find_environment( - workspace: &Workspace, - cache: &Cache, -) -> Result { - PythonEnvironment::from_root(workspace.venv(), cache) -} - #[derive(Debug)] #[allow(clippy::large_enum_variant)] pub(crate) enum FoundInterpreter { @@ -374,7 +366,8 @@ impl FoundInterpreter { } = WorkspacePython::from_request(python_request, workspace).await?; // Read from the virtual environment first. - match find_environment(workspace, cache) { + let venv = workspace.venv(); + match PythonEnvironment::from_root(&venv, cache) { Ok(venv) => { if python_request.as_ref().map_or(true, |request| { if request.satisfied(venv.interpreter(), cache) { @@ -400,6 +393,13 @@ impl FoundInterpreter { } } Err(uv_python::Error::MissingEnvironment(_)) => {} + Err(uv_python::Error::InvalidEnvironment(_)) => { + // If there's an invalid environment with existing content, we error instead of + // deleting it later on. + if fs_err::read_dir(&venv).is_ok_and(|mut dir| dir.next().is_some()) { + return Err(ProjectError::InvalidProjectEnvironmentDir(venv)); + } + } Err(uv_python::Error::Query(uv_python::InterpreterError::NotFound(path))) => { warn_user!( "Ignoring existing virtual environment linked to non-existent Python interpreter: {}", @@ -491,14 +491,6 @@ pub(crate) async fn get_or_init_environment( FoundInterpreter::Interpreter(interpreter) => { let venv = workspace.venv(); - // Before deleting the target directory, we confirm that it is either (1) a virtual - // environment or (2) an empty directory. - if PythonEnvironment::from_root(&venv, cache).is_err() - && fs_err::read_dir(&venv).is_ok_and(|mut dir| dir.next().is_some()) - { - return Err(ProjectError::InvalidProjectEnvironmentDir(venv)); - } - // Remove the existing virtual environment if it doesn't meet the requirements. match fs_err::remove_dir_all(&venv) { Ok(()) => { diff --git a/crates/uv/tests/sync.rs b/crates/uv/tests/sync.rs index e24a912c1..8cd6cdc7a 100644 --- a/crates/uv/tests/sync.rs +++ b/crates/uv/tests/sync.rs @@ -1747,9 +1747,7 @@ fn sync_custom_environment_path() -> Result<()> { ----- stdout ----- ----- stderr ----- - warning: Ignoring existing virtual environment linked to non-existent Python interpreter: foo/[BIN]/python - Using Python 3.12.[X] interpreter at: [PYTHON-3.12] - error: Project virtual environment directory `[TEMP_DIR]/foo` cannot be used because it has existing, non-virtual environment content + error: Project virtual environment directory `[TEMP_DIR]/foo` cannot be used because it is not a virtual environment and is non-empty "###); // But if it's just an incompatible virtual environment...