Rename --force to --allow-existing (#3416)

## Summary

We've found `--force` to be a confusing name here. (Not breaking as this
hasn't shipped in a release.)
This commit is contained in:
Charlie Marsh 2024-05-07 13:11:31 -04:00 committed by GitHub
parent f8901e9989
commit f8a7813add
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 41 additions and 27 deletions

View file

@ -93,13 +93,12 @@ pub fn replace_symlink(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> std::io:
match std::os::unix::fs::symlink(src.as_ref(), dst.as_ref()) { match std::os::unix::fs::symlink(src.as_ref(), dst.as_ref()) {
Ok(()) => Ok(()), Ok(()) => Ok(()),
Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
// Create a symlink to the directory store, using a temporary file to ensure atomicity. // Create a symlink, using a temporary file to ensure atomicity.
let temp_dir = let temp_dir = tempfile::tempdir_in(dst.as_ref().parent().unwrap())?;
tempfile::tempdir_in(dst.as_ref().parent().expect("Cache entry to have parent"))?;
let temp_file = temp_dir.path().join("link"); let temp_file = temp_dir.path().join("link");
std::os::unix::fs::symlink(src, &temp_file)?; std::os::unix::fs::symlink(src, &temp_file)?;
// Move the symlink into the wheel cache. // Move the symlink into the target location.
fs_err::rename(&temp_file, dst.as_ref())?; fs_err::rename(&temp_file, dst.as_ref())?;
Ok(()) Ok(())

View file

@ -49,7 +49,7 @@ pub fn create_bare_venv(
interpreter: &Interpreter, interpreter: &Interpreter,
prompt: Prompt, prompt: Prompt,
system_site_packages: bool, system_site_packages: bool,
force: bool, allow_existing: bool,
) -> Result<VirtualEnvironment, Error> { ) -> Result<VirtualEnvironment, Error> {
// Determine the base Python executable; that is, the Python executable that should be // Determine the base Python executable; that is, the Python executable that should be
// considered the "base" for the virtual environment. This is typically the Python executable // considered the "base" for the virtual environment. This is typically the Python executable
@ -91,8 +91,8 @@ pub fn create_bare_venv(
format!("File exists at `{}`", location.user_display()), format!("File exists at `{}`", location.user_display()),
))); )));
} else if metadata.is_dir() { } else if metadata.is_dir() {
if force { if allow_existing {
info!("Overwriting existing directory"); info!("Allowing existing directory");
} else if location.join("pyvenv.cfg").is_file() { } else if location.join("pyvenv.cfg").is_file() {
info!("Removing existing directory"); info!("Removing existing directory");
fs::remove_dir_all(location)?; fs::remove_dir_all(location)?;

View file

@ -51,10 +51,16 @@ pub fn create_venv(
interpreter: Interpreter, interpreter: Interpreter,
prompt: Prompt, prompt: Prompt,
system_site_packages: bool, system_site_packages: bool,
force: bool, allow_existing: bool,
) -> Result<PythonEnvironment, Error> { ) -> Result<PythonEnvironment, Error> {
// Create the virtualenv at the given location. // Create the virtualenv at the given location.
let virtualenv = create_bare_venv(location, &interpreter, prompt, system_site_packages, force)?; let virtualenv = create_bare_venv(
location,
&interpreter,
prompt,
system_site_packages,
allow_existing,
)?;
// Create the corresponding `PythonEnvironment`. // Create the corresponding `PythonEnvironment`.
let interpreter = interpreter.with_virtualenv(virtualenv); let interpreter = interpreter.with_virtualenv(virtualenv);

View file

@ -1706,14 +1706,17 @@ pub(crate) struct VenvArgs {
#[arg(long)] #[arg(long)]
pub(crate) seed: bool, pub(crate) seed: bool,
/// Overwrite the directory at the specified path when creating the virtual environment. /// Preserve any existing files or directories at the target path.
/// ///
/// By default, `uv venv` will remove an existing virtual environment at the given path, and /// By default, `uv venv` will remove an existing virtual environment at the given path, and
/// exit with an error if the path is non-empty but _not_ a virtual environment. The `--force` /// exit with an error if the path is non-empty but _not_ a virtual environment. The
/// option will instead write to the given path, regardless of its contents, and without /// `--allow-existing` option will instead write to the given path, regardless of its contents,
/// clearing it beforehand. /// and without clearing it beforehand.
///
/// WARNING: This option can lead to unexpected behavior if the existing virtual environment
/// and the newly-created virtual environment are linked to different Python interpreters.
#[clap(long)] #[clap(long)]
pub(crate) force: bool, pub(crate) allow_existing: bool,
/// The path to the virtual environment to create. /// The path to the virtual environment to create.
#[arg(default_value = ".venv")] #[arg(default_value = ".venv")]

View file

@ -44,7 +44,7 @@ pub(crate) async fn venv(
system_site_packages: bool, system_site_packages: bool,
connectivity: Connectivity, connectivity: Connectivity,
seed: bool, seed: bool,
force: bool, allow_existing: bool,
exclude_newer: Option<ExcludeNewer>, exclude_newer: Option<ExcludeNewer>,
native_tls: bool, native_tls: bool,
cache: &Cache, cache: &Cache,
@ -61,7 +61,7 @@ pub(crate) async fn venv(
system_site_packages, system_site_packages,
connectivity, connectivity,
seed, seed,
force, allow_existing,
exclude_newer, exclude_newer,
native_tls, native_tls,
cache, cache,
@ -109,7 +109,7 @@ async fn venv_impl(
system_site_packages: bool, system_site_packages: bool,
connectivity: Connectivity, connectivity: Connectivity,
seed: bool, seed: bool,
force: bool, allow_existing: bool,
exclude_newer: Option<ExcludeNewer>, exclude_newer: Option<ExcludeNewer>,
native_tls: bool, native_tls: bool,
cache: &Cache, cache: &Cache,
@ -146,7 +146,13 @@ async fn venv_impl(
.into_diagnostic()?; .into_diagnostic()?;
// Create the virtual environment. // Create the virtual environment.
let venv = uv_virtualenv::create_venv(path, interpreter, prompt, system_site_packages, force) let venv = uv_virtualenv::create_venv(
path,
interpreter,
prompt,
system_site_packages,
allow_existing,
)
.map_err(VenvError::Creation)?; .map_err(VenvError::Creation)?;
// Install seed packages. // Install seed packages.

View file

@ -484,7 +484,7 @@ async fn run() -> Result<ExitStatus> {
args.system_site_packages, args.system_site_packages,
args.shared.connectivity, args.shared.connectivity,
args.seed, args.seed,
args.force, args.allow_existing,
args.shared.exclude_newer, args.shared.exclude_newer,
globals.native_tls, globals.native_tls,
&cache, &cache,

View file

@ -747,7 +747,7 @@ impl PipCheckSettings {
pub(crate) struct VenvSettings { pub(crate) struct VenvSettings {
// CLI-only settings. // CLI-only settings.
pub(crate) seed: bool, pub(crate) seed: bool,
pub(crate) force: bool, pub(crate) allow_existing: bool,
pub(crate) name: PathBuf, pub(crate) name: PathBuf,
pub(crate) prompt: Option<String>, pub(crate) prompt: Option<String>,
pub(crate) system_site_packages: bool, pub(crate) system_site_packages: bool,
@ -764,7 +764,7 @@ impl VenvSettings {
system, system,
no_system, no_system,
seed, seed,
force, allow_existing,
name, name,
prompt, prompt,
system_site_packages, system_site_packages,
@ -783,7 +783,7 @@ impl VenvSettings {
Self { Self {
// CLI-only settings. // CLI-only settings.
seed, seed,
force, allow_existing,
name, name,
prompt, prompt,
system_site_packages, system_site_packages,

View file

@ -381,11 +381,11 @@ fn non_empty_dir_exists() -> Result<()> {
} }
#[test] #[test]
fn non_empty_dir_exists_force() -> Result<()> { fn non_empty_dir_exists_allow_existing() -> Result<()> {
let context = VenvTestContext::new(&["3.12"]); let context = VenvTestContext::new(&["3.12"]);
// Create a non-empty directory at `.venv`. Creating a virtualenv at the same path should // Create a non-empty directory at `.venv`. Creating a virtualenv at the same path should
// succeed when `--force` is specified, but fail when it is not. // succeed when `--allow-existing` is specified, but fail when it is not.
context.venv.create_dir_all()?; context.venv.create_dir_all()?;
context.venv.child("file").touch()?; context.venv.child("file").touch()?;
@ -409,7 +409,7 @@ fn non_empty_dir_exists_force() -> Result<()> {
uv_snapshot!(context.filters(), context.venv_command() uv_snapshot!(context.filters(), context.venv_command()
.arg(context.venv.as_os_str()) .arg(context.venv.as_os_str())
.arg("--force") .arg("--allow-existing")
.arg("--python") .arg("--python")
.arg("3.12"), @r###" .arg("3.12"), @r###"
success: true success: true
@ -427,7 +427,7 @@ fn non_empty_dir_exists_force() -> Result<()> {
// directories. // directories.
uv_snapshot!(context.filters(), context.venv_command() uv_snapshot!(context.filters(), context.venv_command()
.arg(context.venv.as_os_str()) .arg(context.venv.as_os_str())
.arg("--force") .arg("--allow-existing")
.arg("--python") .arg("--python")
.arg("3.12"), @r###" .arg("3.12"), @r###"
success: true success: true