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

Add --dry-run flag to uv pip install #1436

Merged
merged 23 commits into from
Mar 12, 2024

Conversation

JacobCoffee
Copy link
Contributor

@JacobCoffee JacobCoffee commented Feb 16, 2024

What

Adds a --dry-run flag that ejects out of the installation process early (but after resolution) and displays only what would have installed

Closes

Closes #1244

Out of Scope

I think it may be nice to include a dry-run flag for uninstall even though pip doesn't implement this... thinking Would uninstall X packages: ...

@JacobCoffee JacobCoffee marked this pull request as draft February 16, 2024 05:42
@JacobCoffee

This comment was marked as outdated.

@JacobCoffee
Copy link
Contributor Author

JacobCoffee commented Feb 16, 2024

Sorry if this is a bad way to get the version? It felt bad, anyway...
I'm not sure if we do or can store version at resolution time and use it in dry_run, install, validate, etc.

@JacobCoffee
Copy link
Contributor Author

Might be cool to have a dry-run flag for uninstalling although not native to pip

@JacobCoffee
Copy link
Contributor Author

On the failing CI, I had some failures do to order in stdout there... but I'm wondering how we keep it consistent?

@JacobCoffee JacobCoffee marked this pull request as ready for review February 16, 2024 07:59
@zanieb zanieb self-requested a review February 16, 2024 17:09
@zanieb zanieb self-assigned this Feb 16, 2024
@zanieb
Copy link
Member

zanieb commented Feb 17, 2024

Hey @JacobCoffee, thanks for the pull request! I think we'll actually want to implement this a bit differently e.g. by passing the dry run flag into install

async fn install(

then we can use our existing display which will also show which versions we would remove (and has a sorting implementation for deterministic output)

for event in reinstalls

We'll just need to tweak the install function to avoid doing any downloads, uninstalls, builds, or installs when the flag is set.

@zanieb zanieb marked this pull request as draft February 17, 2024 04:25
@JacobCoffee

This comment was marked as outdated.

@zanieb
Copy link
Member

zanieb commented Feb 22, 2024

I'll give this a try tomorrow!

@zanieb
Copy link
Member

zanieb commented Feb 23, 2024

Hey! I got a chance to play with this today. I'm thinking we should match our normal output much more closely.

Screenshot 2024-02-22 at 18 27 31

I put up a commit for you to build off of at #1890

We still need more test coverage i.e. install a dependency of a package first then dry run install what do we show? or install an old version then dry run install with --upgrade

@JacobCoffee
Copy link
Contributor Author

Just for my notes, i am running into an issue where in testing it passes with 2 $package==$version, but in CLI usage i get 4 ====

➜ ./target/debug/uv pip install litestar==2.0.0 --dry-run --strict
Resolved 17 packages in 974ms
Would download 2 packages
Would install 17 packages
 + anyio====4.3.0
 + litestar====2.0.0
 + faker====23.2.1
 + fast-query-parsers==1.0.3

But removing the == printer config from the +/- sections remove it from the testing :|

@zanieb
Copy link
Member

zanieb commented Feb 28, 2024

@JacobCoffee I'm on vacation this week but let me know when this is ready for review again

@JacobCoffee
Copy link
Contributor Author

sorry ive not wrapped this - i think my remaining issue is the 4 ====, but ive just had surgery so im down for a bit on my hard thinking 😅

@zanieb
Copy link
Member

zanieb commented Mar 3, 2024

I can give it a look! Get better :)

@zanieb
Copy link
Member

zanieb commented Mar 4, 2024

The issue is that InstalledVersion which comes from cached distributions includes == already (fixed by fee46b3).

@zanieb
Copy link
Member

zanieb commented Mar 6, 2024

I think this is vaguely ready for review

Comment on lines 734 to 749
// Nothing to do.
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() {
let s = if resolution.len() == 1 { "" } else { "s" };
writeln!(
printer,
"{}",
format!(
"Audited {} in {}",
format!("{} package{}", resolution.len(), s).bold(),
elapsed(start.elapsed())
)
.dimmed()
)?;
writeln!(printer, "Would make no changes")?;
return Ok(());
}
Copy link
Member

Choose a reason for hiding this comment

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

I believe this is redundant as it is handled before this function is called

@zanieb zanieb marked this pull request as ready for review March 7, 2024 17:43
@zanieb zanieb changed the title feat(pip-install): add --dry-run flag Add --dry-run flag to uv pip install Mar 12, 2024
@zanieb zanieb merged commit 15f6f9f into astral-sh:main Mar 12, 2024
7 checks passed
@zanieb zanieb added the enhancement New feature or request label Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add --dry-run flag for pip install
3 participants