Added `build.build_dir` templating support by ranger-ross · Pull Request #15236 · rust-lang/cargo

ranger-ross · 2025-02-26T15:01:43Z

What does this PR try to resolve?

This PR is a follow up on #15104 and and adds support for the path templating in build.build-dir as defined in #14125.

Supported templates:

{workspace-root}
{cargo-cache} (pointing to CARGO_HOME for now)
{workspace-manifest-path-hash}

Unresolved questions

What should we name {workspace-manifest-path-hash} and what should it include? Should we shorten to {workspace-hash} or even just {hash}? Should we include the Cargo version so we get unique whole-target directories for easier cleanup (#13136)

How should this handle unknown variables (error) or unclosed { / } (ignored), see #15236 (comment)

When using {workspace-manifest-path-hash} this hash will change based on the project path. In the event of a cargo being executed in a symlinked project, the hash will change.

For example, given the following directory

/Users/
└─ user1/
    └─ projects/
        ├─ actual-crate/
        │  └─ Cargo.toml
        └─ symlink-to-crate/ -> actual-crate/

the hash will be unique when running cargo from each of the following directories.

/Users/user1/actual-crate
/Users/user1/symlink-to-crate

Figuring out whether to handle this is deferred out, see

How should we test and review this PR?

This PR is fairly small. I included tests for each template variable.

You can also clone my branch and test it locally with

CARGO_BUILD_BUILD_DIR='{workspace-root}/foo' cargo -Z build-dir build

Additional information

While searching Cargo for any prior art for path templating, I found sources/registry/download.rs doing a simple string replace. Thus I followed the same behavior.

r? @epage

tests/testsuite/build_dir.rs

src/cargo/util/context/mod.rs

tests/testsuite/build_dir.rs

src/cargo/util/context/mod.rs

ranger-ross · 2025-02-27T15:34:44Z

Another question: Should Cargo error if the user adds something like

build-dir =  "/foo/{workspace-root}/bar"

This is invalid as {workspace-root} points to a specific directory so prefixes don't make logical sense.
The same is true for cargo-cache but not {workspace-manifest-path-hash} since the hash is not pointing to a specific directory. It's merely part of some path.

In my opinion it would be best to error on an invalid path.
Although another approach would be to simply ignore the prefix.

epage · 2025-02-27T17:55:05Z

Another question: Should Cargo error if the user adds something like

On Linux systems, that is valid and I could see someone doing that, rather than using a hash.

On Windows systems, that will be a mess.

I would lean towards not doing anything special wrt and instead we can note this in the tracking issue for people to be aware of this when we stabilize.

src/doc/src/reference/unstable.md

epage · 2025-03-03T17:23:49Z

src/cargo/util/context/path.rs

+        let mut value = self.0.val.clone();
+
+        for (from, to) in replacements {
+            value = value.replace(from.as_ref(), to.as_ref());
+        }
+
+        self.0.definition.root(gctx).join(&value)


What should this do on undefined variables?

I gave this some thought and I am of the opinion that we should ignore them. (at least for now)

On unix system's its possible to create paths with curly braces. (not sure about Windows)

> mkdir 'foo/{bar}/baz' > ls foo {bar}

Soo the following would be legal

[build] build-dir = "{workspace-root}/foo/{bar}/baz"

Although this is probably a very niche usecase I think it would be better to allow.

The problem is if we add a new variable, what will be the experience on older Cargo? Is it better to lossily succeed or fail?

Hmmm, good point. Perhaps it's better to start out strict to avoid potential for breaking changes.

If there is a demand for curly braces in paths, we could probably add the ability escape them in the future.

Double checking: this hasn't been put in yet, right?

Correct, I should have time to add it this weekend.

Added unexpected variable validation in 707d637

Since you are already making changes, when I added this as a stabilization question in the PR description, I also mentioned unmatched { / } (and forgot to mention it here).

I'll leave the decision up to you of whether you want to check for that and we can always re-evaluate on the way to stabilization.

I was also thinking about that while implementing the last commit.
I am inclined to ignore unterminated brackets for now since they can technically be valid file paths on unix.

But I agree we should re-evaluate before stabilizing.

I think that it will come down to whether we consider { and } in paths a valid usecase.
I'd be perfectly happy to say "No, you cannot use brackets in file paths in templated path configs." (It would simplify validation to a simple path.contains("{") || path.contains("}") after resolving all known template variables)
But the Cargo team members might have different opinions this

This is in preparation for adding templating suppport to the `build.build-dir` configuration option.

tests/testsuite/build_dir.rs

src/cargo/util/context/mod.rs

This commit adds logic to check for unexpected variables in templated paths like `build.build-dir`. Cargo will error if it finds a variable that it does not know how to expand.

epage · 2025-03-17T18:29:21Z

Thanks!

Update cargo 14 commits in 6cf8267012570f63d6b86e85a2ae5627de52df9e..307cbfda3119f06600e43cd38283f4a746fe1f8b 2025-03-14 15:25:36 +0000 to 2025-03-20 20:00:39 +0000 - feat: Add custom completer for cargo <TAB> to complete aliases defined in config.toml (rust-lang/cargo#15319) - fix(build-dir): Renamed workspace-manifest-path-hash to workspace-path-hash (rust-lang/cargo#15334) - feat: vcs, color, and message format native completion (rust-lang/cargo#15322) - Fix `[env]` `relative` description in reference (rust-lang/cargo#15332) - chore: fix some typos (rust-lang/cargo#15329) - Cleanup for rustc-link-arg-cdylib (rust-lang/cargo#15326) - fix(toml): Report '<target>.edition' deprecation to users (rust-lang/cargo#15321) - test(build-std): address overly-matched snapshot (rust-lang/cargo#15325) - Added `build.build_dir` templating support (rust-lang/cargo#15236) - docs: make it clearer that `rust_version` is enforced during compile (rust-lang/cargo#15303) - feat: Add custom completer for cargo +<TAB> to complete toolchain name (rust-lang/cargo#15301) - chore: fix some typos (rust-lang/cargo#15316) - fix: deduplicate crate types in cargo rustc command (rust-lang/cargo#15314) - docs: mention wrong URLs as a cause of git authentication errors (rust-lang/cargo#15304) r? ghost

rustbot assigned epage Feb 26, 2025

rustbot added A-configuration Area: cargo config files and env vars A-workspaces Area: workspaces S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 26, 2025

epage mentioned this pull request Feb 26, 2025

Remap target dir #11156

Closed