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.

Sign up for GitHub

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

fix(resolve): Don't list transitive, incompatible dependencies as available #14568

Merged
merged 2 commits into from
Sep 21, 2024
Merged

fix(resolve): Don't list transitive, incompatible dependencies as available #14568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2107a43
refactor(update): Clarify what each checks intention is
epage Sep 13, 2024
fd743da
fix(resolve): Don't list transitive, incompatible dependencies as ava…
epage Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 43 additions & 43 deletions src/cargo/ops/cargo_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ fn report_latest(possibilities: &[IndexSummary], change: &PackageChange) -> Opti
let version_req = package_id.version().to_caret_req();
let required_rust_version = change.required_rust_version.as_ref();

if let Some(summary) = possibilities
let compat_ver_compat_msrv_summary = possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| {
Expand All @@ -787,45 +787,43 @@ fn report_latest(possibilities: &[IndexSummary], change: &PackageChange) -> Opti
}
})
.filter(|s| package_id.version() != s.version() && version_req.matches(s.version()))
.max_by_key(|s| s.version())
{
.max_by_key(|s| s.version());
if let Some(summary) = compat_ver_compat_msrv_summary {
let warn = style::WARN;
let version = summary.version();
let report = format!(" {warn}(available: v{version}){warn:#}");
return Some(report);
}

if let Some(summary) = possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| {
if let (Some(summary_rust_version), Some(required_rust_version)) =
(s.rust_version(), required_rust_version)
{
summary_rust_version.is_compatible_with(required_rust_version)
} else {
true
}
})
.filter(|s| is_latest(s.version(), package_id.version()))
.max_by_key(|s| s.version())
{
let warn = if change.is_transitive.unwrap_or(true) {
Default::default()
} else {
style::WARN
};
let version = summary.version();
let report = format!(" {warn}(available: v{version}){warn:#}");
return Some(report);
if !change.is_transitive.unwrap_or(true) {
Copy link
Member

Choose a reason for hiding this comment

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

I might start forgetting things again. What does is_transitive mean here? It doesn't seem to be "transtive dependency" comparing to direct dependency. Or, is everything in PackageDiff is transitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is transitive vs direct dependency.

Copy link
Member

Choose a reason for hiding this comment

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

Understood now. I missed this part that sets it back to false.

cargo/src/cargo/ops/cargo_update.rs

Lines 989 to 994 in 9d5c149

change.is_transitive = Some(false);
for (direct_dep_id, _) in resolve.deps(*member_id) {
let Some(change) = changes.get_mut(&direct_dep_id) else {
continue;
};
change.is_transitive = Some(false);

let incompat_ver_compat_msrv_summary = possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| {
if let (Some(summary_rust_version), Some(required_rust_version)) =
(s.rust_version(), required_rust_version)
{
summary_rust_version.is_compatible_with(required_rust_version)
} else {
true
}
})
.filter(|s| is_latest(s.version(), package_id.version()))
.max_by_key(|s| s.version());
if let Some(summary) = incompat_ver_compat_msrv_summary {
let warn = style::WARN;
let version = summary.version();
let report = format!(" {warn}(available: v{version}){warn:#}");
return Some(report);
}
}

if let Some(summary) = possibilities
let compat_ver_summary = possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| package_id.version() != s.version() && version_req.matches(s.version()))
.max_by_key(|s| s.version())
{
.max_by_key(|s| s.version());
if let Some(summary) = compat_ver_summary {
let msrv_note = summary
.rust_version()
.map(|rv| format!(", requires Rust {rv}"))
Expand All @@ -836,20 +834,22 @@ fn report_latest(possibilities: &[IndexSummary], change: &PackageChange) -> Opti
return Some(report);
}

if let Some(summary) = possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| is_latest(s.version(), package_id.version()))
.max_by_key(|s| s.version())
{
let msrv_note = summary
.rust_version()
.map(|rv| format!(", requires Rust {rv}"))
.unwrap_or_default();
let warn = style::NOP;
let version = summary.version();
let report = format!(" {warn}(available: v{version}{msrv_note}){warn:#}");
return Some(report);
if !change.is_transitive.unwrap_or(true) {
let incompat_ver_summary = possibilities
.iter()
.map(|s| s.as_summary())
.filter(|s| is_latest(s.version(), package_id.version()))
.max_by_key(|s| s.version());
if let Some(summary) = incompat_ver_summary {
let msrv_note = summary
.rust_version()
.map(|rv| format!(", requires Rust {rv}"))
.unwrap_or_default();
let warn = style::NOP;
let version = summary.version();
let report = format!(" {warn}(available: v{version}{msrv_note}){warn:#}");
return Some(report);
}
}

None
Expand Down
1 change: 0 additions & 1 deletion tests/testsuite/paths.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ fn broken_path_override_warns() {
.with_stderr_data(str![[r#"
[UPDATING] `dummy-registry` index
[LOCKING] 2 packages to latest compatible versions
[ADDING] bar v0.1.0 (available: v0.2.0)
[WARNING] path override for crate `a` has altered the original list of
dependencies; the dependency on `bar` was either added or
modified to not match the previously resolved version
Expand Down
5 changes: 2 additions & 3 deletions tests/testsuite/update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,6 @@ fn update_precise() {
.with_stderr_data(str![[r#"
[UPDATING] `dummy-registry` index
[DOWNGRADING] serde v0.2.1 -> v0.2.0
[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest

"#]])
.run();
Expand Down Expand Up @@ -2183,7 +2182,7 @@ fn update_breaking_specific_packages_that_wont_update() {
[UPDATING] non-semver v1.0.0 -> v1.0.1 (available: v2.0.0)
[UPDATING] renamed-from v1.0.0 -> v1.0.1 (available: v2.0.0)
[UPDATING] transitive-compatible v1.0.0 -> v1.0.1
[UPDATING] transitive-incompatible v1.0.0 -> v1.0.1 (available: v2.0.0)
[UPDATING] transitive-incompatible v1.0.0 -> v1.0.1

"#]])
.run();
Expand Down Expand Up @@ -2397,7 +2396,7 @@ fn update_breaking_spec_version_transitive() {
.with_stderr_data(str![[r#"
[UPDATING] `[..]` index
[LOCKING] 1 package to latest compatible version
[UPDATING] dep v1.1.0 -> v1.1.1 (available: v2.0.0)
[UPDATING] dep v1.1.0 -> v1.1.1

"#]])
.run();
Expand Down