Skip to content

Commit

Permalink
Allow system environments during project environment validity check
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Sep 20, 2024
1 parent 8259600 commit efefa59
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 33 deletions.
36 changes: 29 additions & 7 deletions crates/uv-python/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ pub struct EnvironmentNotFound {
#[derive(Clone, Debug, Error)]
pub struct InvalidEnvironment {
path: PathBuf,
reason: String,
pub kind: InvalidEnvironmentKind,
}
#[derive(Debug, Clone)]
pub enum InvalidEnvironmentKind {
NotDirectory,
MissingExecutable(PathBuf),
}

impl From<PythonNotFound> for EnvironmentNotFound {
Expand Down Expand Up @@ -110,11 +115,22 @@ impl std::fmt::Display for InvalidEnvironment {
f,
"Invalid environment at `{}`: {}",
self.path.user_display(),
self.reason
self.kind
)
}
}

impl std::fmt::Display for InvalidEnvironmentKind {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::NotDirectory => write!(f, "expected directory but found a file"),
Self::MissingExecutable(path) => {
write!(f, "missing Python executable at `{}`", path.user_display())
}
}
}
}

impl PythonEnvironment {
/// Find a [`PythonEnvironment`] matching the given request and preference.
///
Expand All @@ -139,6 +155,8 @@ impl PythonEnvironment {
}

/// Create a [`PythonEnvironment`] from the virtual environment at the given root.
///
/// N.B. This function also works for system Python environments and users depend on this.
pub fn from_root(root: impl AsRef<Path>, cache: &Cache) -> Result<Self, Error> {
let venv = match fs_err::canonicalize(root.as_ref()) {
Ok(venv) => venv,
Expand All @@ -154,20 +172,24 @@ impl PythonEnvironment {
if venv.is_file() {
return Err(InvalidEnvironment {
path: venv,
reason: "expected directory but found a file".to_string(),
kind: InvalidEnvironmentKind::NotDirectory,
}
.into());
}

if !venv.join("pyvenv.cfg").is_file() {
let executable = virtualenv_python_executable(&venv);

// Check if the executable exists before querying so we can provide a more specific error
// Note we intentionally don't require a resolved link to exist here, we're just trying to
// tell if this _looks_ like a Python environment.
if !(executable.is_symlink() || executable.is_file()) {
return Err(InvalidEnvironment {
path: venv,
reason: "missing a `pyvenv.cfg` marker".to_string(),
kind: InvalidEnvironmentKind::MissingExecutable(executable.clone()),
}
.into());
}
};

let executable = virtualenv_python_executable(venv);
let interpreter = Interpreter::query(executable, cache)?;

Ok(Self(Arc::new(PythonEnvironmentShared {
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub use crate::discovery::{
find_python_installations, EnvironmentPreference, Error as DiscoveryError, PythonDownloads,
PythonNotFound, PythonPreference, PythonRequest, PythonSource, VersionRequest,
};
pub use crate::environment::PythonEnvironment;
pub use crate::environment::{InvalidEnvironment, InvalidEnvironmentKind, PythonEnvironment};
pub use crate::implementation::ImplementationName;
pub use crate::installation::{PythonInstallation, PythonInstallationKey};
pub use crate::interpreter::{Error as InterpreterError, Interpreter};
Expand Down
45 changes: 31 additions & 14 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use uv_fs::Simplified;
use uv_installer::{SatisfiesResult, SitePackages};
use uv_normalize::PackageName;
use uv_python::{
EnvironmentPreference, Interpreter, PythonDownloads, PythonEnvironment, PythonInstallation,
PythonPreference, PythonRequest, PythonVersionFile, VersionRequest,
EnvironmentPreference, Interpreter, InvalidEnvironmentKind, PythonDownloads, PythonEnvironment,
PythonInstallation, PythonPreference, PythonRequest, PythonVersionFile, VersionRequest,
};
use uv_requirements::{
NamedRequirementsError, NamedRequirementsResolver, RequirementsSpecification,
Expand Down Expand Up @@ -120,8 +120,8 @@ pub(crate) enum ProjectError {
#[error("Environment marker is empty")]
EmptyEnvironment,

#[error("Project virtual environment directory `{0}` cannot be used because it is not a virtual environment and is non-empty")]
InvalidProjectEnvironmentDir(PathBuf),
#[error("Project virtual environment directory `{0}` cannot be used because {1}")]
InvalidProjectEnvironmentDir(PathBuf, String),

#[error("Failed to parse `pyproject.toml`")]
TomlParse(#[source] toml::de::Error),
Expand Down Expand Up @@ -393,12 +393,26 @@ impl FoundInterpreter {
}
}
Err(uv_python::Error::MissingEnvironment(_)) => {}
Err(uv_python::Error::InvalidEnvironment(_)) => {
Err(uv_python::Error::InvalidEnvironment(inner)) => {
// 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));
}
// deleting it later on
match inner.kind {
InvalidEnvironmentKind::NotDirectory => {
return Err(ProjectError::InvalidProjectEnvironmentDir(
venv,
inner.kind.to_string(),
))
}
InvalidEnvironmentKind::MissingExecutable(_) => {
if fs_err::read_dir(&venv).is_ok_and(|mut dir| dir.next().is_some()) {
return Err(ProjectError::InvalidProjectEnvironmentDir(
venv,
"because it is not a valid Python environment (no Python executable was found)"
.to_string(),
));
}
}
};
}
Err(uv_python::Error::Query(uv_python::InterpreterError::NotFound(path))) => {
if path.is_symlink() {
Expand All @@ -408,11 +422,6 @@ impl FoundInterpreter {
path.user_display().cyan(),
target_path.user_display().cyan(),
);
} else {
warn_user!(
"Ignoring existing virtual environment with missing Python interpreter: {}",
path.user_display().cyan()
);
}
}
Err(err) => return Err(err.into()),
Expand Down Expand Up @@ -500,6 +509,14 @@ pub(crate) async fn get_or_init_environment(
FoundInterpreter::Interpreter(interpreter) => {
let venv = workspace.venv();

// Avoid removing things that are not virtual environments
if venv.exists() && !venv.join("pyvenv.cfg").exists() {
return Err(ProjectError::InvalidProjectEnvironmentDir(
venv,
"it is not a compatible environment but cannot be recreated because it is not a virtual environment".to_string(),
));
}

// Remove the existing virtual environment if it doesn't meet the requirements.
match fs_err::remove_dir_all(&venv) {
Ok(()) => {
Expand Down
56 changes: 46 additions & 10 deletions crates/uv/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1747,7 +1747,7 @@ fn sync_custom_environment_path() -> Result<()> {
----- stdout -----
----- stderr -----
error: Project virtual environment directory `[TEMP_DIR]/foo` cannot be used because it is not a virtual environment and is non-empty
error: Project virtual environment directory `[TEMP_DIR]/foo` cannot be used because because it is not a valid Python environment (no Python executable was found)
"###);

// But if it's just an incompatible virtual environment...
Expand Down Expand Up @@ -2640,7 +2640,7 @@ fn sync_invalid_environment() -> Result<()> {
----- stdout -----
----- stderr -----
error: Project virtual environment directory `[VENV]/` cannot be used because it is not a virtual environment and is non-empty
error: Project virtual environment directory `[VENV]/` cannot be used because because it is not a valid Python environment (no Python executable was found)
"###);

// But if it's just an incompatible virtual environment...
Expand Down Expand Up @@ -2677,10 +2677,11 @@ fn sync_invalid_environment() -> Result<()> {

let bin = venv_bin_path(context.temp_dir.join(".venv"));

// If it's there's a broken symlink, we should warn
// If there's just a broken symlink, we should warn
#[cfg(unix)]
{
fs_err::remove_file(bin.join("python"))?;
fs_err::os::unix::fs::symlink(context.temp_dir.join("does-not-exist"), bin.join("python"))?;
uv_snapshot!(context.filters(), context.sync(), @r###"
success: true
exit_code: 0
Expand All @@ -2697,22 +2698,57 @@ fn sync_invalid_environment() -> Result<()> {
"###);
}

// And if the Python executable is missing entirely we should warn
// But if the Python executable is missing entirely we should also fail
fs_err::remove_dir_all(&bin)?;
uv_snapshot!(context.filters(), context.sync(), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: Project virtual environment directory `[VENV]/` cannot be used because because it is not a valid Python environment (no Python executable was found)
"###);

// But if it's not a virtual environment...
fs_err::remove_dir_all(context.temp_dir.join(".venv"))?;
uv_snapshot!(context.filters(), context.venv().arg("--python").arg("3.11"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: Ignoring existing virtual environment with missing Python interpreter: .venv/[BIN]/python
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Removed virtual environment at: .venv
Using Python 3.11.[X] interpreter at: [PYTHON-3.11]
Creating virtual environment at: .venv
Resolved 2 packages in [TIME]
Installed 1 package in [TIME]
+ iniconfig==2.0.0
Activate with: source .venv/[BIN]/activate
"###);

// Which we detect by the presence of a `pyvenv.cfg` file
fs_err::remove_file(context.temp_dir.join(".venv").join("pyvenv.cfg"))?;

// Let's make sure some extraneous content isn't removed
fs_err::write(context.temp_dir.join(".venv").join("file"), b"")?;

// We should never delete it
uv_snapshot!(context.filters(), context.sync(), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
error: Project virtual environment directory `[VENV]/` cannot be used because it is not a compatible environment but cannot be recreated because it is not a virtual environment
"###);

context
.temp_dir
.child(".venv")
.assert(predicate::path::is_dir());

context
.temp_dir
.child(".venv")
.child("file")
.assert(predicate::path::is_file());

Ok(())
}
2 changes: 1 addition & 1 deletion crates/uv/tests/tool_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ fn tool_list_bad_environment() -> Result<()> {
- ruff
----- stderr -----
Python interpreter not found at `[TEMP_DIR]/tools/black/[BIN]/python`
Invalid environment at `tools/black`: missing Python executable at `tools/black/[BIN]/python`
"###
);

Expand Down

0 comments on commit efefa59

Please sign in to comment.