From 1a6bad8e5bc822ce345903ab03b196694bad6c54 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Fri, 22 Apr 2016 17:49:12 -0500 Subject: [PATCH 1/2] cargo-install: prefer building artifacts in the system temporary directory and each cargo-install instance creates and uses its own build directory. This allows running several cargo-install instances in parallel. If we fail to create a temporary directory for whatever reason fallback to creating and using a target-install directory in the current directory. closes #2606 --- Cargo.toml | 2 +- src/cargo/lib.rs | 1 + src/cargo/ops/cargo_install.rs | 15 ++++++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3c2649005d4..b522909ad5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ regex = "0.1" rustc-serialize = "0.3" semver = "0.2.2" tar = "0.4" +tempdir = "0.3" term = "0.4.4" time = "0.1" toml = "0.1" @@ -45,7 +46,6 @@ url = "0.2" winapi = "0.2" [dev-dependencies] -tempdir = "0.3" hamcrest = "0.1" bufstream = "0.1" filetime = "0.1" diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 2d5db07fa11..ae67abc1bc1 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -19,6 +19,7 @@ extern crate regex; extern crate rustc_serialize; extern crate semver; extern crate tar; +extern crate tempdir; extern crate term; extern crate time; extern crate toml; diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 916e35bd707..aab9c1e25af 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -7,6 +7,7 @@ use std::io::prelude::*; use std::io::SeekFrom; use std::path::{Path, PathBuf}; +use tempdir::TempDir; use toml; use core::{SourceId, Source, Package, Registry, Dependency, PackageIdSpec}; @@ -79,13 +80,25 @@ pub fn install(root: Option<&str>, try!(check_overwrites(&dst, &pkg, &opts.filter, &list)); } + let mut td_opt = None; let target_dir = if source_id.is_path() { config.target_dir(&pkg) } else { - Filesystem::new(config.cwd().join("target-install")) + if let Ok(td) = TempDir::new("cargo-install") { + let p = td.path().to_owned(); + td_opt = Some(td); + Filesystem::new(p) + } else { + Filesystem::new(config.cwd().join("target-install")) + } }; config.set_target_dir(target_dir.clone()); let compile = try!(ops::compile_pkg(&pkg, Some(source), opts).chain_error(|| { + if let Some(td) = td_opt.take() { + // preserve the temporary directory, so the user can inspect it + td.into_path(); + } + human(format!("failed to compile `{}`, intermediate artifacts can be \ found at `{}`", pkg, target_dir.display())) })); From b484cd38d750a95f0457590ca9630a0c03552945 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Fri, 22 Apr 2016 22:33:58 -0500 Subject: [PATCH 2/2] add tests for #2606 --- tests/test_cargo_concurrent.rs | 36 +++++++++++++++++++++++++++++++++- tests/test_cargo_install.rs | 15 ++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/tests/test_cargo_concurrent.rs b/tests/test_cargo_concurrent.rs index 99dc624c0fa..ef91c63bbd8 100644 --- a/tests/test_cargo_concurrent.rs +++ b/tests/test_cargo_concurrent.rs @@ -1,4 +1,4 @@ -use std::env; +use std::{env, str}; use std::fs::{self, File}; use std::io::Write; use std::net::TcpListener; @@ -15,6 +15,12 @@ use test_cargo_install::{cargo_home, has_installed_exe}; fn setup() {} +fn pkg(name: &str, vers: &str) { + Package::new(name, vers) + .file("src/main.rs", "fn main() {{}}") + .publish(); +} + test!(multiple_installs { let p = project("foo") .file("a/Cargo.toml", r#" @@ -52,6 +58,34 @@ test!(multiple_installs { assert_that(cargo_home(), has_installed_exe("bar")); }); +test!(concurrent_installs { + const LOCKED_BUILD: &'static str = "waiting for file lock on build directory"; + + pkg("foo", "0.0.1"); + pkg("bar", "0.0.1"); + + let mut a = ::cargo_process().arg("install").arg("foo").build_command(); + let mut b = ::cargo_process().arg("install").arg("bar").build_command(); + + a.stdout(Stdio::piped()).stderr(Stdio::piped()); + b.stdout(Stdio::piped()).stderr(Stdio::piped()); + + let a = a.spawn().unwrap(); + let b = b.spawn().unwrap(); + let a = thread::spawn(move || a.wait_with_output().unwrap()); + let b = b.wait_with_output().unwrap(); + let a = a.join().unwrap(); + + assert!(!str::from_utf8(&a.stderr).unwrap().contains(LOCKED_BUILD)); + assert!(!str::from_utf8(&b.stderr).unwrap().contains(LOCKED_BUILD)); + + assert_that(a, execs().with_status(0)); + assert_that(b, execs().with_status(0)); + + assert_that(cargo_home(), has_installed_exe("foo")); + assert_that(cargo_home(), has_installed_exe("bar")); +}); + test!(one_install_should_be_bad { let p = project("foo") .file("a/Cargo.toml", r#" diff --git a/tests/test_cargo_install.rs b/tests/test_cargo_install.rs index 31d9bf7fcd8..cfad68443c8 100644 --- a/tests/test_cargo_install.rs +++ b/tests/test_cargo_install.rs @@ -659,3 +659,18 @@ test!(q_silences_warnings { assert_that(cargo_process("install").arg("-q").arg("--path").arg(p.root()), execs().with_status(0).with_stderr("")); }); + +test!(readonly_dir { + pkg("foo", "0.0.1"); + + let root = paths::root(); + let dir = &root.join("readonly"); + fs::create_dir(root.join("readonly")).unwrap(); + let mut perms = fs::metadata(dir).unwrap().permissions(); + perms.set_readonly(true); + fs::set_permissions(dir, perms).unwrap(); + + assert_that(cargo_process("install").arg("foo").cwd(dir), + execs().with_status(0)); + assert_that(cargo_home(), has_installed_exe("foo")); +});