From 170b96158d2e880737c15f78b3c28ea4b7da9083 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 4 Jul 2015 11:44:49 +0200 Subject: [PATCH 1/6] Added CargoLock implementation It was previously used in the `Concurrent Cargo` [PR](https://goo.gl/pcUvVH). --- Cargo.lock | 22 ++++++++ Cargo.toml | 5 ++ src/cargo/lib.rs | 2 + src/cargo/util/lock.rs | 114 +++++++++++++++++++++++++++++++++++++++++ src/cargo/util/mod.rs | 2 + 5 files changed, 145 insertions(+) create mode 100644 src/cargo/util/lock.rs diff --git a/Cargo.lock b/Cargo.lock index e4de70567d6..e16ef121e31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7,6 +7,8 @@ dependencies = [ "curl 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", "docopt 0.6.67 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", + "errno 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "file-lock 0.0.16 (git+https://github.com/Byron/file-lock?branch=cargo-adjustments)", "filetime 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "flate2 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)", "git2 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", @@ -92,6 +94,26 @@ dependencies = [ "regex 0.1.30 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "errno" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "kernel32-sys 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.1.22 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "file-lock" +version = "0.0.16" +source = "git+https://github.com/Byron/file-lock?branch=cargo-adjustments#0d28d6cff55e1a7f9bcd469d9dae1543cc236d78" +dependencies = [ + "errno 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "gcc 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "filetime" version = "0.1.4" diff --git a/Cargo.toml b/Cargo.toml index 4fbbcf75cbc..13ffc3a311b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,11 @@ time = "0.1" toml = "0.1" url = "0.2" winapi = "0.1" +errno = "0.1.2" + +[dependencies.file-lock] +git = "https://github.com/Byron/file-lock" +branch = "cargo-adjustments" [dev-dependencies] tempdir = "0.3" diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 33c83add0ba..4a903c665c7 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -22,6 +22,8 @@ extern crate threadpool; extern crate time; extern crate toml; extern crate url; +extern crate errno; +extern crate file_lock; use std::env; use std::error::Error; diff --git a/src/cargo/util/lock.rs b/src/cargo/util/lock.rs new file mode 100644 index 00000000000..dbfe2052804 --- /dev/null +++ b/src/cargo/util/lock.rs @@ -0,0 +1,114 @@ +use file_lock::filename::{Mode, ParseError}; +use file_lock::filename::Lock as FileLock; +use file_lock::filename::Error as FileLockError; +use file_lock::fd::Error as LockError; +use errno; +use libc; + +use std::path::PathBuf; +use std::fs; +use std::thread::sleep_ms; + +use util::{Config, CargoError, CargoResult, human, caused_human}; + +pub use file_lock::filename::Kind as LockKind; + +impl From for Box { + fn from(t: FileLockError) -> Self { + Box::new(t) + } +} + +impl From for Box { + fn from(t: ParseError) -> Self { + Box::new(t) + } +} + +impl CargoError for FileLockError { + fn is_human(&self) -> bool { true } +} +impl CargoError for ParseError {} + +pub struct CargoLock { + kind: LockKind, + inner: FileLock, +} + +impl CargoLock { + + pub fn lock_kind(config: &Config) -> CargoResult { + const CONFIG_KEY: &'static str = "build.lock-kind"; + match try!(config.get_string(CONFIG_KEY)).map(|t| t.0) { + None => Ok(LockKind::NonBlocking), + Some(kind_string) => match kind_string.parse() { + Ok(kind) => Ok(kind), + Err(_) => Err(human(format!("Failed to parse value '{}' at configuration key \ + '{}'. Must be one of '{}' and '{}'", + kind_string, CONFIG_KEY, + LockKind::NonBlocking.as_ref(), + LockKind::Blocking.as_ref()))) + } + } + } + + pub fn new(path: PathBuf, kind: LockKind) -> CargoLock { + CargoLock { + kind: kind, + inner: FileLock::new(path, Mode::Write) + } + } + + pub fn new_shared(path: PathBuf, kind: LockKind) -> CargoLock { + CargoLock { + kind: kind, + inner: FileLock::new(path, Mode::Read) + } + } + + pub fn lock(&mut self) -> CargoResult<()> { + // NOTE(ST): This could fail if cargo is run concurrently for the first time + // The only way to prevent it would be to take a lock in a directory which exists. + // This is why we don't try! here, but hope the directory exists when we + // try to create the lock file + { + let lock_dir = self.inner.path().parent().unwrap(); + if let Err(_) = fs::create_dir_all(lock_dir) { + // We might compete to create one or more directories here + // Give the competing process some time to finish. Then we will + // retry, hoping it the creation works (maybe just because the ) + // directory is available already. + // TODO(ST): magic numbers, especially in sleep, will fail at some point ... . + sleep_ms(100); + if let Err(io_err) = fs::create_dir_all(lock_dir) { + // Fail permanently if it still didn't work ... + return Err(caused_human(format!("Failed to create parent directory of \ + lock-file at '{}'", + lock_dir.display()), io_err)); + } + } + } + debug!("About to acquire file lock: '{}'", self.inner.path().display()); + // TODO(ST): This evil hack will just retry until we have the lock or + // fail with an error that is no "Deadlock avoided". + // When there is high intra-process contention thanks to threads, + // this issue occours even though I would hope that we don't try to + // to have multiple threads obtain a lock on the same lock file. + // However, apparently this does happen. + // Even if it does I don't understand why this is a deadlock for him. + // The good news is that building now works in my manual tests. + loop { + match self.inner.any_lock(self.kind.clone()) { + Err(FileLockError::LockError( + LockError::Errno( + errno::Errno(libc::consts::os::posix88::EDEADLK)))) + => { + sleep_ms(100); + continue + }, + Err(any_err) => return Err(any_err.into()), + Ok(()) => return Ok(()) + } + } + } +} diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index ed4d079f972..9d2ba36e8c2 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -15,6 +15,7 @@ pub use self::to_url::ToUrl; pub use self::to_semver::ToSemver; pub use self::vcs::{GitRepo, HgRepo}; pub use self::sha256::Sha256; +pub use self::lock::{CargoLock, LockKind}; pub mod config; pub mod errors; @@ -32,3 +33,4 @@ mod dependency_queue; mod sha256; mod shell_escape; mod vcs; +mod lock; From b4df29163b303b23ffb73f6d42d717044bd91b28 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 4 Jul 2015 11:57:38 +0200 Subject: [PATCH 2/6] Lock every primary cargo command The lock is obtained in a section shared by all cargo sub-command invocations. By default, failure to obtain a lock will result in gracefully aborting the operation. However, it is possible to configure it to wait until a lock can be obtained. --- src/cargo/lib.rs | 14 ++++++++++++-- src/cargo/util/lock.rs | 1 + 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 4a903c665c7..59820260eb7 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -37,7 +37,8 @@ use core::{Shell, MultiShell, ShellConfig, Verbosity}; use core::shell::Verbosity::{Verbose}; use term::color::{BLACK, RED}; -pub use util::{CargoError, CliError, CliResult, human, Config, ChainError}; +pub use util::{CargoError, CliError, CliResult, human, Config, ChainError, + CargoLock, LockKind}; pub mod core; pub mod ops; @@ -104,7 +105,16 @@ fn process(mut callback: F) human(format!("invalid unicode in argument: {:?}", s)) }) }).collect()); - callback(&args, config.as_ref().unwrap()) + + let config_ref = config.as_ref().unwrap(); + let mut fl = CargoLock::new(config_ref.home().join(".global-lock"), + try!(CargoLock::lock_kind(config_ref))); + + try!(fl.lock().chain_error(|| { + human("Failed to obtain global cargo lock") + })); + + callback(&args, config_ref) })(); let mut verbose_shell = shell(Verbose); let mut shell = config.as_ref().map(|s| s.shell()); diff --git a/src/cargo/util/lock.rs b/src/cargo/util/lock.rs index dbfe2052804..1730eb1fbff 100644 --- a/src/cargo/util/lock.rs +++ b/src/cargo/util/lock.rs @@ -38,6 +38,7 @@ pub struct CargoLock { impl CargoLock { pub fn lock_kind(config: &Config) -> CargoResult { + // TODO(ST): rename config key to something more suitable const CONFIG_KEY: &'static str = "build.lock-kind"; match try!(config.get_string(CONFIG_KEY)).map(|t| t.0) { None => Ok(LockKind::NonBlocking), From 4a2ea699fe58383852282132047da9f37e6908f1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 4 Jul 2015 14:14:13 +0200 Subject: [PATCH 3/6] Added test for global cargo lock However, it suffers from a race condition that make it succeed only occasionally. The lock code was moved into cargo's main execute function, which should be better as the lock will be obtained even sooner after cargo comes up. Removed some special-case code which made the lock work better in a multi-threaded environment, which now is not needed anymore due to the changed lock granularity. This also allows to remove `errno` crate. --- Cargo.lock | 1 - Cargo.toml | 1 - src/bin/cargo.rs | 10 +++++++++- src/cargo/lib.rs | 15 ++------------- src/cargo/util/lock.rs | 25 +------------------------ tests/test_cargo_build_lib.rs | 26 ++++++++++++++++++++++++++ 6 files changed, 38 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e16ef121e31..c5a84f5a93c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7,7 +7,6 @@ dependencies = [ "curl 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", "docopt 0.6.67 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", - "errno 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "file-lock 0.0.16 (git+https://github.com/Byron/file-lock?branch=cargo-adjustments)", "filetime 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "flate2 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index 13ffc3a311b..f1875635a5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,6 @@ time = "0.1" toml = "0.1" url = "0.2" winapi = "0.1" -errno = "0.1.2" [dependencies.file-lock] git = "https://github.com/Byron/file-lock" diff --git a/src/bin/cargo.rs b/src/bin/cargo.rs index 73646756d31..be6825be423 100644 --- a/src/bin/cargo.rs +++ b/src/bin/cargo.rs @@ -14,7 +14,8 @@ use std::process::Command; use cargo::{execute_main_without_stdin, handle_error, shell}; use cargo::core::MultiShell; -use cargo::util::{CliError, CliResult, lev_distance, Config}; +use cargo::util::{CliError, CliResult, lev_distance, Config, CargoLock, human, + ChainError}; #[derive(RustcDecodable)] struct Flags { @@ -91,6 +92,13 @@ macro_rules! each_subcommand{ ($mac:ident) => ({ on this top-level information. */ fn execute(flags: Flags, config: &Config) -> CliResult> { + let mut fl = CargoLock::new(config.home().join(".global-lock"), + try!(CargoLock::lock_kind(config))); + + try!(fl.lock().chain_error(|| { + human("Failed to obtain global cargo lock") + })); + try!(config.shell().set_verbosity(flags.flag_verbose, flags.flag_quiet)); init_git_transports(config); diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 59820260eb7..a104da6611f 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -22,7 +22,6 @@ extern crate threadpool; extern crate time; extern crate toml; extern crate url; -extern crate errno; extern crate file_lock; use std::env; @@ -37,8 +36,7 @@ use core::{Shell, MultiShell, ShellConfig, Verbosity}; use core::shell::Verbosity::{Verbose}; use term::color::{BLACK, RED}; -pub use util::{CargoError, CliError, CliResult, human, Config, ChainError, - CargoLock, LockKind}; +pub use util::{CargoError, CliError, CliResult, human, Config, ChainError}; pub mod core; pub mod ops; @@ -105,16 +103,7 @@ fn process(mut callback: F) human(format!("invalid unicode in argument: {:?}", s)) }) }).collect()); - - let config_ref = config.as_ref().unwrap(); - let mut fl = CargoLock::new(config_ref.home().join(".global-lock"), - try!(CargoLock::lock_kind(config_ref))); - - try!(fl.lock().chain_error(|| { - human("Failed to obtain global cargo lock") - })); - - callback(&args, config_ref) + callback(&args, config.as_ref().unwrap()) })(); let mut verbose_shell = shell(Verbose); let mut shell = config.as_ref().map(|s| s.shell()); diff --git a/src/cargo/util/lock.rs b/src/cargo/util/lock.rs index 1730eb1fbff..2afd07cc253 100644 --- a/src/cargo/util/lock.rs +++ b/src/cargo/util/lock.rs @@ -1,9 +1,6 @@ use file_lock::filename::{Mode, ParseError}; use file_lock::filename::Lock as FileLock; use file_lock::filename::Error as FileLockError; -use file_lock::fd::Error as LockError; -use errno; -use libc; use std::path::PathBuf; use std::fs; @@ -90,26 +87,6 @@ impl CargoLock { } } debug!("About to acquire file lock: '{}'", self.inner.path().display()); - // TODO(ST): This evil hack will just retry until we have the lock or - // fail with an error that is no "Deadlock avoided". - // When there is high intra-process contention thanks to threads, - // this issue occours even though I would hope that we don't try to - // to have multiple threads obtain a lock on the same lock file. - // However, apparently this does happen. - // Even if it does I don't understand why this is a deadlock for him. - // The good news is that building now works in my manual tests. - loop { - match self.inner.any_lock(self.kind.clone()) { - Err(FileLockError::LockError( - LockError::Errno( - errno::Errno(libc::consts::os::posix88::EDEADLK)))) - => { - sleep_ms(100); - continue - }, - Err(any_err) => return Err(any_err.into()), - Ok(()) => return Ok(()) - } - } + Ok(try!(self.inner.any_lock(self.kind.clone()))) } } diff --git a/tests/test_cargo_build_lib.rs b/tests/test_cargo_build_lib.rs index 476d2f14bdf..66a73b85d88 100644 --- a/tests/test_cargo_build_lib.rs +++ b/tests/test_cargo_build_lib.rs @@ -53,6 +53,32 @@ test!(build_with_no_lib { .with_stderr("no library targets found")); }); +test!(build_global_lock { + + let p = project("foo") + .file("Cargo.toml", r#" + [package] + + name = "foo" + version = "0.0.1" + authors = ["wycats@example.com"] + "#) + .file("src/main.rs", r#" + fn main() {} + "#) + .file("src/lib.rs", r#" "#); + + p.build(); + + let mut bg_process = p.cargo("build"); + let fg_process = bg_process.clone(); + let mut bg_process_handle = bg_process.build_command().spawn().unwrap(); + + assert_eq!(fg_process.build_command().output().unwrap().status.code().unwrap(), 101); + assert!(bg_process_handle.wait().unwrap().success()); +}); + + test!(build_with_relative_cargo_home_path { let p = project("foo") .file("Cargo.toml", r#" From 9c2124e3db9e14d173b039481500cd7feaa333fa Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Jul 2015 10:11:44 +0200 Subject: [PATCH 4/6] CargoLock LockKind is now specified per lock call * removed access to configuration, and will instead default to blocking semantics. This caused a change in the API of the `CargoLock` type, that generally reduced complexity. --- src/bin/cargo.rs | 7 +++---- src/cargo/util/lock.rs | 34 ++++------------------------------ 2 files changed, 7 insertions(+), 34 deletions(-) diff --git a/src/bin/cargo.rs b/src/bin/cargo.rs index be6825be423..55fd18b2f1e 100644 --- a/src/bin/cargo.rs +++ b/src/bin/cargo.rs @@ -14,7 +14,7 @@ use std::process::Command; use cargo::{execute_main_without_stdin, handle_error, shell}; use cargo::core::MultiShell; -use cargo::util::{CliError, CliResult, lev_distance, Config, CargoLock, human, +use cargo::util::{CliError, CliResult, lev_distance, Config, CargoLock, LockKind, human, ChainError}; #[derive(RustcDecodable)] @@ -92,10 +92,9 @@ macro_rules! each_subcommand{ ($mac:ident) => ({ on this top-level information. */ fn execute(flags: Flags, config: &Config) -> CliResult> { - let mut fl = CargoLock::new(config.home().join(".global-lock"), - try!(CargoLock::lock_kind(config))); + let mut fl = CargoLock::new(config.home().join(".global-lock")); - try!(fl.lock().chain_error(|| { + try!(fl.lock(LockKind::Blocking).chain_error(|| { human("Failed to obtain global cargo lock") })); diff --git a/src/cargo/util/lock.rs b/src/cargo/util/lock.rs index 2afd07cc253..71af379535e 100644 --- a/src/cargo/util/lock.rs +++ b/src/cargo/util/lock.rs @@ -6,7 +6,7 @@ use std::path::PathBuf; use std::fs; use std::thread::sleep_ms; -use util::{Config, CargoError, CargoResult, human, caused_human}; +use util::{CargoError, CargoResult, caused_human}; pub use file_lock::filename::Kind as LockKind; @@ -28,43 +28,17 @@ impl CargoError for FileLockError { impl CargoError for ParseError {} pub struct CargoLock { - kind: LockKind, inner: FileLock, } impl CargoLock { - - pub fn lock_kind(config: &Config) -> CargoResult { - // TODO(ST): rename config key to something more suitable - const CONFIG_KEY: &'static str = "build.lock-kind"; - match try!(config.get_string(CONFIG_KEY)).map(|t| t.0) { - None => Ok(LockKind::NonBlocking), - Some(kind_string) => match kind_string.parse() { - Ok(kind) => Ok(kind), - Err(_) => Err(human(format!("Failed to parse value '{}' at configuration key \ - '{}'. Must be one of '{}' and '{}'", - kind_string, CONFIG_KEY, - LockKind::NonBlocking.as_ref(), - LockKind::Blocking.as_ref()))) - } - } - } - - pub fn new(path: PathBuf, kind: LockKind) -> CargoLock { + pub fn new(path: PathBuf) -> CargoLock { CargoLock { - kind: kind, inner: FileLock::new(path, Mode::Write) } } - pub fn new_shared(path: PathBuf, kind: LockKind) -> CargoLock { - CargoLock { - kind: kind, - inner: FileLock::new(path, Mode::Read) - } - } - - pub fn lock(&mut self) -> CargoResult<()> { + pub fn lock(&mut self, kind: LockKind) -> CargoResult<()> { // NOTE(ST): This could fail if cargo is run concurrently for the first time // The only way to prevent it would be to take a lock in a directory which exists. // This is why we don't try! here, but hope the directory exists when we @@ -87,6 +61,6 @@ impl CargoLock { } } debug!("About to acquire file lock: '{}'", self.inner.path().display()); - Ok(try!(self.inner.any_lock(self.kind.clone()))) + Ok(try!(self.inner.any_lock(kind))) } } From b900af971fe45b90fc4d2934a749b366a2559906 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Jul 2015 10:31:16 +0200 Subject: [PATCH 5/6] Moved LockError related code to errors.rs That way, all error related code can be found in one module, and isn't sprinkled all over the place. --- src/cargo/util/errors.rs | 8 ++++++++ src/cargo/util/lock.rs | 22 ++-------------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 4c24b34c59b..da32305f31c 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -12,6 +12,7 @@ use curl; use git2; use toml; use url; +use file_lock; pub type CargoResult = Result>; @@ -262,12 +263,18 @@ from_error! { url::ParseError, toml::DecodeError, ffi::NulError, + file_lock::filename::Error, + file_lock::filename::ParseError, } impl From> for Box { fn from(t: Human) -> Box { Box::new(t) } } +impl CargoError for file_lock::filename::Error { + fn is_human(&self) -> bool { true } +} + impl CargoError for semver::ReqParseError {} impl CargoError for io::Error {} impl CargoError for git2::Error {} @@ -279,6 +286,7 @@ impl CargoError for toml::Error {} impl CargoError for toml::DecodeError {} impl CargoError for url::ParseError {} impl CargoError for ffi::NulError {} +impl CargoError for file_lock::filename::ParseError {} // ============================================================================= // Construction helpers diff --git a/src/cargo/util/lock.rs b/src/cargo/util/lock.rs index 71af379535e..5aa3e243e2f 100644 --- a/src/cargo/util/lock.rs +++ b/src/cargo/util/lock.rs @@ -1,32 +1,14 @@ -use file_lock::filename::{Mode, ParseError}; +use file_lock::filename::Mode; use file_lock::filename::Lock as FileLock; -use file_lock::filename::Error as FileLockError; use std::path::PathBuf; use std::fs; use std::thread::sleep_ms; -use util::{CargoError, CargoResult, caused_human}; +use util::{CargoResult, caused_human}; pub use file_lock::filename::Kind as LockKind; -impl From for Box { - fn from(t: FileLockError) -> Self { - Box::new(t) - } -} - -impl From for Box { - fn from(t: ParseError) -> Self { - Box::new(t) - } -} - -impl CargoError for FileLockError { - fn is_human(&self) -> bool { true } -} -impl CargoError for ParseError {} - pub struct CargoLock { inner: FileLock, } From b337c5ee832098c81d96df3bbf38bd2843734f44 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Jul 2015 10:46:42 +0200 Subject: [PATCH 6/6] Improved error handling in CargoLock.lock() * just fail if the directory creation fails. This could happen if there is a race, but such a race would only possibly occour if cargo has never been invoked before on a particular CARGO_HOME * use try! + chain_error + human instead of more complex error handling --- src/cargo/util/lock.rs | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/cargo/util/lock.rs b/src/cargo/util/lock.rs index 5aa3e243e2f..a15c2859160 100644 --- a/src/cargo/util/lock.rs +++ b/src/cargo/util/lock.rs @@ -3,9 +3,8 @@ use file_lock::filename::Lock as FileLock; use std::path::PathBuf; use std::fs; -use std::thread::sleep_ms; -use util::{CargoResult, caused_human}; +use util::{CargoResult, ChainError, human}; pub use file_lock::filename::Kind as LockKind; @@ -21,26 +20,12 @@ impl CargoLock { } pub fn lock(&mut self, kind: LockKind) -> CargoResult<()> { - // NOTE(ST): This could fail if cargo is run concurrently for the first time - // The only way to prevent it would be to take a lock in a directory which exists. - // This is why we don't try! here, but hope the directory exists when we - // try to create the lock file { let lock_dir = self.inner.path().parent().unwrap(); - if let Err(_) = fs::create_dir_all(lock_dir) { - // We might compete to create one or more directories here - // Give the competing process some time to finish. Then we will - // retry, hoping it the creation works (maybe just because the ) - // directory is available already. - // TODO(ST): magic numbers, especially in sleep, will fail at some point ... . - sleep_ms(100); - if let Err(io_err) = fs::create_dir_all(lock_dir) { - // Fail permanently if it still didn't work ... - return Err(caused_human(format!("Failed to create parent directory of \ - lock-file at '{}'", - lock_dir.display()), io_err)); - } - } + try!(fs::create_dir_all(lock_dir).chain_error(|| { + human(format!("Failed to create parent directory of lock-file at '{}'", + lock_dir.display())) + })); } debug!("About to acquire file lock: '{}'", self.inner.path().display()); Ok(try!(self.inner.any_lock(kind)))