Skip to content

Commit 3fb97ec

Browse files
committed
Fix off-by-one error when future-incompat report is cached
This fixes a problem introduced by #11648 where the future-incompat report will tell you to run with an `--id` flag with the wrong value if the report is already cached. The solution is to add a method to determine which ID to use for the suggestions *before* attempting to save the report.
1 parent afdfd92 commit 3fb97ec

File tree

2 files changed

+24
-18
lines changed

2 files changed

+24
-18
lines changed

src/cargo/core/compiler/future_incompat.rs

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -140,16 +140,10 @@ impl OnDiskReports {
140140
mut self,
141141
ws: &Workspace<'_>,
142142
suggestion_message: String,
143-
per_package_reports: &[FutureIncompatReportPackage],
143+
per_package: BTreeMap<String, String>,
144144
) -> 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;
145+
if let Some(existing_id) = self.has_report(&per_package) {
146+
return existing_id;
153147
}
154148

155149
let report = OnDiskReport {
@@ -189,6 +183,14 @@ impl OnDiskReports {
189183
saved_id
190184
}
191185

186+
/// Returns the ID of a report if it is already on disk.
187+
fn has_report(&self, rendered_per_package: &BTreeMap<String, String>) -> Option<u32> {
188+
self.reports
189+
.iter()
190+
.find(|existing| &existing.per_package == rendered_per_package)
191+
.map(|report| report.id)
192+
}
193+
192194
/// Loads the on-disk reports.
193195
pub fn load(ws: &Workspace<'_>) -> CargoResult<OnDiskReports> {
194196
let report_file = match ws.build_dir().open_ro_shared(
@@ -408,7 +410,14 @@ pub fn save_and_display_report(
408410
OnDiskReports::default()
409411
}
410412
};
411-
let report_id = current_reports.next_id;
413+
414+
let rendered_report = render_report(per_package_future_incompat_reports);
415+
416+
// If the report is already on disk, then it will reuse the same ID,
417+
// otherwise prepare for the next ID.
418+
let report_id = current_reports
419+
.has_report(&rendered_report)
420+
.unwrap_or(current_reports.next_id);
412421

413422
// Get a list of unique and sorted package name/versions.
414423
let package_ids: BTreeSet<_> = per_package_future_incompat_reports
@@ -481,11 +490,8 @@ https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#the-patch
481490
update_message = update_message,
482491
);
483492

484-
let saved_report_id = current_reports.save_report(
485-
bcx.ws,
486-
suggestion_message.clone(),
487-
per_package_future_incompat_reports,
488-
);
493+
let saved_report_id =
494+
current_reports.save_report(bcx.ws, suggestion_message.clone(), rendered_report);
489495

490496
if bcx.build_config.future_incompat_report {
491497
drop(bcx.gctx.shell().note(&suggestion_message));

tests/testsuite/future_incompat_report.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ fix to the maintainers (e.g. by creating a pull request):
102102
103103
- foo@0.0.0
104104
- Repository: <not found>
105-
- Detailed warning command: `cargo report future-incompatibilities --id 2 --package foo@0.0.0`
105+
- Detailed warning command: `cargo report future-incompatibilities --id 1 --package foo@0.0.0`
106106
107107
- If waiting for an upstream fix is not an option, you can use the `[patch]`
108108
section in `Cargo.toml` to use your own version of the dependency. For more
@@ -192,7 +192,7 @@ fix to the maintainers (e.g. by creating a pull request):
192192
193193
- bar@1.0.0
194194
- Repository: https://example.com/
195-
- Detailed warning command: `cargo report future-incompatibilities --id 2 --package bar@1.0.0`
195+
- Detailed warning command: `cargo report future-incompatibilities --id 1 --package bar@1.0.0`
196196
197197
- If waiting for an upstream fix is not an option, you can use the `[patch]`
198198
section in `Cargo.toml` to use your own version of the dependency. For more
@@ -708,7 +708,7 @@ fix to the maintainers (e.g. by creating a pull request):
708708
709709
- bar@1.0.0
710710
- Repository: https://example.com/
711-
- Detailed warning command: `cargo report future-incompatibilities --id 2 --package bar@1.0.0`
711+
- Detailed warning command: `cargo report future-incompatibilities --id 1 --package bar@1.0.0`
712712
713713
- If waiting for an upstream fix is not an option, you can use the `[patch]`
714714
section in `Cargo.toml` to use your own version of the dependency. For more

0 commit comments

Comments
 (0)