Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid warning about bad Python interpreter links for empty project environment directories #7527

Merged
merged 1 commit into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading