From 49040249316d63c1785080523ec99dc18ded351c Mon Sep 17 00:00:00 2001
From: Ted Mielczarek <ted@mielczarek.org>
Date: Tue, 3 Jul 2018 08:02:13 -0400
Subject: [PATCH] Don't cache Rust compiles with incremental compilation. Fixes
 #257

rustc's incremental compilation mode isn't well-handled by sccache currently
because we don't cache the separate artifacts it creates. Additionally,
if there are existing incremental artifacts it's likely that rustc will do
a better job than sccache anyway, so don't bother trying to cache Rust
compilation when incremental compilation is enabled for now.

Longer-term we would like to make sccache and rustc incremental work
together nicely: https://github.com/mozilla/sccache/issues/236
---
 src/compiler/rust.rs        | 19 ++++++++++++++++++-
 tests/sccache_cargo.rs      |  3 +++
 tests/test-crate/Cargo.lock | 10 ++++++++++
 tests/test-crate/Cargo.toml |  2 ++
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs
index 3b23a119b8..5ee5e0663f 100644
--- a/src/compiler/rust.rs
+++ b/src/compiler/rust.rs
@@ -464,6 +464,15 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments<Pars
                     if let (Some(name), Some(val)) = (name, val) {
                         match name {
                             "extra-filename" => extra_filename = Some(val.to_owned()),
+                            // Incremental compilation makes a mess of sccache's entire world
+                            // view. It produces additional compiler outputs that we don't cache,
+                            // and just letting rustc do its work in incremental mode is likely
+                            // to be faster than trying to fetch a result from cache anyway, so
+                            // don't bother caching compiles where it's enabled currently.
+                            // Longer-term we would like to figure out better integration between
+                            // sccache and rustc in the incremental scenario:
+                            // https://github.com/mozilla/sccache/issues/236
+                            "incremental" => return CompilerArguments::CannotCache("incremental"),
                             _ => {},
                         }
                     }
@@ -768,7 +777,7 @@ mod test {
             match _parse_arguments(&[ $( $s.to_string(), )* ]) {
                 CompilerArguments::Ok(_) => panic!("Should not have parsed ok: `{}`", stringify!($( $s, )*)),
 
-                _ => {}
+                o @ _ => o,
             }
         }
     }
@@ -815,6 +824,14 @@ mod test {
         assert_eq!(h.externs, ovec!["/foo/target/debug/deps/liblibc-89a24418d48d484a.rlib", "/foo/target/debug/deps/liblog-2f7366be74992849.rlib"]);
     }
 
+    #[test]
+    fn test_parse_arguments_incremental() {
+        parses!("--emit", "link", "foo.rs", "--out-dir", "out", "--crate-name", "foo");
+        let r = fails!("--emit", "link", "foo.rs", "--out-dir", "out", "--crate-name", "foo",
+                       "-C", "incremental=/foo");
+        assert_eq!(r, CompilerArguments::CannotCache("incremental"))
+    }
+
     #[test]
     fn test_parse_arguments_dep_info_no_extra_filename() {
         let h = parses!("--crate-name", "foo", "src/lib.rs",
diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs
index 00d72102de..fabb2b3c55 100644
--- a/tests/sccache_cargo.rs
+++ b/tests/sccache_cargo.rs
@@ -103,6 +103,9 @@ fn test_rust_cargo() {
     trace!("cargo build: {:?}", a);
     a.unwrap();
     // Now get the stats and ensure that we had a cache hit for the second build.
+    // Ideally we'd check the stats more usefully here--the test crate has one dependency (itoa)
+    // so there are two separate compilations, but cargo will build the test crate with
+    // incremental compilation enabled, so sccache will not cache it.
     trace!("sccache --show-stats");
     Assert::command(&[&sccache.to_string_lossy()])
         .with_args(&["--show-stats", "--stats-format=json"])
diff --git a/tests/test-crate/Cargo.lock b/tests/test-crate/Cargo.lock
index d5408138be..b0b44fee07 100644
--- a/tests/test-crate/Cargo.lock
+++ b/tests/test-crate/Cargo.lock
@@ -1,4 +1,14 @@
+[[package]]
+name = "itoa"
+version = "0.3.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "test-crate"
 version = "0.1.0"
+dependencies = [
+ "itoa 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)",
+]
 
+[metadata]
+"checksum itoa 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "8324a32baf01e2ae060e9de58ed0bc2320c9a2833491ee36cd3b4c414de4db8c"
diff --git a/tests/test-crate/Cargo.toml b/tests/test-crate/Cargo.toml
index ac454f0cb5..46d69fad14 100644
--- a/tests/test-crate/Cargo.toml
+++ b/tests/test-crate/Cargo.toml
@@ -4,3 +4,5 @@ version = "0.1.0"
 authors = ["Ted Mielczarek <ted@mielczarek.org>"]
 
 [dependencies]
+# Arbitrary crate dependency that doesn't pull in any transitive dependencies.
+itoa = "0.3.4"