Skip to content

Commit 7d11b58

Browse files
authored
Fix some issues with future-incompat report generation (#15345)
This fixes various issues with the future-incompat report. In particular, it addresses the off-by-one for the `--id` flag when the report is already cached, and it doesn't recommend updating dependencies when the error comes from a local crate. See each commit for more details. Fixes #15337
2 parents 307cbfd + 0b2299e commit 7d11b58

File tree

3 files changed

+378
-39
lines changed

3 files changed

+378
-39
lines changed

src/cargo/core/compiler/future_incompat.rs

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ pub struct FutureIncompatReport {
7070
/// Structure used for collecting reports in-memory.
7171
pub struct FutureIncompatReportPackage {
7272
pub package_id: PackageId,
73+
/// Whether or not this is a local package, or a remote dependency.
74+
pub is_local: bool,
7375
pub items: Vec<FutureBreakageItem>,
7476
}
7577

@@ -140,16 +142,10 @@ impl OnDiskReports {
140142
mut self,
141143
ws: &Workspace<'_>,
142144
suggestion_message: String,
143-
per_package_reports: &[FutureIncompatReportPackage],
145+
per_package: BTreeMap<String, String>,
144146
) -> u32 {
145-
let per_package = render_report(per_package_reports);
146-
147-
if let Some(existing_report) = self
148-
.reports
149-
.iter()
150-
.find(|existing| existing.per_package == per_package)
151-
{
152-
return existing_report.id;
147+
if let Some(existing_id) = self.has_report(&per_package) {
148+
return existing_id;
153149
}
154150

155151
let report = OnDiskReport {
@@ -189,6 +185,14 @@ impl OnDiskReports {
189185
saved_id
190186
}
191187

188+
/// Returns the ID of a report if it is already on disk.
189+
fn has_report(&self, rendered_per_package: &BTreeMap<String, String>) -> Option<u32> {
190+
self.reports
191+
.iter()
192+
.find(|existing| &existing.per_package == rendered_per_package)
193+
.map(|report| report.id)
194+
}
195+
192196
/// Loads the on-disk reports.
193197
pub fn load(ws: &Workspace<'_>) -> CargoResult<OnDiskReports> {
194198
let report_file = match ws.build_dir().open_ro_shared(
@@ -408,7 +412,14 @@ pub fn save_and_display_report(
408412
OnDiskReports::default()
409413
}
410414
};
411-
let report_id = current_reports.next_id;
415+
416+
let rendered_report = render_report(per_package_future_incompat_reports);
417+
418+
// If the report is already on disk, then it will reuse the same ID,
419+
// otherwise prepare for the next ID.
420+
let report_id = current_reports
421+
.has_report(&rendered_report)
422+
.unwrap_or(current_reports.next_id);
412423

413424
// Get a list of unique and sorted package name/versions.
414425
let package_ids: BTreeSet<_> = per_package_future_incompat_reports
@@ -461,11 +472,18 @@ You may want to consider updating them to a newer version to see if the issue ha
461472
.collect::<Vec<_>>()
462473
.join("\n");
463474

464-
let suggestion_message = format!(
465-
"
475+
let all_is_local = per_package_future_incompat_reports
476+
.iter()
477+
.all(|report| report.is_local);
478+
479+
let suggestion_message = if all_is_local {
480+
String::new()
481+
} else {
482+
format!(
483+
"
466484
To solve this problem, you can try the following approaches:
467485
468-
{update_message}
486+
{update_message}\
469487
- If the issue is not solved by updating the dependencies, a fix has to be
470488
implemented by those dependencies. You can help with that by notifying the
471489
maintainers of this problem (e.g. by creating a bug report) or by proposing a
@@ -476,19 +494,19 @@ fix to the maintainers (e.g. by creating a pull request):
476494
section in `Cargo.toml` to use your own version of the dependency. For more
477495
information, see:
478496
https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#the-patch-section
479-
",
480-
upstream_info = upstream_info,
481-
update_message = update_message,
482-
);
497+
",
498+
upstream_info = upstream_info,
499+
update_message = update_message,
500+
)
501+
};
483502

484-
let saved_report_id = current_reports.save_report(
485-
bcx.ws,
486-
suggestion_message.clone(),
487-
per_package_future_incompat_reports,
488-
);
503+
let saved_report_id =
504+
current_reports.save_report(bcx.ws, suggestion_message.clone(), rendered_report);
489505

490506
if bcx.build_config.future_incompat_report {
491-
drop(bcx.gctx.shell().note(&suggestion_message));
507+
if !suggestion_message.is_empty() {
508+
drop(bcx.gctx.shell().note(&suggestion_message));
509+
}
492510
drop(bcx.gctx.shell().note(&format!(
493511
"this report can be shown with `cargo report \
494512
future-incompatibilities --id {}`",

src/cargo/core/compiler/job_queue/mod.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -700,9 +700,15 @@ impl<'gctx> DrainState<'gctx> {
700700
}
701701
}
702702
Message::FutureIncompatReport(id, items) => {
703-
let package_id = self.active[&id].pkg.package_id();
703+
let unit = &self.active[&id];
704+
let package_id = unit.pkg.package_id();
705+
let is_local = unit.is_local();
704706
self.per_package_future_incompat_reports
705-
.push(FutureIncompatReportPackage { package_id, items });
707+
.push(FutureIncompatReportPackage {
708+
package_id,
709+
is_local,
710+
items,
711+
});
706712
}
707713
Message::Token(acquired_token) => {
708714
let token = acquired_token.context("failed to acquire jobserver token")?;

0 commit comments

Comments
 (0)