Make --reinstall imply --refresh (#5425)

## Summary

It's hard for me to imagine a scenario in which a user passed
`--reinstall`, but wanted us to keep respecting cached data for a
package. For example, to actually "rebuild and reinstall" an editable
today, you have to pass both `--reinstall` and `--refresh`.

This PR makes `--reinstall` imply `--refresh`, so we always validate
that the cached data is fresh.

Closes https://github.com/astral-sh/uv/issues/5424.
This commit is contained in:
Charlie Marsh 2024-07-25 09:45:58 -04:00 committed by GitHub
parent 4d9098a1d7
commit d0919329fd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 114 additions and 23 deletions

View file

@ -2548,14 +2548,16 @@ pub struct InstallerArgs {
#[command(flatten)]
pub index_args: IndexArgs,
/// Reinstall all packages, regardless of whether they're already installed.
/// Reinstall all packages, regardless of whether they're already installed. Implies
/// `--refresh`.
#[arg(long, alias = "force-reinstall", overrides_with("no_reinstall"))]
pub reinstall: bool,
#[arg(long, overrides_with("reinstall"), hide = true)]
pub no_reinstall: bool,
/// Reinstall a specific package, regardless of whether it's already installed.
/// Reinstall a specific package, regardless of whether it's already installed. Implies
/// `--refresh-package`.
#[arg(long)]
pub reinstall_package: Vec<PackageName>,
@ -2712,14 +2714,16 @@ pub struct ResolverInstallerArgs {
#[arg(long, short = 'P')]
pub upgrade_package: Vec<Requirement<VerbatimParsedUrl>>,
/// Reinstall all packages, regardless of whether they're already installed.
/// Reinstall all packages, regardless of whether they're already installed. Implies
/// `--refresh`.
#[arg(long, alias = "force-reinstall", overrides_with("no_reinstall"))]
pub reinstall: bool,
#[arg(long, overrides_with("reinstall"), hide = true)]
pub no_reinstall: bool,
/// Reinstall a specific package, regardless of whether it's already installed.
/// Reinstall a specific package, regardless of whether it's already installed. Implies
/// `--refresh-package`.
#[arg(long)]
pub reinstall_package: Vec<PackageName>,

View file

@ -17,6 +17,7 @@ pep508_rs = { workspace = true, features = ["schemars"] }
platform-tags = { workspace = true }
pypi-types = { workspace = true }
uv-auth = { workspace = true }
uv-cache = { workspace = true }
uv-normalize = { workspace = true }
clap = { workspace = true, features = ["derive"], optional = true }

View file

@ -3,6 +3,7 @@ use pep508_rs::PackageName;
use pypi_types::Requirement;
use rustc_hash::FxHashMap;
use uv_cache::Refresh;
/// Whether to reinstall packages.
#[derive(Debug, Default, Clone)]
@ -43,6 +44,32 @@ impl Reinstall {
pub fn is_all(&self) -> bool {
matches!(self, Self::All)
}
/// Create a [`Refresh`] policy by integrating the [`Reinstall`] policy.
pub fn to_refresh(self, refresh: Refresh) -> Refresh {
match (self, refresh) {
// If the policy is `None`, return the existing refresh policy.
(Self::None, Refresh::None(timestamp)) => Refresh::None(timestamp),
(Self::None, Refresh::All(timestamp)) => Refresh::All(timestamp),
(Self::None, Refresh::Packages(packages, timestamp)) => {
Refresh::Packages(packages, timestamp)
}
// If the policy is `All`, refresh all packages.
(Self::All, Refresh::None(timestamp)) => Refresh::All(timestamp),
(Self::All, Refresh::All(timestamp)) => Refresh::All(timestamp),
(Self::All, Refresh::Packages(_packages, timestamp)) => Refresh::All(timestamp),
// If the policy is `Packages`, take the "max" of the two policies.
(Self::Packages(packages), Refresh::None(timestamp)) => {
Refresh::Packages(packages, timestamp)
}
(Self::Packages(_packages), Refresh::All(timestamp)) => Refresh::All(timestamp),
(Self::Packages(packages1), Refresh::Packages(packages2, timestamp)) => {
Refresh::Packages(packages1.into_iter().chain(packages2).collect(), timestamp)
}
}
}
}
/// Whether to allow package upgrades.

View file

@ -380,7 +380,7 @@ pub struct ResolverInstallerOptions {
"#
)]
pub upgrade_package: Option<Vec<Requirement<VerbatimParsedUrl>>>,
/// Reinstall all packages, regardless of whether they're already installed.
/// Reinstall all packages, regardless of whether they're already installed. Implies `refresh`.
#[option(
default = "false",
value_type = "bool",
@ -389,7 +389,8 @@ pub struct ResolverInstallerOptions {
"#
)]
pub reinstall: Option<bool>,
/// Reinstall a specific package, regardless of whether it's already installed.
/// Reinstall a specific package, regardless of whether it's already installed. Implies
/// `refresh-package`.
#[option(
default = "[]",
value_type = "list[str]",
@ -1056,7 +1057,7 @@ pub struct PipOptions {
"#
)]
pub upgrade_package: Option<Vec<Requirement<VerbatimParsedUrl>>>,
/// Reinstall all packages, regardless of whether they're already installed.
/// Reinstall all packages, regardless of whether they're already installed. Implies `refresh`.
#[option(
default = "false",
value_type = "bool",
@ -1065,7 +1066,8 @@ pub struct PipOptions {
"#
)]
pub reinstall: Option<bool>,
/// Reinstall a specific package, regardless of whether it's already installed.
/// Reinstall a specific package, regardless of whether it's already installed. Implies
/// `refresh-package`.
#[option(
default = "[]",
value_type = "list[str]",

View file

@ -179,6 +179,7 @@ async fn run(cli: Cli) -> Result<ExitStatus> {
.expect("failed to initialize global rayon pool");
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let requirements = args
@ -262,7 +263,9 @@ async fn run(cli: Cli) -> Result<ExitStatus> {
.expect("failed to initialize global rayon pool");
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let cache = cache
.init()?
.with_refresh(args.settings.reinstall.clone().to_refresh(args.refresh));
let requirements = args
.src_file
@ -324,7 +327,10 @@ async fn run(cli: Cli) -> Result<ExitStatus> {
.expect("failed to initialize global rayon pool");
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let cache = cache
.init()?
.with_refresh(args.settings.reinstall.clone().to_refresh(args.refresh));
let requirements = args
.package
.into_iter()
@ -880,7 +886,9 @@ async fn run_project(
show_settings!(args);
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let cache = cache
.init()?
.with_refresh(args.settings.reinstall.clone().to_refresh(args.refresh));
let requirements = args
.with
@ -921,7 +929,9 @@ async fn run_project(
show_settings!(args);
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let cache = cache
.init()?
.with_refresh(args.settings.reinstall.clone().to_refresh(args.refresh));
commands::sync(
args.locked,
@ -948,7 +958,7 @@ async fn run_project(
show_settings!(args);
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let cache = cache.init()?;
commands::lock(
args.locked,
@ -972,7 +982,9 @@ async fn run_project(
show_settings!(args);
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let cache = cache
.init()?
.with_refresh(args.settings.reinstall.clone().to_refresh(args.refresh));
commands::add(
args.locked,
@ -1005,7 +1017,9 @@ async fn run_project(
show_settings!(args);
// Initialize the cache.
let cache = cache.init()?.with_refresh(args.refresh);
let cache = cache
.init()?
.with_refresh(args.settings.reinstall.clone().to_refresh(args.refresh));
commands::remove(
args.locked,

View file

@ -585,6 +585,7 @@ fn respect_installed_and_reinstall() -> Result<()> {
----- stderr -----
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Uninstalled [N] packages in [TIME]
Installed [N] packages in [TIME]
- flask==3.0.2
@ -1816,6 +1817,7 @@ fn reinstall_no_binary() {
----- stderr -----
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Uninstalled [N] packages in [TIME]
Installed [N] packages in [TIME]
- anyio==4.3.0
@ -4972,6 +4974,7 @@ fn already_installed_remote_url() {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389)

View file

@ -1962,6 +1962,7 @@ fn reinstall() -> Result<()> {
----- stderr -----
Resolved 2 packages in [TIME]
Prepared 2 packages in [TIME]
Uninstalled 2 packages in [TIME]
Installed 2 packages in [TIME]
- markupsafe==2.1.3
@ -2016,6 +2017,7 @@ fn reinstall_package() -> Result<()> {
----- stderr -----
Resolved 2 packages in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- tomli==2.0.1
@ -2069,6 +2071,7 @@ fn reinstall_git() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389)
@ -2340,6 +2343,31 @@ fn sync_editable() -> Result<()> {
"###
);
// Modify the `pyproject.toml` file.
let pyproject_toml = poetry_editable.path().join("pyproject.toml");
let pyproject_toml_contents = fs_err::read_to_string(&pyproject_toml)?;
fs_err::write(
&pyproject_toml,
pyproject_toml_contents.replace("0.1.0", "0.1.1"),
)?;
// Reinstall the editable package. This will trigger a rebuild and reinstall.
uv_snapshot!(context.filters(), context.pip_sync()
.arg(requirements_txt.path()), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 3 packages in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- poetry-editable==0.1.0 (from file://[TEMP_DIR]/poetry_editable)
+ poetry-editable==0.1.1 (from file://[TEMP_DIR]/poetry_editable)
"###
);
Ok(())
}
@ -2781,6 +2809,7 @@ fn find_links_wheel_cache() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- tqdm==1000.0.0
@ -2831,6 +2860,7 @@ fn find_links_source_cache() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- tqdm==999.0.0
@ -3746,6 +3776,7 @@ fn require_hashes_source_url() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- source-distribution==0.0.1 (from https://files.pythonhosted.org/packages/10/1f/57aa4cce1b1abf6b433106676e15f9fa2c92ed2bd4cf77c3b50a9e9ac773/source_distribution-0.0.1.tar.gz)
@ -3847,6 +3878,7 @@ fn require_hashes_wheel_url() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- anyio==4.0.0 (from https://files.pythonhosted.org/packages/36/55/ad4de788d84a630656ece71059665e01ca793c04294c463fd84132f40fe6/anyio-4.0.0-py3-none-any.whl)
@ -4059,6 +4091,7 @@ fn require_hashes_re_download() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- anyio==4.0.0
@ -4459,6 +4492,7 @@ fn require_hashes_at_least_one() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- anyio==4.0.0
@ -4481,6 +4515,7 @@ fn require_hashes_at_least_one() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- anyio==4.0.0
@ -4716,6 +4751,7 @@ fn require_hashes_find_links_invalid_hash() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ example-a-961b4c22==1.0.0
"###
@ -4922,6 +4958,7 @@ fn require_hashes_registry_invalid_hash() -> Result<()> {
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ example-a-961b4c22==1.0.0
"###