Skip to content

Commit 1a93dc4

Browse files
tomoverlundtomoverlund
committed
Don't use cache results for failed commands. Fixes rust-lang#15358 
1 parent c5f58e9 commit 1a93dc4

File tree

2 files changed

+69
-29
lines changed

2 files changed

+69
-29
lines changed

‎src/cargo/util/rustc.rs

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,21 @@ impl Rustc {
172172
struct Cache {
173173
cache_location: Option<PathBuf>,
174174
dirty: bool,
175+
dirty_reason: CacheReason,
175176
data: CacheData,
176177
}
177178

179+
/// These are used in debug! messages, which testsuite/rustc_info_cache.rs
180+
/// relies on for testing. See Cargo issue #15358 for further motivation.
181+
#[derive(Debug)]
182+
enum CacheReason {
183+
NotDirty,
184+
NewCache,
185+
HitSuccess,
186+
HitFailure,
187+
MissNoEntry,
188+
}
189+
178190
#[derive(Serialize, Deserialize, Debug, Default)]
179191
struct CacheData {
180192
rustc_fingerprint: u64,
@@ -211,11 +223,13 @@ impl Cache {
211223
successes: HashMap::new(),
212224
};
213225
let mut dirty = true;
226+
let mut dirty_reason = CacheReason::NewCache;
214227
let data = match read(&cache_location) {
215228
Ok(data) => {
216229
if data.rustc_fingerprint == rustc_fingerprint {
217230
debug!("reusing existing rustc info cache");
218231
dirty = false;
232+
dirty_reason = CacheReason::NotDirty;
219233
data
220234
} else {
221235
debug!("different compiler, creating new rustc info cache");
@@ -230,6 +244,7 @@ impl Cache {
230244
return Cache {
231245
cache_location: Some(cache_location),
232246
dirty,
247+
dirty_reason,
233248
data,
234249
};
235250

@@ -246,6 +261,7 @@ impl Cache {
246261
Cache {
247262
cache_location: None,
248263
dirty: false,
264+
dirty_reason: CacheReason::NotDirty,
249265
data: CacheData::default(),
250266
}
251267
}
@@ -258,10 +274,19 @@ impl Cache {
258274
extra_fingerprint: u64,
259275
) -> CargoResult<(String, String)> {
260276
let key = process_fingerprint(cmd, extra_fingerprint);
261-
if self.data.outputs.contains_key(&key) {
262-
debug!("rustc info cache hit");
277+
if self.data.outputs.contains_key(&key) && self.data.outputs[&key].success {
278+
debug!("rustc info cache hit ({:?})", CacheReason::HitSuccess);
263279
} else {
264-
debug!("rustc info cache miss");
280+
let reason: CacheReason = if self.data.outputs.contains_key(&key) {
281+
debug!(
282+
"rustc info cache hit ({:?}), re-running on failure",
283+
CacheReason::HitFailure
284+
);
285+
CacheReason::HitFailure
286+
} else {
287+
debug!("rustc info cache miss ({:?})", CacheReason::MissNoEntry);
288+
CacheReason::MissNoEntry
289+
};
265290
debug!("running {}", cmd);
266291
let output = cmd.output()?;
267292
let stdout = String::from_utf8(output.stdout)
@@ -284,7 +309,11 @@ impl Cache {
284309
stderr,
285310
},
286311
);
287-
self.dirty = true;
312+
// Don't overwrite the old reason.
313+
if !self.dirty {
314+
self.dirty = true;
315+
self.dirty_reason = reason;
316+
}
288317
}
289318
let output = &self.data.outputs[&key];
290319
if output.success {
@@ -310,7 +339,7 @@ impl Drop for Cache {
310339
if let Some(ref path) = self.cache_location {
311340
let json = serde_json::to_string(&self.data).unwrap();
312341
match paths::write(path, json.as_bytes()) {
313-
Ok(()) => info!("updated rustc info cache"),
342+
Ok(()) => info!("updated rustc info cache ({:?})", self.dirty_reason),
314343
Err(e) => warn!("failed to update rustc info cache: {}", e),
315344
}
316345
}

‎tests/testsuite/rustc_info_cache.rs

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,18 @@ use cargo_test_support::basic_bin_manifest;
66
use cargo_test_support::prelude::*;
77
use cargo_test_support::{basic_manifest, project};
88

9-
const MISS: &str = "[..] rustc info cache miss[..]";
10-
const HIT: &str = "[..]rustc info cache hit[..]";
11-
const UPDATE: &str = "[..]updated rustc info cache[..]";
9+
// Here, we distinguish between a cache hit where the command succeeded versus
10+
// a cache hit where the command failed. Commands that fail are recorded into
11+
// the cache, but are re-run anyways.
12+
//
13+
// Beware of checking for the absence of failure, as a probe for supported
14+
// rustc flags may fail under normal conditions, such as when a new feature
15+
// is added. See Cargo issue #15358 for details.
16+
const MISS: &str = "[..] rustc info cache miss (MissNoEntry)[..]";
17+
const HIT_SUCCESS: &str = "[..]rustc info cache hit (HitSuccess)[..]";
18+
const HIT_FAILURE: &str = "[..]rustc info cache hit (HitFailure)[..]";
19+
const UPDATE_NEWCACHE: &str = "[..]updated rustc info cache (NewCache)[..]";
20+
const UPDATE_FAILURE: &str = "[..]updated rustc info cache (HitFailure)[..]";
1221

1322
#[cargo_test]
1423
fn rustc_info_cache() {
@@ -20,23 +29,23 @@ fn rustc_info_cache() {
2029
.env("CARGO_LOG", "cargo::util::rustc=debug")
2130
.with_stderr_contains("[..]failed to read rustc info cache[..]")
2231
.with_stderr_contains(MISS)
23-
.with_stderr_does_not_contain(HIT)
24-
.with_stderr_contains(UPDATE)
32+
.with_stderr_does_not_contain(HIT_SUCCESS)
33+
.with_stderr_contains(UPDATE_NEWCACHE)
2534
.run();
2635

2736
p.cargo("build")
2837
.env("CARGO_LOG", "cargo::util::rustc=debug")
2938
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
30-
.with_stderr_contains(HIT)
39+
.with_stderr_contains(HIT_SUCCESS)
3140
.with_stderr_does_not_contain(MISS)
32-
.with_stderr_does_not_contain(UPDATE)
41+
.with_stderr_does_not_contain(UPDATE_NEWCACHE)
3342
.run();
3443

3544
p.cargo("build")
3645
.env("CARGO_LOG", "cargo::util::rustc=debug")
3746
.env("CARGO_CACHE_RUSTC_INFO", "0")
3847
.with_stderr_contains("[..]rustc info cache disabled[..]")
39-
.with_stderr_does_not_contain(UPDATE)
48+
.with_stderr_does_not_contain(UPDATE_NEWCACHE)
4049
.run();
4150

4251
let other_rustc = {
@@ -71,17 +80,17 @@ fn rustc_info_cache() {
7180
.env("RUSTC", other_rustc.display().to_string())
7281
.with_stderr_contains("[..]different compiler, creating new rustc info cache[..]")
7382
.with_stderr_contains(MISS)
74-
.with_stderr_does_not_contain(HIT)
75-
.with_stderr_contains(UPDATE)
83+
.with_stderr_does_not_contain(HIT_SUCCESS)
84+
.with_stderr_contains(UPDATE_NEWCACHE)
7685
.run();
7786

7887
p.cargo("build")
7988
.env("CARGO_LOG", "cargo::util::rustc=debug")
8089
.env("RUSTC", other_rustc.display().to_string())
8190
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
82-
.with_stderr_contains(HIT)
91+
.with_stderr_contains(HIT_SUCCESS)
8392
.with_stderr_does_not_contain(MISS)
84-
.with_stderr_does_not_contain(UPDATE)
93+
.with_stderr_does_not_contain(UPDATE_NEWCACHE)
8594
.run();
8695

8796
other_rustc.move_into_the_future();
@@ -91,17 +100,17 @@ fn rustc_info_cache() {
91100
.env("RUSTC", other_rustc.display().to_string())
92101
.with_stderr_contains("[..]different compiler, creating new rustc info cache[..]")
93102
.with_stderr_contains(MISS)
94-
.with_stderr_does_not_contain(HIT)
95-
.with_stderr_contains(UPDATE)
103+
.with_stderr_does_not_contain(HIT_SUCCESS)
104+
.with_stderr_contains(UPDATE_NEWCACHE)
96105
.run();
97106

98107
p.cargo("build")
99108
.env("CARGO_LOG", "cargo::util::rustc=debug")
100109
.env("RUSTC", other_rustc.display().to_string())
101110
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
102-
.with_stderr_contains(HIT)
111+
.with_stderr_contains(HIT_SUCCESS)
103112
.with_stderr_does_not_contain(MISS)
104-
.with_stderr_does_not_contain(UPDATE)
113+
.with_stderr_does_not_contain(UPDATE_NEWCACHE)
105114
.run();
106115
}
107116

@@ -149,16 +158,16 @@ fn rustc_info_cache_with_wrappers() {
149158
.env(wrapper_env, &wrapper)
150159
.with_stderr_contains("[..]failed to read rustc info cache[..]")
151160
.with_stderr_contains(MISS)
152-
.with_stderr_contains(UPDATE)
153-
.with_stderr_does_not_contain(HIT)
161+
.with_stderr_contains(UPDATE_NEWCACHE)
162+
.with_stderr_does_not_contain(HIT_SUCCESS)
154163
.with_status(0)
155164
.run();
156165
p.cargo("build")
157166
.env("CARGO_LOG", "cargo::util::rustc=debug")
158167
.env(wrapper_env, &wrapper)
159168
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
160-
.with_stderr_contains(HIT)
161-
.with_stderr_does_not_contain(UPDATE)
169+
.with_stderr_contains(HIT_SUCCESS)
170+
.with_stderr_does_not_contain(UPDATE_NEWCACHE)
162171
.with_stderr_does_not_contain(MISS)
163172
.with_status(0)
164173
.run();
@@ -171,16 +180,18 @@ fn rustc_info_cache_with_wrappers() {
171180
.env(wrapper_env, &wrapper)
172181
.with_stderr_contains("[..]different compiler, creating new rustc info cache[..]")
173182
.with_stderr_contains(MISS)
174-
.with_stderr_contains(UPDATE)
175-
.with_stderr_does_not_contain(HIT)
183+
.with_stderr_contains(UPDATE_NEWCACHE)
184+
.with_stderr_does_not_contain(HIT_SUCCESS)
176185
.with_status(101)
177186
.run();
178187
p.cargo("build")
179188
.env("CARGO_LOG", "cargo::util::rustc=debug")
180189
.env(wrapper_env, &wrapper)
181190
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
182-
.with_stderr_contains(HIT)
183-
.with_stderr_does_not_contain(UPDATE)
191+
.with_stderr_does_not_contain(HIT_SUCCESS)
192+
.with_stderr_contains(HIT_FAILURE)
193+
.with_stderr_does_not_contain(UPDATE_NEWCACHE)
194+
.with_stderr_contains(UPDATE_FAILURE)
184195
.with_stderr_does_not_contain(MISS)
185196
.with_status(101)
186197
.run();

0 commit comments

Comments
 (0)