Parse env-deps from dep-info (#1107)

* Refactor sccache cargo test

- Improve cleanup by using Result instead of panicking, to ensure that
the temporary folder gets cleaned up.
Previously, when a test failed, no clean up would happen. Now the cleanup
happens, unless there is an unexpected panic somewhere in  the intergration test code.

- this commit aims to make it easier to add new tests

Split sccache cargo tests

To make it clearer what is failing, split the cargo test into multiple
tests. Since Sccache can't be invoked in parallel, we use the
`serial_test`  to serialize testing,otherwise cargo by default would
start them in parallel.

The logger is now also lazily initialized, so the first test to run will
initialize it. It now writes a linebreak, because otherwise the output
is hard to read with
`RUST_LOG=debug cargo test --test sccache_cargo -- --nocapture`.

We catch panics (which are not intended by the tests ) to ensure that the
temporary directory for the test gets cleaned up.

Signed-off-by: Jonathan Schwender <[email protected]>

Remove panic=abort from CI coverage test

I suspect the commandline was just taken like that from the example
here: https://doc.rust-lang.org/stable/unstable-book/compiler-flags/profile.html

However, there seems to be no reason to actually abort, since we don't have
a custom test harness or anything.
Since we want to catch panics, so we can correctly clean up, we need
panic=unwind.

Signed-off-by: Jonathan Schwender <[email protected]>

* Parse env_deps from dep-info

This will trigger rebuilds if environment variables changed that the rust code
depended on with env!.
On the Rust side this requires at least Rust 1.46, otherwise there will be
no env-dep info in the dep-info file. In that case we cannot detect
the dependency on the env-value, and behaviour is unchanged compared to
the current sccache behaviour.
With recent Rust versions however, we can trigger a rebuild if a variable
that is referenced via env! or option_env! is changed.

Other env variables like `CARGO_*` or `RUSTFLAGS` which may affect
the compilation are not listed in the dep-info file, so they have to
be blanket added (as is currently already the case).

Signed-off-by: Jonathan Schwender <[email protected]>

* Add test for cargo env_deps

The previous commit added support for parsing "env_dep"
information from dep_info files. This commit adds a test
for changing an environment variable, that is referenced
in rust code via env!, and asserts that sccache rebuilds
and the rust code uses the new value of the changed
environment variable.

Signed-off-by: Jonathan Schwender <[email protected]>

Author: jschwe

.github/workflows/ci.yml

@@ -200,7 +200,7 @@ jobs:
         env:
           CARGO_INCREMENTAL: '0'
           RUSTC_WRAPPER: ''
-          RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort'
+          RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off'
 
       - name: Generate coverage data (via `grcov`)
         id: coverage

Cargo.lock

@@ -100,6 +100,8 @@ chrono = "0.4"
 itertools = "0.10"
 predicates = "=2.1.1"
 thirtyfour_sync = "0.27"
+once_cell = "1.9"
+serial_test = "0.5"
 
 [target.'cfg(unix)'.dependencies]
 daemonize = "0.4"

Cargo.toml

@@ -5,7 +5,7 @@ sccache includes support for caching Rust compilation. This includes many caveat
 * `--out-dir` is required.
 * `-o file` is not supported.
 * Compilation from stdin is not supported, a source file must be provided.
-* Values from `env!` will not be tracked in caching.
+* Values from `env!` require Rust >= 1.46 to be tracked in caching.
 * Procedural macros that read files from the filesystem may not be cached properly
 * Target specs aren't hashed (e.g. custom target specs)
 

docs/Rust.md

@@ -197,16 +197,16 @@ lazy_static! {
 /// Version number for cache key.
 const CACHE_VERSION: &[u8] = b"6";
 
-/// Get absolute paths for all source files listed in rustc's dep-info output.
-async fn get_source_files<T>(
+/// Get absolute paths for all source files and env-deps listed in rustc's dep-info output.
+async fn get_source_files_and_env_deps<T>(
     creator: &T,
     crate_name: &str,
     executable: &Path,
     arguments: &[OsString],
     cwd: &Path,
     env_vars: &[(OsString, OsString)],
     pool: &tokio::runtime::Handle,
-) -> Result<Vec<PathBuf>>
+) -> Result<(Vec<PathBuf>, Vec<(OsString, OsString)>)>
 where
     T: CommandCreatorSync,
 {
@@ -238,30 +238,31 @@ where
         })
         .await?;
 
-    parsed.map(move |files| {
+    parsed.map(move |(files, env_deps)| {
         trace!(
-            "[{}]: got {} source files from dep-info in {}",
+            "[{}]: got {} source files and {} env-deps from dep-info in {}",
             crate_name,
             files.len(),
+            env_deps.len(),
             fmt_duration_as_secs(&start.elapsed())
         );
         // Just to make sure we capture temp_dir.
         drop(temp_dir);
-        files
+        (files, env_deps)
     })
 }
 
 /// Parse dependency info from `file` and return a Vec of files mentioned.
 /// Treat paths as relative to `cwd`.
-fn parse_dep_file<T, U>(file: T, cwd: U) -> Result<Vec<PathBuf>>
+fn parse_dep_file<T, U>(file: T, cwd: U) -> Result<(Vec<PathBuf>, Vec<(OsString, OsString)>)>
 where
     T: AsRef<Path>,
     U: AsRef<Path>,
 {
     let mut f = fs::File::open(file)?;
     let mut deps = String::new();
     f.read_to_string(&mut deps)?;
-    Ok(parse_dep_info(&deps, cwd))
+    Ok((parse_dep_info(&deps, cwd), parse_env_dep_info(&deps)))
 }
 
 fn parse_dep_info<T>(dep_info: &str, cwd: T) -> Vec<PathBuf>
@@ -315,6 +316,20 @@ where
     deps
 }
 
+fn parse_env_dep_info(dep_info: &str) -> Vec<(OsString, OsString)> {
+    let mut env_deps = Vec::new();
+    for line in dep_info.lines() {
+        if let Some(env_dep) = line.strip_prefix("# env-dep:") {
+            let mut split = env_dep.splitn(2, '=');
+            match (split.next(), split.next()) {
+                (Some(var), Some(val)) => env_deps.push((var.into(), val.into())),
+                _ => env_deps.push((env_dep.into(), "".into())),
+            }
+        }
+    }
+    env_deps
+}
+
 /// Run `rustc --print file-names` to get the outputs of compilation.
 async fn get_compiler_outputs<T>(
     creator: &T,
@@ -1300,8 +1315,8 @@ where
             .collect::<Vec<_>>();
         // Find all the source files and hash them
         let source_hashes_pool = pool.clone();
-        let source_files_and_hashes = async {
-            let source_files = get_source_files(
+        let source_files_and_hashes_and_env_deps = async {
+            let (source_files, env_deps) = get_source_files_and_env_deps(
                 creator,
                 &crate_name,
                 &executable,
@@ -1312,7 +1327,7 @@ where
             )
             .await?;
             let source_hashes = hash_all(&source_files, &source_hashes_pool).await?;
-            Ok((source_files, source_hashes))
+            Ok((source_files, source_hashes, env_deps))
         };
 
         // Hash the contents of the externs listed on the commandline.
@@ -1324,8 +1339,11 @@ where
         let abs_staticlibs = staticlibs.iter().map(|s| cwd.join(s)).collect::<Vec<_>>();
         let staticlib_hashes = hash_all(&abs_staticlibs, pool);
 
-        let ((source_files, source_hashes), extern_hashes, staticlib_hashes) =
-            futures::try_join!(source_files_and_hashes, extern_hashes, staticlib_hashes)?;
+        let ((source_files, source_hashes, mut env_deps), extern_hashes, staticlib_hashes) = futures::try_join!(
+            source_files_and_hashes_and_env_deps,
+            extern_hashes,
+            staticlib_hashes
+        )?;
         // If you change any of the inputs to the hash, you should change `CACHE_VERSION`.
         let mut m = Digest::new();
         // Hash inputs:
@@ -1375,12 +1393,15 @@ where
         {
             m.update(h.as_bytes());
         }
-        // 7. Environment variables. Ideally we'd use anything referenced
-        // via env! in the program, but we don't have a way to determine that
-        // currently, and hashing all environment variables is too much, so
-        // we'll just hash the CARGO_ env vars and hope that's sufficient.
-        // Upstream Rust issue tracking getting information about env! usage:
-        // https://github.com/rust-lang/rust/issues/40364
+        // 7. Environment variables: Hash all environment variables listed in the rustc dep-info
+        //    output. Additionally also has all environment variables starting with `CARGO_`,
+        //    since those are not listed in dep-info but affect cacheability.
+        env_deps.sort();
+        for &(ref var, ref val) in env_deps.iter() {
+            var.hash(&mut HashToDigest { digest: &mut m });
+            m.update(b"=");
+            val.hash(&mut HashToDigest { digest: &mut m });
+        }
         let mut env_vars: Vec<_> = env_vars
             .iter()
             // Filter out RUSTC_COLOR since we control color usage with command line flags.