From 5cc0eb94c08fd4f9bf4331ae4c46c4681d4839cc Mon Sep 17 00:00:00 2001 From: Daniel Mueller Date: Tue, 25 Dec 2018 14:10:36 -0800 Subject: [PATCH] Add support for CARGO_TARGET_DIR_PREFIX This change adds support for a new environment variable, CARGO_TARGET_DIR_PREFIX, to cargo. This variable, when set, is treated as a prefix to the target directory. Note that support for the functionality behind this variable is not trivial to implement with the current design. In particular, we wanted to stick as close to the existing CARGO_TARGET_DIR logic. However, the Config in which it is implemented really does not know anything about the directory of the particular crate we concerned with. As a quick work around to this problem, we just pass in the path to the Cargo.toml from the "upper layer". That works, but ultimately it would be better to make the other layer handle the CARGO_TARGET_DIR_PREFIX logic. This change addresses https://github.com/rust-lang/cargo/issues/5544. TODO: Definitely not finished. This patch needs more tests and may need additional config.toml support (?). TODO: There is also the potential for a permission related problems. E.g., when user root compiles something below /tmp/ and then user nobody tries to do the same the resulting directory ${CARGO_TARGET_DIR_PREFIX}/tmp/ may be owned by root, causing the build for nobody to fail with a permission denied error. --- src/cargo/core/workspace.rs | 19 +++++-- src/cargo/ops/cargo_install.rs | 3 +- src/cargo/util/config/mod.rs | 17 +++++- .../src/reference/environment-variables.md | 4 ++ tests/testsuite/build.rs | 57 +++++++++++++++++++ 5 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 2a0ef5b5d74..e4257e85c5a 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -149,7 +149,6 @@ impl<'cfg> Workspace<'cfg> { /// before returning it, so `Ok` is only returned for valid workspaces. pub fn new(manifest_path: &Path, config: &'cfg Config) -> CargoResult> { let mut ws = Workspace::new_default(manifest_path.to_path_buf(), config); - ws.target_dir = config.target_dir()?; if manifest_path.is_relative() { anyhow::bail!( @@ -160,6 +159,12 @@ impl<'cfg> Workspace<'cfg> { ws.root_manifest = ws.find_root(manifest_path)?; } + if let Some(ref root_manifest) = ws.root_manifest { + ws.target_dir = config.target_dir(root_manifest)?; + } else { + ws.target_dir = config.target_dir(manifest_path)?; + } + ws.custom_metadata = ws .load_workspace_config()? .and_then(|cfg| cfg.custom_metadata); @@ -199,7 +204,11 @@ impl<'cfg> Workspace<'cfg> { ) -> CargoResult> { let mut ws = Workspace::new_default(current_manifest, config); ws.root_manifest = Some(root_path.join("Cargo.toml")); - ws.target_dir = config.target_dir()?; + if let Some(ref root_manifest) = ws.root_manifest { + ws.target_dir = config.target_dir(root_manifest)?; + } else { + ws.target_dir = config.target_dir(&ws.current_manifest)?; + } ws.packages .packages .insert(root_path, MaybePackage::Virtual(manifest)); @@ -230,13 +239,13 @@ impl<'cfg> Workspace<'cfg> { ws.require_optional_deps = require_optional_deps; let key = ws.current_manifest.parent().unwrap(); let id = package.package_id(); - let package = MaybePackage::Package(package); - ws.packages.packages.insert(key.to_path_buf(), package); ws.target_dir = if let Some(dir) = target_dir { Some(dir) } else { - ws.config.target_dir()? + ws.config.target_dir(package.manifest_path())? }; + let package = MaybePackage::Package(package); + ws.packages.packages.insert(key.to_path_buf(), package); ws.members.push(ws.current_manifest.clone()); ws.member_ids.insert(id); ws.default_members.push(ws.current_manifest.clone()); diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 5d3fbb8c658..fb7a92eb998 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -250,6 +250,7 @@ fn install_one( } }; + let manifest_path = pkg.manifest_path().to_path_buf(); let (mut ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; // If we're installing in --locked mode and there's no `Cargo.lock` published // ie. the bin was published before https://github.com/rust-lang/cargo/pull/7026 @@ -270,7 +271,7 @@ fn install_one( let mut td_opt = None; let mut needs_cleanup = false; if !source_id.is_path() { - let target_dir = if let Some(dir) = config.target_dir()? { + let target_dir = if let Some(dir) = config.target_dir(manifest_path)? { dir } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { let p = td.path().to_owned(); diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index b7b4ed36712..31467d73911 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -485,7 +485,7 @@ impl Config { /// Returns `None` if the user has not chosen an explicit directory. /// /// Callers should prefer `Workspace::target_dir` instead. - pub fn target_dir(&self) -> CargoResult> { + pub fn target_dir(&self, manifest: impl Into) -> CargoResult> { if let Some(dir) = &self.target_dir { Ok(Some(dir.clone())) } else if let Some(dir) = self.env.get("CARGO_TARGET_DIR") { @@ -498,6 +498,21 @@ impl Config { } Ok(Some(Filesystem::new(self.cwd.join(dir)))) + } else if let Some(dir) = env::var_os("CARGO_TARGET_DIR_PREFIX") { + let prefix = Path::new(&dir); + if !prefix.is_absolute() { + bail!("CARGO_TARGET_DIR_PREFIX must describe an absolute path"); + } + let mut manifest = manifest.into(); + let result = manifest.pop(); + assert!(result); + + match manifest.strip_prefix("/") { + Ok(dir) => Ok(Some(Filesystem::new(prefix.join(&dir).join("target")))), + // FIXME: This logic is probably not safe on Windows. Not sure how + // to make a path relative there. + Err(_) => bail!("Current directory must be an absolute path"), + } } else if let Some(val) = &self.build_config()?.target_dir { let path = val.resolve_path(self); diff --git a/src/doc/src/reference/environment-variables.md b/src/doc/src/reference/environment-variables.md index 39ccb7e263b..5e6fc460e1f 100644 --- a/src/doc/src/reference/environment-variables.md +++ b/src/doc/src/reference/environment-variables.md @@ -15,6 +15,10 @@ system: location of this directory. Once a crate is cached it is not removed by the clean command. For more details refer to the [guide](../guide/cargo-home.md). +* `CARGO_TARGET_DIR_PREFIX` — Prefix to the location where to place all + generated artifacts. The current working directory will be appended to this + prefix to form the final path for generated artifacts. Note that + `CARGO_TARGET_DIR`, if set, takes precedence over this variable. * `CARGO_TARGET_DIR` — Location of where to place all generated artifacts, relative to the current working directory. See [`build.target-dir`] to set via config. diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 7276aea8710..7cbcb6a295d 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -3110,6 +3110,63 @@ fn panic_abort_compiles_with_panic_abort() { .run(); } +#[cargo_test] +fn custom_target_dir_prefix() { + fn test(cwd: &str) { + let tmpdir = tempfile::Builder::new() + .tempdir() + .unwrap() + .path() + .to_path_buf(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + let root = p.root(); + let root_suffix = root.strip_prefix("/").unwrap(); + let exe_name = format!("foo{}", env::consts::EXE_SUFFIX); + + p.cargo("build") + .env("CARGO_TARGET_DIR_PREFIX", tmpdir.clone()) + .cwd(p.root().join(cwd)) + .run(); + + assert!( + tmpdir + .clone() + .join(root_suffix) + .join("target/debug") + .join(&exe_name) + .is_file() + ); + assert!(!&p.root().join("target/debug").join(&exe_name).is_file()); + + p.cargo("build").run(); + assert!( + tmpdir + .clone() + .join(root_suffix) + .join("target/debug") + .join(&exe_name) + .is_file() + ); + assert!(&p.root().join("target/debug").join(&exe_name).is_file()) + }; + + test("."); + test("src"); +} + #[cargo_test] fn compiler_json_error_format() { let p = project()