Take dist archive into account when hashing rust compiles and caching…

… compiler info. The hash for rust compilations do not currently take the dist archive into account, meaning that updates to the dist archive may require clearing the cache to purge old compiled artifacts. This takes the dist archive into account when hashing rust compiles by hashing it when constructing a rust compiler instance. It also takes the dist archive and its mtime into account when caching compiler info in order to account for swapping out archives without re-starting the local daemon. Fixes #554
mozilla · Feb 4, 2020 · c701598 · c701598
1 parent 9981926
commit c701598
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 32 deletions.
diff --git a/src/commands.rs b/src/commands.rs
@@ -663,7 +663,7 @@ pub fn run_command(cmd: Command) -> Result<i32> {
             let pool = CpuPool::new(1);
             let out_file = File::create(out)?;
 
-            let compiler = compiler::get_compiler_info(&creator, &executable, &env, &pool);
+            let compiler = compiler::get_compiler_info(&creator, &executable, &env, &pool, None);
             let packager = compiler.map(|c| c.get_toolchain_packager());
             let res = packager.and_then(|p| p.write_pkg(out_file));
             runtime.block_on(res)?

diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs
@@ -864,6 +864,7 @@ fn detect_compiler<T>(
     executable: &Path,
     env: &[(OsString, OsString)],
     pool: &CpuPool,
+    dist_archive: Option<PathBuf>,
 ) -> SFuture<Box<dyn Compiler<T>>>
 where
     T: CommandCreatorSync,
@@ -901,8 +902,15 @@ where
         Some(Ok(rustc_verbose_version)) => {
             debug!("Found rustc");
             Box::new(
-                Rust::new(creator, executable, &env, &rustc_verbose_version, pool)
-                    .map(|c| Box::new(c) as Box<dyn Compiler<T>>),
+                Rust::new(
+                    creator,
+                    executable,
+                    &env,
+                    &rustc_verbose_version,
+                    dist_archive,
+                    pool,
+                )
+                .map(|c| Box::new(c) as Box<dyn Compiler<T>>),
             )
         }
         Some(Err(e)) => f_err(e),
@@ -1026,12 +1034,13 @@ pub fn get_compiler_info<T>(
     executable: &Path,
     env: &[(OsString, OsString)],
     pool: &CpuPool,
+    dist_archive: Option<PathBuf>,
 ) -> SFuture<Box<dyn Compiler<T>>>
 where
     T: CommandCreatorSync,
 {
     let pool = pool.clone();
-    detect_compiler(creator, executable, env, &pool)
+    detect_compiler(creator, executable, env, &pool, dist_archive)
 }
 
 #[cfg(test)]
@@ -1060,7 +1069,7 @@ mod test {
             &creator,
             Ok(MockChild::new(exit_status(0), "foo\nbar\ngcc", "")),
         );
-        let c = detect_compiler(&creator, &f.bins[0], &[], &pool)
+        let c = detect_compiler(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         assert_eq!(CompilerKind::C(CCompilerKind::GCC), c.kind());
@@ -1075,7 +1084,7 @@ mod test {
             &creator,
             Ok(MockChild::new(exit_status(0), "clang\nfoo", "")),
         );
-        let c = detect_compiler(&creator, &f.bins[0], &[], &pool)
+        let c = detect_compiler(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         assert_eq!(CompilerKind::C(CCompilerKind::Clang), c.kind());
@@ -1104,7 +1113,7 @@ mod test {
             &creator,
             Ok(MockChild::new(exit_status(0), &stdout, &String::new())),
         );
-        let c = detect_compiler(&creator, &f.bins[0], &[], &pool)
+        let c = detect_compiler(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         assert_eq!(CompilerKind::C(CCompilerKind::MSVC), c.kind());
@@ -1138,7 +1147,7 @@ LLVM version: 6.0",
         // rustc --print=sysroot
         let sysroot = f.tempdir.path().to_str().unwrap();
         next_command(&creator, Ok(MockChild::new(exit_status(0), &sysroot, "")));
-        let c = detect_compiler(&creator, &rustc, &[], &pool)
+        let c = detect_compiler(&creator, &rustc, &[], &pool, None)
             .wait()
             .unwrap();
         assert_eq!(CompilerKind::Rust, c.kind());
@@ -1153,7 +1162,7 @@ LLVM version: 6.0",
             &creator,
             Ok(MockChild::new(exit_status(0), "foo\ndiab\nbar", "")),
         );
-        let c = detect_compiler(&creator, &f.bins[0], &[], &pool)
+        let c = detect_compiler(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         assert_eq!(CompilerKind::C(CCompilerKind::Diab), c.kind());
@@ -1167,19 +1176,23 @@ LLVM version: 6.0",
             &creator,
             Ok(MockChild::new(exit_status(0), "something", "")),
         );
-        assert!(detect_compiler(&creator, "/foo/bar".as_ref(), &[], &pool)
-            .wait()
-            .is_err());
+        assert!(
+            detect_compiler(&creator, "/foo/bar".as_ref(), &[], &pool, None)
+                .wait()
+                .is_err()
+        );
     }
 
     #[test]
     fn test_detect_compiler_kind_process_fail() {
         let creator = new_creator();
         let pool = CpuPool::new(1);
         next_command(&creator, Ok(MockChild::new(exit_status(1), "", "")));
-        assert!(detect_compiler(&creator, "/foo/bar".as_ref(), &[], &pool)
-            .wait()
-            .is_err());
+        assert!(
+            detect_compiler(&creator, "/foo/bar".as_ref(), &[], &pool, None)
+                .wait()
+                .is_err()
+        );
     }
 
     #[test]
@@ -1189,7 +1202,7 @@ LLVM version: 6.0",
         let f = TestFixture::new();
         // Pretend to be GCC.
         next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
-        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool)
+        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         // sha-1 digest of an empty file.
@@ -1207,7 +1220,7 @@ LLVM version: 6.0",
         let storage: Arc<dyn Storage> = Arc::new(storage);
         // Pretend to be GCC.
         next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
-        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool)
+        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         // The preprocessor invocation.
@@ -1311,7 +1324,7 @@ LLVM version: 6.0",
         let storage: Arc<dyn Storage> = Arc::new(storage);
         // Pretend to be GCC.
         next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
-        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool)
+        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         // The preprocessor invocation.
@@ -1411,7 +1424,7 @@ LLVM version: 6.0",
         let storage: Arc<MockStorage> = Arc::new(storage);
         // Pretend to be GCC.
         next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
-        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool)
+        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         // The preprocessor invocation.
@@ -1485,7 +1498,7 @@ LLVM version: 6.0",
         let storage: Arc<dyn Storage> = Arc::new(storage);
         // Pretend to be GCC.
         next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
-        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool)
+        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         const COMPILER_STDOUT: &'static [u8] = b"compiler stdout";
@@ -1598,7 +1611,7 @@ LLVM version: 6.0",
             f.write_all(b"file contents")?;
             Ok(MockChild::new(exit_status(0), "gcc", ""))
         });
-        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool)
+        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         // We should now have a fake object file.
@@ -1659,7 +1672,7 @@ LLVM version: 6.0",
         let storage: Arc<dyn Storage> = Arc::new(storage);
         // Pretend to be GCC.
         next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
-        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool)
+        let c = get_compiler_info(&creator, &f.bins[0], &[], &pool, None)
             .wait()
             .unwrap();
         const COMPILER_STDOUT: &'static [u8] = b"compiler stdout";
@@ -1781,6 +1794,9 @@ mod test_dist {
         fn rewrite_includes_only(&self) -> bool {
             false
         }
+        fn get_custom_toolchain(&self, _exe: &PathBuf) -> Option<PathBuf> {
+            None
+        }
     }
 
     pub struct ErrorAllocJobClient {
@@ -1826,6 +1842,9 @@ mod test_dist {
         fn rewrite_includes_only(&self) -> bool {
             false
         }
+        fn get_custom_toolchain(&self, _exe: &PathBuf) -> Option<PathBuf> {
+            None
+        }
     }
 
     pub struct ErrorSubmitToolchainClient {
@@ -1887,6 +1906,9 @@ mod test_dist {
         fn rewrite_includes_only(&self) -> bool {
             false
         }
+        fn get_custom_toolchain(&self, _exe: &PathBuf) -> Option<PathBuf> {
+            None
+        }
     }
 
     pub struct ErrorRunJobClient {
@@ -1956,6 +1978,9 @@ mod test_dist {
         fn rewrite_includes_only(&self) -> bool {
             false
         }
+        fn get_custom_toolchain(&self, _exe: &PathBuf) -> Option<PathBuf> {
+            None
+        }
     }
 
     pub struct OneshotClient {
@@ -2046,5 +2071,8 @@ mod test_dist {
         fn rewrite_includes_only(&self) -> bool {
             false
         }
+        fn get_custom_toolchain(&self, _exe: &PathBuf) -> Option<PathBuf> {
+            None
+        }
     }
 }
diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs
@@ -342,6 +342,7 @@ impl Rust {
         executable: PathBuf,
         env_vars: &[(OsString, OsString)],
         rustc_verbose_version: &str,
+        dist_archive: Option<PathBuf>,
         pool: CpuPool,
     ) -> SFuture<Rust>
     where
@@ -384,6 +385,10 @@ impl Rust {
                     })
                 })
                 .collect::<Vec<_>>();
+            if let Some(path) = dist_archive {
+                trace!("Hashing {:?} along with rustc libs.", path);
+                libs.push(path.to_path_buf());
+            };
             libs.sort();
             Ok((sysroot, libs))
         });

diff --git a/src/dist/cache.rs b/src/dist/cache.rs
@@ -213,7 +213,7 @@ mod client {
             Ok((tc, None))
         }
 
-        fn get_custom_toolchain(
+        pub fn get_custom_toolchain(
             &self,
             compiler_path: &Path,
         ) -> Option<Result<(Toolchain, String, PathBuf)>> {

diff --git a/src/dist/http.rs b/src/dist/http.rs
@@ -1062,7 +1062,7 @@ mod client {
     use futures_cpupool::CpuPool;
     use std::collections::HashMap;
     use std::io::Write;
-    use std::path::Path;
+    use std::path::{Path, PathBuf};
     use std::sync::{Arc, Mutex};
     use std::time::Duration;
 
@@ -1295,7 +1295,7 @@ mod client {
             compiler_path: &Path,
             weak_key: &str,
             toolchain_packager: Box<dyn ToolchainPackager>,
-        ) -> SFuture<(Toolchain, Option<(String, std::path::PathBuf)>)> {
+        ) -> SFuture<(Toolchain, Option<(String, PathBuf)>)> {
             let compiler_path = compiler_path.to_owned();
             let weak_key = weak_key.to_owned();
             let tc_cache = self.tc_cache.clone();
@@ -1307,5 +1307,11 @@ mod client {
         fn rewrite_includes_only(&self) -> bool {
             self.rewrite_includes_only
         }
+        fn get_custom_toolchain(&self, exe: &PathBuf) -> Option<PathBuf> {
+            match self.tc_cache.get_custom_toolchain(exe) {
+                Some(Ok((_, _, path))) => Some(path),
+                _ => None,
+            }
+        }
     }
 }
diff --git a/src/dist/mod.rs b/src/dist/mod.rs
@@ -18,7 +18,7 @@ use std::ffi::OsString;
 use std::fmt;
 use std::io::{self, Read};
 use std::net::SocketAddr;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process;
 use std::str::FromStr;
 #[cfg(feature = "dist-server")]
@@ -746,6 +746,7 @@ pub trait Client {
         compiler_path: &Path,
         weak_key: &str,
         toolchain_packager: Box<dyn pkg::ToolchainPackager>,
-    ) -> SFuture<(Toolchain, Option<(String, std::path::PathBuf)>)>;
+    ) -> SFuture<(Toolchain, Option<(String, PathBuf)>)>;
     fn rewrite_includes_only(&self) -> bool;
+    fn get_custom_toolchain(&self, exe: &PathBuf) -> Option<PathBuf>;
 }
diff --git a/src/server.rs b/src/server.rs
@@ -613,7 +613,11 @@ struct SccacheService<C: CommandCreatorSync> {
     storage: Arc<dyn Storage>,
 
     /// A cache of known compiler info.
-    compilers: Rc<RefCell<HashMap<PathBuf, Option<(Box<dyn Compiler<C>>, FileTime)>>>>,
+    compilers: Rc<
+        RefCell<
+            HashMap<PathBuf, Option<(Box<dyn Compiler<C>>, FileTime, Option<(PathBuf, FileTime)>)>>,
+        >,
+    >,
 
     /// Thread pool to execute work in
     pool: CpuPool,
@@ -831,12 +835,33 @@ where
     ) -> SFuture<Result<Box<dyn Compiler<C>>>> {
         trace!("compiler_info");
         let mtime = ftry!(metadata(&path).map(|attr| FileTime::from_last_modification_time(&attr)));
+        let dist_info = match self.dist_client.get_client() {
+            Ok(Some(ref client)) => {
+                if let Some(archive) = client.get_custom_toolchain(&path) {
+                    match metadata(&archive)
+                        .map(|attr| FileTime::from_last_modification_time(&attr))
+                    {
+                        Ok(mtime) => Some((archive, mtime)),
+                        _ => None,
+                    }
+                } else {
+                    None
+                }
+            }
+            _ => None,
+        };
         //TODO: properly handle rustup overrides. Currently this will
         // cache based on the rustup rustc path, ignoring overrides.
         // https://github.com/mozilla/sccache/issues/87
         let result = match self.compilers.borrow().get(&path) {
-            // It's a hit only if the mtime matches.
-            Some(&Some((ref c, ref cached_mtime))) if *cached_mtime == mtime => Some(c.clone()),
+            // It's a hit only if the mtime and dist archive data matches.
+            Some(&Some((ref c, ref cached_mtime, ref cached_dist_info))) => {
+                if *cached_mtime == mtime && *cached_dist_info == dist_info {
+                    Some(c.clone())
+                } else {
+                    None
+                }
+            }
             _ => None,
         };
         match result {
@@ -851,10 +876,16 @@ where
                 // so do it asynchronously.
                 let me = self.clone();
 
-                let info = get_compiler_info(&self.creator, &path, env, &self.pool);
+                let info = get_compiler_info(
+                    &self.creator,
+                    &path,
+                    env,
+                    &self.pool,
+                    dist_info.clone().map(|(p, _)| p),
+                );
                 Box::new(info.then(move |info| {
                     let map_info = match info {
-                        Ok(ref c) => Some((c.clone(), mtime)),
+                        Ok(ref c) => Some((c.clone(), mtime, dist_info)),
                         Err(_) => None,
                     };
                     me.compilers.borrow_mut().insert(path, map_info);