Skip to content

Commit

Permalink
Add support for CARGO_TARGET_DIR_PREFIX
Browse files Browse the repository at this point in the history
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 rust-lang#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.
  • Loading branch information
d-e-s-o committed May 18, 2021
1 parent 6976741 commit 5cc0eb9
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 7 deletions.
19 changes: 14 additions & 5 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Workspace<'cfg>> {
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!(
Expand All @@ -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);
Expand Down Expand Up @@ -199,7 +204,11 @@ impl<'cfg> Workspace<'cfg> {
) -> CargoResult<Workspace<'cfg>> {
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));
Expand Down Expand Up @@ -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());
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down
17 changes: 16 additions & 1 deletion src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<Filesystem>> {
pub fn target_dir(&self, manifest: impl Into<PathBuf>) -> CargoResult<Option<Filesystem>> {
if let Some(dir) = &self.target_dir {
Ok(Some(dir.clone()))
} else if let Some(dir) = self.env.get("CARGO_TARGET_DIR") {
Expand All @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions src/doc/src/reference/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
57 changes: 57 additions & 0 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 5cc0eb9

Please sign in to comment.