Thread through last update time to index cache

Removed in the previous commit this now adds dedicated tracking for the
last update.

Author: alexcrichton

src/cargo/sources/registry/index.rs

@@ -316,14 +316,7 @@ impl<'cfg> RegistryIndex<'cfg> {
         // let root = self.config.assert_package_cache_locked(&self.path);
         let root = load.assert_index_locked(&self.path);
         let cache_root = root.join(".cache");
-
-        // TODO: comment
-        let lock_mtime = None;
-        // let lock_mtime = lock
-        //     .as_ref()
-        //     .and_then(|l| l.file().metadata().ok())
-        //     .map(|t| FileTime::from_last_modification_time(&t));
-
+        let last_index_update = load.last_modified();;
 
         // See module comment in `registry/mod.rs` for why this is structured
         // the way it is.
@@ -345,7 +338,7 @@ impl<'cfg> RegistryIndex<'cfg> {
         // along the way produce helpful "did you mean?" suggestions.
         for path in UncanonicalizedIter::new(&raw_path).take(1024) {
             let summaries = Summaries::parse(
-                lock_mtime,
+                last_index_update,
                 &root,
                 &cache_root,
                 path.as_ref(),
@@ -465,7 +458,7 @@ impl Summaries {
     /// for `relative` from the underlying index (aka typically libgit2 with
     /// crates.io) and then parse everything in there.
     ///
-    /// * `lock_mtime` - this is a file modification time where if any cache
+    /// * `last_index_update` - this is a file modification time where if any cache
     ///   file is older than this the cache should be considered out of date and
     ///   needs to be rebuilt.
     /// * `root` - this is the root argument passed to `load`
@@ -478,7 +471,7 @@ impl Summaries {
     /// * `load` - the actual index implementation which may be very slow to
     ///   call. We avoid this if we can.
     pub fn parse(
-        lock_mtime: Option<FileTime>,
+        last_index_update: Option<FileTime>,
         root: &Path,
         cache_root: &Path,
         relative: &Path,
@@ -490,12 +483,12 @@ impl Summaries {
         // of reasons, but consider all of them non-fatal and just log their
         // occurrence in case anyone is debugging anything.
         let cache_path = cache_root.join(relative);
-        if let Some(lock_mtime) = lock_mtime {
+        if let Some(last_index_update) = last_index_update {
             match File::open(&cache_path) {
                 Ok(file) => {
                     let metadata = file.metadata()?;
                     let cache_mtime = FileTime::from_last_modification_time(&metadata);
-                    if cache_mtime > lock_mtime {
+                    if cache_mtime > last_index_update {
                         log::debug!("cache for {:?} is fresh", relative);
                         match Summaries::parse_cache(&file, &metadata) {
                             Ok(s) => return Ok(Some(s)),
@@ -560,7 +553,10 @@ impl Summaries {
         //
         // This is opportunistic so we ignore failure here but are sure to log
         // something in case of error.
-        if fs::create_dir_all(cache_path.parent().unwrap()).is_ok() {
+        //
+        // Note that we also skip this when `last_index_update` is `None` because it
+        // means we can't handle the cache anyway.
+        if last_index_update.is_some() && fs::create_dir_all(cache_path.parent().unwrap()).is_ok() {
             let path = Filesystem::new(cache_path.clone());
             config.assert_package_cache_locked(&path);
             if let Err(e) = fs::write(cache_path, cache_bytes) {

src/cargo/sources/registry/local.rs

@@ -1,14 +1,14 @@
-use std::fs::File;
-use std::io::SeekFrom;
-use std::io::prelude::*;
-use std::path::Path;
-
 use crate::core::PackageId;
 use crate::sources::registry::{MaybeLock, RegistryConfig, RegistryData};
 use crate::util::errors::{CargoResult, CargoResultExt};
 use crate::util::paths;
 use crate::util::{Config, Filesystem, Sha256};
+use filetime::FileTime;
 use hex;
+use std::fs::File;
+use std::io::prelude::*;
+use std::io::SeekFrom;
+use std::path::Path;
 
 pub struct LocalRegistry<'cfg> {
     index_path: Filesystem,
@@ -43,6 +43,10 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> {
         path.as_path_unlocked()
     }
 
+    fn last_modified(&self) -> Option<FileTime> {
+        None
+    }
+
     fn load(
         &self,
         root: &Path,

src/cargo/sources/registry/mod.rs

@@ -165,6 +165,7 @@ use std::fs::{File, OpenOptions};
 use std::io::Write;
 use std::path::{Path, PathBuf};
 
+use filetime::FileTime;
 use flate2::read::GzDecoder;
 use log::debug;
 use semver::{Version, VersionReq};
@@ -371,6 +372,7 @@ pub trait RegistryData {
         true
     }
     fn assert_index_locked<'a>(&self, path: &'a Filesystem) -> &'a Path;
+    fn last_modified(&self) -> Option<FileTime>;
 }
 
 pub enum MaybeLock {

src/cargo/sources/registry/remote.rs

@@ -1,3 +1,13 @@
+use crate::core::{PackageId, SourceId};
+use crate::sources::git;
+use crate::sources::registry::MaybeLock;
+use crate::sources::registry::{RegistryConfig, RegistryData, CRATE_TEMPLATE, VERSION_TEMPLATE};
+use crate::util::errors::{CargoResult, CargoResultExt};
+use crate::util::{Config, Filesystem, Sha256};
+use crate::util::paths;
+use filetime::FileTime;
+use lazycell::LazyCell;
+use log::{debug, trace};
 use std::cell::{Cell, Ref, RefCell};
 use std::fmt::Write as FmtWrite;
 use std::fs::{self, File, OpenOptions};
@@ -7,16 +17,6 @@ use std::mem;
 use std::path::Path;
 use std::str;
 
-use lazycell::LazyCell;
-use log::{debug, trace};
-
-use crate::core::{PackageId, SourceId};
-use crate::sources::git;
-use crate::sources::registry::MaybeLock;
-use crate::sources::registry::{RegistryConfig, RegistryData, CRATE_TEMPLATE, VERSION_TEMPLATE};
-use crate::util::errors::{CargoResult, CargoResultExt};
-use crate::util::{Config, Filesystem, Sha256};
-
 pub struct RemoteRegistry<'cfg> {
     index_path: Filesystem,
     cache_path: Filesystem,
@@ -25,6 +25,7 @@ pub struct RemoteRegistry<'cfg> {
     tree: RefCell<Option<git2::Tree<'static>>>,
     repo: LazyCell<git2::Repository>,
     head: Cell<Option<git2::Oid>>,
+    last_updated: Cell<Option<FileTime>>,
 }
 
 impl<'cfg> RemoteRegistry<'cfg> {
@@ -37,6 +38,7 @@ impl<'cfg> RemoteRegistry<'cfg> {
             tree: RefCell::new(None),
             repo: LazyCell::new(),
             head: Cell::new(None),
+            last_updated: Cell::new(None),
         }
     }
 
@@ -123,6 +125,8 @@ impl<'cfg> RemoteRegistry<'cfg> {
     }
 }
 
+const LAST_UPDATED_FILE: &str = ".last-updated";
+
 impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
     fn prepare(&self) -> CargoResult<()> {
         self.repo()?; // create intermediate dirs and initialize the repo
@@ -137,6 +141,16 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
         self.config.assert_package_cache_locked(path)
     }
 
+    fn last_modified(&self) -> Option<FileTime> {
+        if let Some(time) = self.last_updated.get() {
+            return Some(time);
+        }
+        let path = self.config.assert_package_cache_locked(&self.index_path);
+        let mtime = paths::mtime(&path.join(LAST_UPDATED_FILE)).ok();
+        self.last_updated.set(mtime);
+        self.last_updated.get()
+    }
+
     fn load(
         &self,
         _root: &Path,
@@ -209,7 +223,8 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
         self.prepare()?;
         self.head.set(None);
         *self.tree.borrow_mut() = None;
-        self.config.assert_package_cache_locked(&self.index_path);
+        self.last_updated.set(None);
+        let path = self.config.assert_package_cache_locked(&self.index_path);
         self.config
             .shell()
             .status("Updating", self.source_id.display_index())?;
@@ -222,6 +237,10 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
             .chain_err(|| format!("failed to fetch `{}`", url))?;
         self.config.updated_sources().insert(self.source_id);
 
+        // Create a dummy file to record the mtime for when we updated the
+        // index.
+        File::create(&path.join(LAST_UPDATED_FILE))?;
+
         Ok(())
     }
 

tests/testsuite/concurrent.rs

@@ -453,15 +453,15 @@ fn debug_release_ok() {
     let a = a.join().unwrap();
 
     execs()
-        .with_stderr(
+        .with_stderr_contains(
             "\
 [COMPILING] foo v0.0.1 [..]
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
 ",
         )
         .run_output(&a);
     execs()
-        .with_stderr(
+        .with_stderr_contains(
             "\
 [COMPILING] foo v0.0.1 [..]
 [FINISHED] release [optimized] target(s) in [..]