Skip to content

Commit

Permalink
Avoid warning about bad Python interpreter links for empty project en…
Browse files Browse the repository at this point in the history
…vironment directories (#7527)

Someone reported this a while back (will try to find the issue), and I
ran into it working on #7522
  • Loading branch information
zanieb authored Sep 19, 2024
1 parent 4fdf5fc commit 6b08aae
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 21 deletions.
34 changes: 34 additions & 0 deletions crates/uv-python/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ pub struct EnvironmentNotFound {
preference: EnvironmentPreference,
}

#[derive(Clone, Debug, Error)]
pub struct InvalidEnvironment {
path: PathBuf,
reason: String,
}

impl From<PythonNotFound> for EnvironmentNotFound {
fn from(value: PythonNotFound) -> Self {
Self {
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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)?;

Expand Down
3 changes: 3 additions & 0 deletions crates/uv-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 10 additions & 18 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`")]
Expand Down Expand Up @@ -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, uv_python::Error> {
PythonEnvironment::from_root(workspace.venv(), cache)
}

#[derive(Debug)]
#[allow(clippy::large_enum_variant)]
pub(crate) enum FoundInterpreter {
Expand Down Expand Up @@ -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) {
Expand All @@ -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: {}",
Expand Down Expand Up @@ -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(()) => {
Expand Down
4 changes: 1 addition & 3 deletions crates/uv/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand Down

0 comments on commit 6b08aae

Please sign in to comment.