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

Rework --clear flag for venv command #1824

Closed
wants to merge 1 commit into from

Conversation

flyaroundme
Copy link
Contributor

@flyaroundme flyaroundme commented Feb 21, 2024

Summary

Reworked the --clear flag for venv command for it not to recreate the existing virtualenv unless --clear flag is set.

Based on this discussion #1472

Test Plan

Create venv using uv venv command. Then use uv venv command again without flag. Check that virtualenv is not cleared. Then use uv venv --clear, check that it is recreated.

$ uv venv
Using Python 3.10.12 interpreter at /usr/bin/python3.10
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate
$ uv pip install ruff # fill virtualenv with something
Resolved 1 package in 141ms
Installed 1 package in 21ms
 + ruff==0.2.2
$ uv pip freeze # memorize the previous state of virtualenv
ruff==0.2.2
$ uv venv # without flag
Using Python 3.10.12 interpreter at /usr/bin/python3.10
Virtualenv already exists at .venv. Use --clear option to recreate
$ uv pip freeze # check that virtualenv is not cleared
ruff==0.2.2
$ uv venv --clear # with flag
Using Python 3.10.12 interpreter at /usr/bin/python3.10
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate
$ uv pip freeze # check virtualenv is cleared
$

Now venv command does not recreate the existing virtualenv unless --clear flag is set
@flyaroundme flyaroundme marked this pull request as draft February 21, 2024 18:50
Comment on lines +117 to +126
if path.exists() && !clear {
writeln!(
printer,
"Virtualenv already exists at {}. Use --clear option to recreate",
path.normalized_display().cyan()
)
.into_diagnostic()?;
return Ok(ExitStatus::Success);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather to fail with non-zero exit code.
Also, message could be more strict since the command failing:

Failed to create virtualenv at '{}': Already exists.
hint: Use --clear option to force recreate new virtualenv at {}

@T-256
Copy link
Contributor

T-256 commented Feb 25, 2024

IMO, We could also consider --force instead of --clear to also fix #1863 where target path is exists but is not venv.

@zanieb
Copy link
Member

zanieb commented Apr 22, 2024

Quite a bit of discussion about this over in #2548

@flyaroundme flyaroundme closed this May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants