Skip to content

Commit

Permalink
Auto merge of #13754 - epage:resolve-type, r=weihanglo
Browse files Browse the repository at this point in the history
feat(resolve): Tell the user the style of resovle done

### What does this PR try to resolve?

This is to help with #9930

Example changes:
```diff
-[LOCKING] 4 packages
+[LOCKING] 4 packages to latest compatible version
-[LOCKING] 2 packages
+[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
-[LOCKING] 2 packages
+[LOCKING] 2 packages to earliest compatible versions
```

Benefits
- The package count is of "added" packages and this makes that more
  logically clear
- This gives users transparency into what is happening, especially with
  - what rust-version is use
  - the transition to this feature in the new edition
  - whether the planned config was applied or not (as I don't want it to
    require an MSRV bump)
- Will make it easier in tests to show what changed
- Provides more motiviation to show this message in `cargo update` and
  `cargo install` (that will be explored in a follow up PR)

This does come at the cost of more verbose output but hopefully not too
verbose.  This is why I left off other factors, like avoid-dev-deps.

### How should we test and review this PR?

### Additional information
  • Loading branch information
bors committed Apr 15, 2024
2 parents 624233b + 1876326 commit 9f8adff
Show file tree
Hide file tree
Showing 166 changed files with 613 additions and 541 deletions.
96 changes: 68 additions & 28 deletions src/cargo/ops/cargo_generate_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> {
true,
)?;
ops::write_pkg_lockfile(ws, &mut resolve)?;
print_lockfile_changes(ws.gctx(), previous_resolve, &resolve, &mut registry)?;
print_lockfile_changes(ws, previous_resolve, &resolve, &mut registry)?;
Ok(())
}

Expand Down Expand Up @@ -170,7 +170,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
true,
)?;

print_lockfile_updates(opts.gctx, &previous_resolve, &resolve, &mut registry)?;
print_lockfile_updates(ws, &previous_resolve, &resolve, &mut registry)?;
if opts.dry_run {
opts.gctx
.shell()
Expand All @@ -186,21 +186,23 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
/// This would acquire the package-cache lock, as it may update the index to
/// show users latest available versions.
pub fn print_lockfile_changes(
gctx: &GlobalContext,
ws: &Workspace<'_>,
previous_resolve: Option<&Resolve>,
resolve: &Resolve,
registry: &mut PackageRegistry<'_>,
) -> CargoResult<()> {
let _lock = gctx.acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?;
let _lock = ws
.gctx()
.acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?;
if let Some(previous_resolve) = previous_resolve {
print_lockfile_sync(gctx, previous_resolve, resolve, registry)
print_lockfile_sync(ws, previous_resolve, resolve, registry)
} else {
print_lockfile_generation(gctx, resolve, registry)
print_lockfile_generation(ws, resolve, registry)
}
}

fn print_lockfile_generation(
gctx: &GlobalContext,
ws: &Workspace<'_>,
resolve: &Resolve,
registry: &mut PackageRegistry<'_>,
) -> CargoResult<()> {
Expand All @@ -210,8 +212,7 @@ fn print_lockfile_generation(
// just ourself, nothing worth reporting
return Ok(());
}
gctx.shell()
.status("Locking", format!("{num_pkgs} packages"))?;
status_locking(ws, num_pkgs)?;

for diff in diff {
fn format_latest(version: semver::Version) -> String {
Expand Down Expand Up @@ -245,7 +246,7 @@ fn print_lockfile_generation(
};

if let Some(latest) = latest {
gctx.shell().status_with_color(
ws.gctx().shell().status_with_color(
"Adding",
format!("{package}{latest}"),
&style::NOTE,
Expand All @@ -258,7 +259,7 @@ fn print_lockfile_generation(
}

fn print_lockfile_sync(
gctx: &GlobalContext,
ws: &Workspace<'_>,
previous_resolve: &Resolve,
resolve: &Resolve,
registry: &mut PackageRegistry<'_>,
Expand All @@ -268,9 +269,7 @@ fn print_lockfile_sync(
if num_pkgs == 0 {
return Ok(());
}
let plural = if num_pkgs == 1 { "" } else { "s" };
gctx.shell()
.status("Locking", format!("{num_pkgs} package{plural}"))?;
status_locking(ws, num_pkgs)?;

for diff in diff {
fn format_latest(version: semver::Version) -> String {
Expand Down Expand Up @@ -318,10 +317,12 @@ fn print_lockfile_sync(
// This metadata is often stuff like git commit hashes, which are
// not meaningfully ordered.
if removed.version().cmp_precedence(added.version()) == Ordering::Greater {
gctx.shell()
ws.gctx()
.shell()
.status_with_color("Downgrading", msg, &style::WARN)?;
} else {
gctx.shell()
ws.gctx()
.shell()
.status_with_color("Updating", msg, &style::GOOD)?;
}
} else {
Expand All @@ -339,7 +340,7 @@ fn print_lockfile_sync(
}
.unwrap_or_default();

gctx.shell().status_with_color(
ws.gctx().shell().status_with_color(
"Adding",
format!("{package}{latest}"),
&style::NOTE,
Expand All @@ -352,7 +353,7 @@ fn print_lockfile_sync(
}

fn print_lockfile_updates(
gctx: &GlobalContext,
ws: &Workspace<'_>,
previous_resolve: &Resolve,
resolve: &Resolve,
registry: &mut PackageRegistry<'_>,
Expand Down Expand Up @@ -404,16 +405,21 @@ fn print_lockfile_updates(
// This metadata is often stuff like git commit hashes, which are
// not meaningfully ordered.
if removed.version().cmp_precedence(added.version()) == Ordering::Greater {
gctx.shell()
ws.gctx()
.shell()
.status_with_color("Downgrading", msg, &style::WARN)?;
} else {
gctx.shell()
ws.gctx()
.shell()
.status_with_color("Updating", msg, &style::GOOD)?;
}
} else {
for package in diff.removed.iter() {
gctx.shell()
.status_with_color("Removing", format!("{package}"), &style::ERROR)?;
ws.gctx().shell().status_with_color(
"Removing",
format!("{package}"),
&style::ERROR,
)?;
}
for package in diff.added.iter() {
let latest = if !possibilities.is_empty() {
Expand All @@ -429,7 +435,7 @@ fn print_lockfile_updates(
}
.unwrap_or_default();

gctx.shell().status_with_color(
ws.gctx().shell().status_with_color(
"Adding",
format!("{package}{latest}"),
&style::NOTE,
Expand All @@ -451,8 +457,8 @@ fn print_lockfile_updates(

if let Some(latest) = latest {
unchanged_behind += 1;
if gctx.shell().verbosity() == Verbosity::Verbose {
gctx.shell().status_with_color(
if ws.gctx().shell().verbosity() == Verbosity::Verbose {
ws.gctx().shell().status_with_color(
"Unchanged",
format!("{package}{latest}"),
&anstyle::Style::new().bold(),
Expand All @@ -462,13 +468,13 @@ fn print_lockfile_updates(
}
}

if gctx.shell().verbosity() == Verbosity::Verbose {
gctx.shell().note(
if ws.gctx().shell().verbosity() == Verbosity::Verbose {
ws.gctx().shell().note(
"to see how you depend on a package, run `cargo tree --invert --package <dep>@<ver>`",
)?;
} else {
if 0 < unchanged_behind {
gctx.shell().note(format!(
ws.gctx().shell().note(format!(
"pass `--verbose` to see {unchanged_behind} unchanged dependencies behind latest"
))?;
}
Expand All @@ -477,6 +483,40 @@ fn print_lockfile_updates(
Ok(())
}

fn status_locking(ws: &Workspace<'_>, num_pkgs: usize) -> CargoResult<()> {
use std::fmt::Write as _;

let plural = if num_pkgs == 1 { "" } else { "s" };

let mut cfg = String::new();
// Don't have a good way to describe `direct_minimal_versions` atm
if !ws.gctx().cli_unstable().direct_minimal_versions {
write!(&mut cfg, " to")?;
if ws.gctx().cli_unstable().minimal_versions {
write!(&mut cfg, " earliest")?;
} else {
write!(&mut cfg, " latest")?;
}

if ws.resolve_honors_rust_version() {
let rust_version = if let Some(ver) = ws.rust_version() {
ver.clone().into_partial()
} else {
let rustc = ws.gctx().load_global_rustc(Some(ws))?;
let rustc_version = rustc.version.clone().into();
rustc_version
};
write!(&mut cfg, " Rust {rust_version}")?;
}
write!(&mut cfg, " compatible version{plural}")?;
}

ws.gctx()
.shell()
.status("Locking", format!("{num_pkgs} package{plural}{cfg}"))?;
Ok(())
}

fn is_latest(candidate: &semver::Version, current: &semver::Version) -> bool {
current < candidate
// Only match pre-release if major.minor.patch are the same
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ fn resolve_with_registry<'gctx>(
false
};
if print {
ops::print_lockfile_changes(ws.gctx(), prev.as_ref(), &resolve, registry)?;
ops::print_lockfile_changes(ws, prev.as_ref(), &resolve, registry)?;
}
Ok(resolve)
}
Expand Down
18 changes: 9 additions & 9 deletions tests/testsuite/alt_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn depend_on_alt_registry() {
.with_stderr(
"\
[UPDATING] `alternative` index
[LOCKING] 2 packages
[LOCKING] 2 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] bar v0.0.1 (registry `alternative`)
[CHECKING] bar v0.0.1 (registry `alternative`)
Expand Down Expand Up @@ -88,7 +88,7 @@ fn depend_on_alt_registry_depends_on_same_registry_no_index() {
.with_stderr(
"\
[UPDATING] `alternative` index
[LOCKING] 3 packages
[LOCKING] 3 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] [..] v0.0.1 (registry `alternative`)
[DOWNLOADED] [..] v0.0.1 (registry `alternative`)
Expand Down Expand Up @@ -132,7 +132,7 @@ fn depend_on_alt_registry_depends_on_same_registry() {
.with_stderr(
"\
[UPDATING] `alternative` index
[LOCKING] 3 packages
[LOCKING] 3 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] [..] v0.0.1 (registry `alternative`)
[DOWNLOADED] [..] v0.0.1 (registry `alternative`)
Expand Down Expand Up @@ -177,7 +177,7 @@ fn depend_on_alt_registry_depends_on_crates_io() {
"\
[UPDATING] `alternative` index
[UPDATING] `dummy-registry` index
[LOCKING] 3 packages
[LOCKING] 3 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] baz v0.0.1 (registry `dummy-registry`)
[DOWNLOADED] bar v0.0.1 (registry `alternative`)
Expand Down Expand Up @@ -217,7 +217,7 @@ fn registry_and_path_dep_works() {
p.cargo("check")
.with_stderr(
"\
[LOCKING] 2 packages
[LOCKING] 2 packages to latest compatible versions
[CHECKING] bar v0.0.1 ([CWD]/bar)
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]s
Expand Down Expand Up @@ -421,7 +421,7 @@ fn alt_registry_and_crates_io_deps() {
"\
[UPDATING] `alternative` index
[UPDATING] `dummy-registry` index
[LOCKING] 3 packages
[LOCKING] 3 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] crates_io_dep v0.0.1 (registry `dummy-registry`)
[DOWNLOADED] alt_reg_dep v0.1.0 (registry `alternative`)
Expand Down Expand Up @@ -698,7 +698,7 @@ fn patch_alt_reg() {
.with_stderr(
"\
[UPDATING] `alternative` index
[LOCKING] 2 packages
[LOCKING] 2 packages to latest compatible versions
[CHECKING] bar v0.1.0 ([CWD]/bar)
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
Expand Down Expand Up @@ -791,7 +791,7 @@ fn no_api() {
.with_stderr(
"\
[UPDATING] `alternative` index
[LOCKING] 2 packages
[LOCKING] 2 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] bar v0.0.1 (registry `alternative`)
[CHECKING] bar v0.0.1 (registry `alternative`)
Expand Down Expand Up @@ -1354,7 +1354,7 @@ fn registries_index_relative_url() {
.with_stderr(
"\
[UPDATING] `relative` index
[LOCKING] 2 packages
[LOCKING] 2 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] bar v0.0.1 (registry `relative`)
[CHECKING] bar v0.0.1 (registry `relative`)
Expand Down
Loading

0 comments on commit 9f8adff

Please sign in to comment.