Lock unpacking instead of local registry tarball.

Author: hugwijst

src/cargo/sources/registry/local.rs

@@ -11,15 +11,15 @@ use hex;
 
 pub struct LocalRegistry<'cfg> {
     index_path: Filesystem,
-    cache_path: Filesystem,
     root: Filesystem,
+    src_path: Filesystem,
     config: &'cfg Config,
 }
 
 impl<'cfg> LocalRegistry<'cfg> {
     pub fn new(root: &Path, config: &'cfg Config, name: &str) -> LocalRegistry<'cfg> {
         LocalRegistry {
-            cache_path: config.registry_cache_path().join(name),
+            src_path: config.registry_source_path().join(name),
             index_path: Filesystem::new(root.join("index")),
             root: Filesystem::new(root.to_path_buf()),
             config,
@@ -70,39 +70,38 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> {
     }
 
     fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult<MaybeLock> {
-        let filename = format!("{}-{}.crate", pkg.name(), pkg.version());
-
-        // Attempt to open an read-only lock first to avoid an exclusive write lock.
-        //
-        // If this fails then we fall through to the exclusive path where we copy
-        // the file.
-        if let Ok(dst) = self.cache_path.open_ro(&filename, self.config, &filename) {
-            let meta = dst.file().metadata()?;
-            if meta.len() > 0 {
-                return Ok(MaybeLock::Ready(dst));
-            }
+        let crate_file = format!("{}-{}.crate", pkg.name(), pkg.version());
+        let mut crate_file = self.root.open_ro(&crate_file, self.config, "crate file")?;
+
+        // If we've already got an unpacked version of this crate, then skip the
+        // checksum below as it is in theory already verified.
+        let dst = format!("{}-{}", pkg.name(), pkg.version());
+        if self.src_path.join(dst).into_path_unlocked().exists() {
+            return Ok(MaybeLock::Ready(crate_file));
         }
 
         self.config.shell().status("Unpacking", pkg)?;
 
-        // Verify the checksum and copy over the .crate.
-        let mut buf = Vec::new();
-        let mut crate_file_source = self.root.open_ro(&filename, self.config, "crate file")?;
-        let _ = crate_file_source
-            .read_to_end(&mut buf)
-            .chain_err(|| format!("failed to read `{}`", crate_file_source.path().display()))?;
-
+        // We don't actually need to download anything per-se, we just need to
+        // verify the checksum matches the .crate file itself.
         let mut state = Sha256::new();
-        state.update(&buf);
+        let mut buf = [0; 64 * 1024];
+        loop {
+            let n = crate_file
+                .read(&mut buf)
+                .chain_err(|| format!("failed to read `{}`", crate_file.path().display()))?;
+            if n == 0 {
+                break;
+            }
+            state.update(&buf[..n]);
+        }
         if hex::encode(state.finish()) != checksum {
             failure::bail!("failed to verify the checksum of `{}`", pkg)
         }
 
-        let mut dst = self.cache_path.open_rw(&filename, self.config, &filename)?;
-        dst.write_all(&buf)?;
-        dst.seek(SeekFrom::Start(0))?;
+        crate_file.seek(SeekFrom::Start(0))?;
 
-        Ok(MaybeLock::Ready(dst))
+        Ok(MaybeLock::Ready(crate_file))
     }
 
     fn finish_download(

src/cargo/sources/registry/mod.rs

@@ -160,7 +160,7 @@
 
 use std::borrow::Cow;
 use std::collections::BTreeMap;
-use std::fs::File;
+use std::io::Write;
 use std::path::{Path, PathBuf};
 
 use flate2::read::GzDecoder;
@@ -426,23 +426,28 @@ impl<'cfg> RegistrySource<'cfg> {
     ///
     /// No action is taken if the source looks like it's already unpacked.
     fn unpack_package(&self, pkg: PackageId, tarball: &FileLock) -> CargoResult<PathBuf> {
-        let dst = self
-            .src_path
-            .join(&format!("{}-{}", pkg.name(), pkg.version()));
-        dst.create_dir()?;
-        // Note that we've already got the `tarball` locked above, and that
-        // implies a lock on the unpacked destination as well, so this access
-        // via `into_path_unlocked` should be ok.
-        let dst = dst.into_path_unlocked();
-        let ok = dst.join(".cargo-ok");
-        if ok.exists() {
-            return Ok(dst);
+        // The `.cargo-ok` file is used to track if the source is already
+        // unpacked and to lock the directory for unpacking.
+        let mut ok = {
+            let package_dir = format!("{}-{}", pkg.name(), pkg.version());
+            let dst = self
+                .src_path
+                .join(&package_dir);
+            dst.create_dir()?;
+            dst.open_rw(".cargo-ok", self.config, &package_dir)?
+        };
+        let unpack_dir = ok.parent().to_path_buf();
+
+        // If the file has any data, assume the source is already unpacked.
+        let meta = ok.file().metadata()?;
+        if meta.len() > 0 {
+            return Ok(unpack_dir);
         }
 
         let gz = GzDecoder::new(tarball.file());
         let mut tar = Archive::new(gz);
-        let prefix = dst.file_name().unwrap();
-        let parent = dst.parent().unwrap();
+        let prefix = unpack_dir.file_name().unwrap();
+        let parent = unpack_dir.parent().unwrap();
         for entry in tar.entries()? {
             let mut entry = entry.chain_err(|| "failed to iterate over archive")?;
             let entry_path = entry
@@ -470,8 +475,11 @@ impl<'cfg> RegistrySource<'cfg> {
                 .unpack_in(parent)
                 .chain_err(|| format!("failed to unpack entry at `{}`", entry_path.display()))?;
         }
-        File::create(&ok)?;
-        Ok(dst)
+
+        // Write to the lock file to indicate that unpacking was successful.
+        write!(ok, "ok")?;
+
+        Ok(unpack_dir)
     }
 
     fn do_update(&mut self) -> CargoResult<()> {