Protect against concurrent access to Cargo.lock

CI tests seen to be spurious failing on OSX due to this, I believe it's because
one process is concurrently writing out Cargo.lock while the other is trying to
read it in, so one of them gets a corrupt version.

This would ideally be fixed from `pkg.root()` returning a `Filesystem`, but that
was unfortunately such an invasive change that it seemed untenable. Additionally
we generally don't write files into the source tree, so if this is the only
instance it's perhaps not so bad trying to be robust in to the future.

Author: alexcrichton

src/cargo/ops/cargo_generate_lockfile.rs

@@ -23,7 +23,7 @@ pub fn generate_lockfile(manifest_path: &Path, config: &Config)
     let resolve = try!(ops::resolve_with_previous(&mut registry, &package,
                                                   Method::Everything,
                                                   None, None));
-    try!(ops::write_pkg_lockfile(&package, &resolve));
+    try!(ops::write_pkg_lockfile(&package, &resolve, config));
     Ok(())
 }
 
@@ -105,7 +105,7 @@ pub fn update_lockfile(manifest_path: &Path,
         }
     }
 
-    try!(ops::write_pkg_lockfile(&package, &resolve));
+    try!(ops::write_pkg_lockfile(&package, &resolve, opts.config));
     return Ok(());
 
     fn fill_with_deps<'a>(resolve: &'a Resolve, dep: &'a PackageId,

src/cargo/ops/cargo_pkgid.rs

@@ -8,9 +8,7 @@ pub fn pkgid(manifest_path: &Path,
              spec: Option<&str>,
              config: &Config) -> CargoResult<PackageIdSpec> {
     let package = try!(Package::for_path(manifest_path, config));
-
-    let lockfile = package.root().join("Cargo.lock");
-    let resolve = match try!(ops::load_lockfile(&lockfile, &package, config)) {
+    let resolve = match try!(ops::load_pkg_lockfile(&package, config)) {
         Some(resolve) => resolve,
         None => bail!("a Cargo.lock must exist for this command"),
     };

src/cargo/ops/lockfile.rs

@@ -1,45 +1,39 @@
-use std::fs::File;
 use std::io::prelude::*;
-use std::path::Path;
 
 use rustc_serialize::{Encodable, Decodable};
 use toml::{self, Encoder, Value};
 
 use core::{Resolve, resolver, Package};
-use util::{CargoResult, ChainError, human, paths, Config};
+use util::{CargoResult, ChainError, human, Config, Filesystem};
 use util::toml as cargo_toml;
 
 pub fn load_pkg_lockfile(pkg: &Package, config: &Config)
                          -> CargoResult<Option<Resolve>> {
-    let lockfile = pkg.root().join("Cargo.lock");
-    load_lockfile(&lockfile, pkg, config).chain_error(|| {
-        human(format!("failed to parse lock file at: {}", lockfile.display()))
-    })
-}
+    if !pkg.root().join("Cargo.lock").exists() {
+        return Ok(None)
+    }
 
-pub fn load_lockfile(path: &Path, pkg: &Package, config: &Config)
-                     -> CargoResult<Option<Resolve>> {
-    // If there is no lockfile, return none.
-    let mut f = match File::open(path) {
-        Ok(f) => f,
-        Err(_) => return Ok(None)
-    };
+    let root = Filesystem::new(pkg.root().to_path_buf());
+    let mut f = try!(root.open_ro("Cargo.lock", config, "Cargo.lock file"));
 
     let mut s = String::new();
-    try!(f.read_to_string(&mut s));
-
-    let table = toml::Value::Table(try!(cargo_toml::parse(&s, path)));
-    let mut d = toml::Decoder::new(table);
-    let v: resolver::EncodableResolve = try!(Decodable::decode(&mut d));
-    Ok(Some(try!(v.to_resolve(pkg, config))))
-}
-
-pub fn write_pkg_lockfile(pkg: &Package, resolve: &Resolve) -> CargoResult<()> {
-    let loc = pkg.root().join("Cargo.lock");
-    write_lockfile(&loc, resolve)
+    try!(f.read_to_string(&mut s).chain_error(|| {
+        human(format!("failed to read file: {}", f.path().display()))
+    }));
+
+    (|| {
+        let table = toml::Value::Table(try!(cargo_toml::parse(&s, f.path())));
+        let mut d = toml::Decoder::new(table);
+        let v: resolver::EncodableResolve = try!(Decodable::decode(&mut d));
+        Ok(Some(try!(v.to_resolve(pkg, config))))
+    }).chain_error(|| {
+        human(format!("failed to parse lock file at: {}", f.path().display()))
+    })
 }
 
-pub fn write_lockfile(dst: &Path, resolve: &Resolve) -> CargoResult<()> {
+pub fn write_pkg_lockfile(pkg: &Package,
+                          resolve: &Resolve,
+                          config: &Config) -> CargoResult<()> {
     let mut e = Encoder::new();
     resolve.encode(&mut e).unwrap();
 
@@ -69,20 +63,36 @@ pub fn write_lockfile(dst: &Path, resolve: &Resolve) -> CargoResult<()> {
         None => {}
     }
 
+    let root = Filesystem::new(pkg.root().to_path_buf());
+
     // Load the original lockfile if it exists.
-    if let Ok(orig) = paths::read(dst) {
+    //
+    // If the lockfile contents haven't changed so don't rewrite it. This is
+    // helpful on read-only filesystems.
+    let orig = root.open_ro("Cargo.lock", config, "Cargo.lock file");
+    let orig = orig.and_then(|mut f| {
+        let mut s = String::new();
+        try!(f.read_to_string(&mut s));
+        Ok(s)
+    });
+    if let Ok(orig) = orig {
         if has_crlf_line_endings(&orig) {
             out = out.replace("\n", "\r\n");
         }
         if out == orig {
-            // The lockfile contents haven't changed so don't rewrite it.
-            // This is helpful on read-only filesystems.
             return Ok(())
         }
     }
 
-    try!(paths::write(dst, out.as_bytes()));
-    Ok(())
+    // Ok, if that didn't work just write it out
+    root.open_rw("Cargo.lock", config, "Cargo.lock file").and_then(|mut f| {
+        try!(f.file().set_len(0));
+        try!(f.write_all(out.as_bytes()));
+        Ok(())
+    }).chain_error(|| {
+        human(format!("failed to write {}",
+                      pkg.root().join("Cargo.lock").display()))
+    })
 }
 
 fn has_crlf_line_endings(s: &str) -> bool {

src/cargo/ops/mod.rs

@@ -13,8 +13,7 @@ pub use self::cargo_doc::{doc, DocOptions};
 pub use self::cargo_generate_lockfile::{generate_lockfile};
 pub use self::cargo_generate_lockfile::{update_lockfile};
 pub use self::cargo_generate_lockfile::UpdateOptions;
-pub use self::lockfile::{load_lockfile, load_pkg_lockfile};
-pub use self::lockfile::{write_lockfile, write_pkg_lockfile};
+pub use self::lockfile::{load_pkg_lockfile, write_pkg_lockfile};
 pub use self::cargo_test::{run_tests, run_benches, TestOptions};
 pub use self::cargo_package::package;
 pub use self::registry::{publish, registry_configuration, RegistryConfig};

src/cargo/ops/resolve.rs

@@ -20,7 +20,7 @@ pub fn resolve_pkg(registry: &mut PackageRegistry,
                                              Method::Everything,
                                              prev.as_ref(), None));
     if package.package_id().source_id().is_path() {
-        try!(ops::write_pkg_lockfile(package, &resolve));
+        try!(ops::write_pkg_lockfile(package, &resolve, config));
     }
     Ok(resolve)
 }

tests/test_cargo_generate_lockfile.rs

@@ -62,6 +62,7 @@ test!(adding_and_removing_packages {
     assert!(lock2 != lock3);
 
     // remove the dep
+    println!("lock4");
     File::create(&toml).unwrap().write_all(br#"
         [package]
         name = "foo"

tests/tests.rs

@@ -93,6 +93,7 @@ fn process<T: AsRef<OsStr>>(t: T) -> cargo::util::ProcessBuilder {
     p.cwd(&support::paths::root())
      .env("HOME", &support::paths::home())
      .env_remove("CARGO_HOME")
+     .env_remove("RUSTC")
      .env_remove("XDG_CONFIG_HOME")      // see #2345
      .env("GIT_CONFIG_NOSYSTEM", "1")    // keep trying to sandbox ourselves
      .env_remove("CARGO_TARGET_DIR")     // we assume 'target'