Allow virtualenv creation at existing, empty directories (#1281)

## Summary

If the directory exists but is empty, we should allow `puffin venv`
without erroring.

Also adds test cases for a variety of error cases.
This commit is contained in:
Charlie Marsh 2024-02-11 22:13:13 -05:00 committed by GitHub
parent b7e3933fe7
commit 93b7a1140f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 210 additions and 10 deletions

View file

@ -61,18 +61,39 @@ pub fn create_bare_venv(location: &Utf8Path, interpreter: &Interpreter) -> io::R
let base_python: Utf8PathBuf = fs_err::canonicalize(interpreter.sys_executable())?
.try_into()
.map_err(|err: FromPathBufError| err.into_io_error())?;
if location.exists() {
if location.join("pyvenv.cfg").is_file() {
info!("Removing existing directory");
fs::remove_dir_all(location)?;
} else {
return Err(io::Error::new(
io::ErrorKind::AlreadyExists,
format!("The directory {location} exists, but it is not virtualenv"),
));
// Validate the existing location.
match location.metadata() {
Ok(metadata) => {
if metadata.is_file() {
return Err(io::Error::new(
io::ErrorKind::AlreadyExists,
format!("File exists at `{location}`"),
));
} else if metadata.is_dir() {
if location.join("pyvenv.cfg").is_file() {
info!("Removing existing directory");
fs::remove_dir_all(location)?;
fs::create_dir_all(location)?;
} else if location
.read_dir()
.is_ok_and(|mut dir| dir.next().is_none())
{
info!("Ignoring empty directory");
} else {
return Err(io::Error::new(
io::ErrorKind::AlreadyExists,
format!("The directory `{location}` exists, but it's not a virtualenv"),
));
}
}
}
Err(err) if err.kind() == io::ErrorKind::NotFound => {
fs::create_dir_all(location)?;
}
Err(err) => return Err(err),
}
fs::create_dir_all(location)?;
// TODO(konstin): I bet on windows we'll have to strip the prefix again
let location = location.canonicalize_utf8()?;
let bin_name = if cfg!(unix) {

View file

@ -18,6 +18,7 @@ fn create_venv() -> Result<()> {
let bin = create_bin_with_executables(&temp_dir, &["3.12"]).expect("Failed to create bin dir");
let venv = temp_dir.child(".venv");
// Create a virtual environment at `.venv`.
let filter_venv = regex::escape(&venv.normalized_display().to_string());
let filters = &[
(
@ -35,6 +36,39 @@ fn create_venv() -> Result<()> {
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_TEST_PYTHON_PATH", bin.clone())
.current_dir(&temp_dir), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Using Python [VERSION] interpreter at [PATH]
Creating virtualenv at: /home/ferris/project/.venv
"###
);
venv.assert(predicates::path::is_dir());
// Create a virtual environment at the same location, which should replace it.
let filter_venv = regex::escape(&venv.normalized_display().to_string());
let filters = &[
(
r"Using Python 3\.\d+\.\d+ interpreter at .+",
"Using Python [VERSION] interpreter at [PATH]",
),
(&filter_venv, "/home/ferris/project/.venv"),
];
puffin_snapshot!(filters, Command::new(get_bin())
.arg("venv")
.arg(venv.as_os_str())
.arg("--python")
.arg("3.12")
.arg("--cache-dir")
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: true
@ -75,6 +109,7 @@ fn create_venv_defaults_to_cwd() -> Result<()> {
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: true
@ -117,6 +152,7 @@ fn seed() -> Result<()> {
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: true
@ -154,6 +190,7 @@ fn create_venv_unknown_python_minor() -> Result<()> {
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir);
if cfg!(windows) {
@ -208,6 +245,7 @@ fn create_venv_unknown_python_patch() -> Result<()> {
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: false
@ -248,6 +286,7 @@ fn create_venv_python_patch() -> Result<()> {
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: true
@ -264,3 +303,143 @@ fn create_venv_python_patch() -> Result<()> {
Ok(())
}
#[test]
fn file_exists() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let bin = create_bin_with_executables(&temp_dir, &["3.12"]).expect("Failed to create bin dir");
let venv = temp_dir.child(".venv");
// Create a file at `.venv`. Creating a virtualenv at the same path should fail.
venv.touch()?;
let filter_venv = regex::escape(&venv.normalized_display().to_string());
let filters = &[
(
r"Using Python 3\.\d+\.\d+ interpreter at .+",
"Using Python [VERSION] interpreter at [PATH]",
),
(&filter_venv, "/home/ferris/project/.venv"),
];
puffin_snapshot!(filters, Command::new(get_bin())
.arg("venv")
.arg(venv.as_os_str())
.arg("--python")
.arg("3.12")
.arg("--cache-dir")
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
Using Python [VERSION] interpreter at [PATH]
Creating virtualenv at: /home/ferris/project/.venv
puffin::venv::creation
× Failed to create virtualenv
File exists at `/home/ferris/project/.venv`
"###
);
Ok(())
}
#[test]
fn empty_dir_exists() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let bin = create_bin_with_executables(&temp_dir, &["3.12"]).expect("Failed to create bin dir");
let venv = temp_dir.child(".venv");
// Create an empty directory at `.venv`. Creating a virtualenv at the same path should succeed.
venv.create_dir_all()?;
let filter_venv = regex::escape(&venv.normalized_display().to_string());
let filters = &[
(
r"Using Python 3\.\d+\.\d+ interpreter at .+",
"Using Python [VERSION] interpreter at [PATH]",
),
(&filter_venv, "/home/ferris/project/.venv"),
];
puffin_snapshot!(filters, Command::new(get_bin())
.arg("venv")
.arg(venv.as_os_str())
.arg("--python")
.arg("3.12")
.arg("--cache-dir")
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Using Python [VERSION] interpreter at [PATH]
Creating virtualenv at: /home/ferris/project/.venv
"###
);
venv.assert(predicates::path::is_dir());
Ok(())
}
#[test]
fn non_empty_dir_exists() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let bin = create_bin_with_executables(&temp_dir, &["3.12"]).expect("Failed to create bin dir");
let venv = temp_dir.child(".venv");
// Create a non-empty directory at `.venv`. Creating a virtualenv at the same path should fail.
venv.create_dir_all()?;
venv.child("file").touch()?;
let filter_venv = regex::escape(&venv.normalized_display().to_string());
let filters = &[
(
r"Using Python 3\.\d+\.\d+ interpreter at .+",
"Using Python [VERSION] interpreter at [PATH]",
),
(&filter_venv, "/home/ferris/project/.venv"),
];
puffin_snapshot!(filters, Command::new(get_bin())
.arg("venv")
.arg(venv.as_os_str())
.arg("--python")
.arg("3.12")
.arg("--cache-dir")
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("PUFFIN_NO_WRAP", "1")
.env("PUFFIN_TEST_PYTHON_PATH", bin)
.current_dir(&temp_dir), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
Using Python [VERSION] interpreter at [PATH]
Creating virtualenv at: /home/ferris/project/.venv
puffin::venv::creation
× Failed to create virtualenv
The directory `/home/ferris/project/.venv` exists, but it's not a virtualenv
"###
);
Ok(())
}