Skip to content

Commit

Permalink
Auto merge of #13372 - epage:update-report, r=weihanglo
Browse files Browse the repository at this point in the history
feat(update): Tell users when they are still behind

### What does this PR try to resolve?

Part of this is an offshoot of #12425 which is about pulling  some of `cargo upgrade`s behavior into `cargo update`.  One of the "'Potential related `cargo update` improvements" is informing the user when they are behind.

Part of this is to help close the gap of users being behind on their dependencies unaware.  This is commonly raised when discussing an MSRV-aware resolver (see rust-lang/rfcs#3537) but breaking changes are just as big of a deal so I'm starting this now.

See also #7167, #4309

Compared to `cargo upgrade` / `cargo outdated`, I'm taking a fairly conservative approach and tweaking the existing output as a starting point / MVP.  We can experiment with a richer or easier-to-consume way of expressing this over time.

I view us telling people they aren't on the latest as a warning, so I made that text yellow.

`clap $ cargo update --dry-run`
![image](https://github.com/rust-lang/cargo/assets/60961/4bf151e3-6b57-4073-8822-9140dd731d5e)

`clap $ cargo update --dry-run --verbose`
![image](https://github.com/rust-lang/cargo/assets/60961/fbf802fb-3a6a-4e8b-a6ec-4ce49fb505f6)

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

This sets up the minimal implementation and slowly adds bits at a time, with a test first that demonstrates it.

### Additional information

I'm expecting that the `cargo upgrade` integration will extend the notes to say something like "X dependencies may be updated with `--breaking`"
  • Loading branch information
bors committed Feb 5, 2024
2 parents 7bc317b + 93e369a commit b0df265
Show file tree
Hide file tree
Showing 8 changed files with 301 additions and 48 deletions.
1 change: 1 addition & 0 deletions crates/cargo-test-support/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ fn substitute_macros(input: &str) -> String {
("[ADDING]", " Adding"),
("[REMOVING]", " Removing"),
("[REMOVED]", " Removed"),
("[UNCHANGED]", " Unchanged"),
("[DOCTEST]", " Doc-tests"),
("[PACKAGING]", " Packaging"),
("[PACKAGED]", " Packaged"),
Expand Down
210 changes: 164 additions & 46 deletions src/cargo/ops/cargo_generate_lockfile.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use crate::core::registry::PackageRegistry;
use crate::core::resolver::features::{CliFeatures, HasDevUnits};
use crate::core::shell::Verbosity;
use crate::core::Registry as _;
use crate::core::{PackageId, PackageIdSpec, PackageIdSpecQuery};
use crate::core::{Resolve, SourceId, Workspace};
use crate::ops;
use crate::sources::source::QueryKind;
use crate::util::cache_lock::CacheLockMode;
use crate::util::config::Config;
use crate::util::style;
Expand Down Expand Up @@ -161,36 +164,137 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
let print_change = |status: &str, msg: String, color: &Style| {
opts.config.shell().status_with_color(status, msg, color)
};
for (removed, added) in compare_dependency_graphs(&previous_resolve, &resolve) {
let mut unchanged_behind = 0;
for ResolvedPackageVersions {
removed,
added,
unchanged,
} in compare_dependency_graphs(&previous_resolve, &resolve)
{
fn format_latest(version: semver::Version) -> String {
let warn = style::WARN;
format!(" {warn}(latest: v{version}){warn:#}")
}
fn is_latest(candidate: &semver::Version, current: &semver::Version) -> bool {
current < candidate
// Only match pre-release if major.minor.patch are the same
&& (candidate.pre.is_empty()
|| (candidate.major == current.major
&& candidate.minor == current.minor
&& candidate.patch == current.patch))
}
let possibilities = if let Some(query) = [added.iter(), unchanged.iter()]
.into_iter()
.flatten()
.next()
.filter(|s| s.source_id().is_registry())
{
let query =
crate::core::dependency::Dependency::parse(query.name(), None, query.source_id())?;
loop {
match registry.query_vec(&query, QueryKind::Exact) {
std::task::Poll::Ready(res) => {
break res?;
}
std::task::Poll::Pending => registry.block_until_ready()?,
}
}
} else {
vec![]
};

if removed.len() == 1 && added.len() == 1 {
let msg = if removed[0].source_id().is_git() {
let added = added.into_iter().next().unwrap();
let removed = removed.into_iter().next().unwrap();

let latest = if !possibilities.is_empty() {
possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| is_latest(s.version(), added.version()))
.map(|s| s.version().clone())
.max()
.map(format_latest)
} else {
None
}
.unwrap_or_default();

let msg = if removed.source_id().is_git() {
format!(
"{} -> #{}",
removed[0],
&added[0].source_id().precise_git_fragment().unwrap()[..8],
"{removed} -> #{}",
&added.source_id().precise_git_fragment().unwrap()[..8],
)
} else {
format!("{} -> v{}", removed[0], added[0].version())
format!("{removed} -> v{}{latest}", added.version())
};

// If versions differ only in build metadata, we call it an "update"
// regardless of whether the build metadata has gone up or down.
// This metadata is often stuff like git commit hashes, which are
// not meaningfully ordered.
if removed[0].version().cmp_precedence(added[0].version()) == Ordering::Greater {
if removed.version().cmp_precedence(added.version()) == Ordering::Greater {
print_change("Downgrading", msg, &style::WARN)?;
} else {
print_change("Updating", msg, &style::GOOD)?;
}
} else {
for package in removed.iter() {
print_change("Removing", format!("{}", package), &style::ERROR)?;
print_change("Removing", format!("{package}"), &style::ERROR)?;
}
for package in added.iter() {
print_change("Adding", format!("{}", package), &style::NOTE)?;
let latest = if !possibilities.is_empty() {
possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| is_latest(s.version(), package.version()))
.map(|s| s.version().clone())
.max()
.map(format_latest)
} else {
None
}
.unwrap_or_default();

print_change("Adding", format!("{package}{latest}"), &style::NOTE)?;
}
}
for package in &unchanged {
let latest = if !possibilities.is_empty() {
possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| is_latest(s.version(), package.version()))
.map(|s| s.version().clone())
.max()
.map(format_latest)
} else {
None
};

if let Some(latest) = latest {
unchanged_behind += 1;
if opts.config.shell().verbosity() == Verbosity::Verbose {
opts.config.shell().status_with_color(
"Unchanged",
format!("{package}{latest}"),
&anstyle::Style::new().bold(),
)?;
}
}
}
}
if opts.config.shell().verbosity() == Verbosity::Verbose {
opts.config.shell().note(
"to see how you depend on a package, run `cargo tree --invert --package <dep>@<ver>`",
)?;
} else {
if 0 < unchanged_behind {
opts.config.shell().note(format!(
"pass `--verbose` to see {unchanged_behind} unchanged dependencies behind latest"
))?;
}
}
if opts.dry_run {
opts.config
.shell()
Expand All @@ -215,73 +319,87 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
}
}

#[derive(Default, Clone, Debug)]
struct ResolvedPackageVersions {
removed: Vec<PackageId>,
added: Vec<PackageId>,
unchanged: Vec<PackageId>,
}
fn compare_dependency_graphs(
previous_resolve: &Resolve,
resolve: &Resolve,
) -> Vec<(Vec<PackageId>, Vec<PackageId>)> {
) -> Vec<ResolvedPackageVersions> {
fn key(dep: PackageId) -> (&'static str, SourceId) {
(dep.name().as_str(), dep.source_id())
}

// Removes all package IDs in `b` from `a`. Note that this is somewhat
// more complicated because the equality for source IDs does not take
// precise versions into account (e.g., git shas), but we want to take
// that into account here.
fn vec_subtract(a: &[PackageId], b: &[PackageId]) -> Vec<PackageId> {
a.iter()
.filter(|a| {
// If this package ID is not found in `b`, then it's definitely
// in the subtracted set.
let Ok(i) = b.binary_search(a) else {
return true;
};
fn vec_subset(a: &[PackageId], b: &[PackageId]) -> Vec<PackageId> {
a.iter().filter(|a| !contains_id(b, a)).cloned().collect()
}

// If we've found `a` in `b`, then we iterate over all instances
// (we know `b` is sorted) and see if they all have different
// precise versions. If so, then `a` isn't actually in `b` so
// we'll let it through.
//
// Note that we only check this for non-registry sources,
// however, as registries contain enough version information in
// the package ID to disambiguate.
if a.source_id().is_registry() {
return false;
}
b[i..]
.iter()
.take_while(|b| a == b)
.all(|b| !a.source_id().has_same_precise_as(b.source_id()))
})
.cloned()
.collect()
fn vec_intersection(a: &[PackageId], b: &[PackageId]) -> Vec<PackageId> {
a.iter().filter(|a| contains_id(b, a)).cloned().collect()
}

// Check if a PackageId is present `b` from `a`.
//
// Note that this is somewhat more complicated because the equality for source IDs does not
// take precise versions into account (e.g., git shas), but we want to take that into
// account here.
fn contains_id(haystack: &[PackageId], needle: &PackageId) -> bool {
let Ok(i) = haystack.binary_search(needle) else {
return false;
};

// If we've found `a` in `b`, then we iterate over all instances
// (we know `b` is sorted) and see if they all have different
// precise versions. If so, then `a` isn't actually in `b` so
// we'll let it through.
//
// Note that we only check this for non-registry sources,
// however, as registries contain enough version information in
// the package ID to disambiguate.
if needle.source_id().is_registry() {
return true;
}
haystack[i..]
.iter()
.take_while(|b| &needle == b)
.any(|b| needle.source_id().has_same_precise_as(b.source_id()))
}

// Map `(package name, package source)` to `(removed versions, added versions)`.
let mut changes = BTreeMap::new();
let empty = (Vec::new(), Vec::new());
let empty = ResolvedPackageVersions::default();
for dep in previous_resolve.iter() {
changes
.entry(key(dep))
.or_insert_with(|| empty.clone())
.0
.removed
.push(dep);
}
for dep in resolve.iter() {
changes
.entry(key(dep))
.or_insert_with(|| empty.clone())
.1
.added
.push(dep);
}

for v in changes.values_mut() {
let (ref mut old, ref mut new) = *v;
let ResolvedPackageVersions {
removed: ref mut old,
added: ref mut new,
unchanged: ref mut other,
} = *v;
old.sort();
new.sort();
let removed = vec_subtract(old, new);
let added = vec_subtract(new, old);
let removed = vec_subset(old, new);
let added = vec_subset(new, old);
let unchanged = vec_intersection(new, old);
*old = removed;
*new = added;
*other = unchanged;
}
debug!("{:#?}", changes);

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ fn dep_with_changed_submodule() {
sleep_ms(1000);
// Update the dependency and carry on!
println!("update");
p.cargo("update -v")
p.cargo("update")
.with_stderr("")
.with_stderr(&format!(
"[UPDATING] git repository `{}`\n\
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/local_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ fn multiple_versions() {
.file("src/lib.rs", "pub fn bar() {}")
.publish();

p.cargo("update -v")
p.cargo("update")
.with_stderr("[UPDATING] bar v0.1.0 -> v0.2.0")
.run();
}
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2430,6 +2430,7 @@ fn can_update_with_alt_reg() {
"\
[UPDATING] `alternative` index
[UPDATING] `dummy-registry` index
[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
",
)
.run();
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,7 @@ fn update_multiple_packages() {
[UPDATING] `[..]` index
[UPDATING] a v0.1.0 -> v0.1.1
[UPDATING] b v0.1.0 -> v0.1.1
[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
",
)
.run();
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,13 +539,15 @@ fn override_adds_some_deps() {
"\
[UPDATING] git repository `file://[..]`
[UPDATING] `dummy-registry` index
[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
",
)
.run();
p.cargo("update https://github.com/rust-lang/crates.io-index#bar")
.with_stderr(
"\
[UPDATING] `dummy-registry` index
[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
",
)
.run();
Expand Down
Loading

0 comments on commit b0df265

Please sign in to comment.