Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Templating CARGO_TARGET_DIR to make it the parent of all target directories #3371

Closed
Closed
Changes from 1 commit
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
4faf132
rfc: CARGO_TARGET_DIRECTORIES: initial proposal version
poliorcetics Jan 12, 2023
0a0df4c
chore: use PR number in file name
poliorcetics Feb 5, 2023
0b53b39
feat: Improve "alternatives" and "prior art" sections following review.
poliorcetics Feb 5, 2023
6082243
nit: clarify some lines
poliorcetics Feb 27, 2023
ed55d46
fix: improve bazel section, fix missing link
poliorcetics Feb 27, 2023
5f869bc
feat: use hashing in the naming scheme
poliorcetics Feb 27, 2023
aafee1c
feat: discuss remapping
poliorcetics Feb 27, 2023
4c89e34
nit: missing `<hash>`
poliorcetics Feb 28, 2023
a3b8c7c
fix: decisively declare non-stability of naming scheme
poliorcetics Feb 28, 2023
4ea813c
feat: talk about symlinks resolving
poliorcetics Feb 28, 2023
a8fae16
nit: clarify `cargo clean` behaviour explanation
poliorcetics Feb 28, 2023
f2a8448
fix: link to targo and add backlinks subsection
poliorcetics Feb 28, 2023
bd8b690
Expand on using `CARGO_TARGET_DIRECTORIES` as the default
poliorcetics May 28, 2023
7a1ee20
Clarify we hash on the symlink-resolved form of the path
poliorcetics May 28, 2023
35e99f6
rename CARGO_TARGET_DIRECTORIES to CARGO_TARGET_BASE_DIR
poliorcetics Jun 19, 2023
c2300bb
rename file to follow feature renaming
poliorcetics Jun 19, 2023
4a6cb64
question: how do we handle possibly thousand of directories in the sa…
poliorcetics Jun 19, 2023
7ac2013
clarify situation for third party cargo tools
poliorcetics Jun 19, 2023
b1aa45e
typo: dire -> directory
poliorcetics Jun 19, 2023
3676816
clarify CARGO_HOME as target base dir
poliorcetics Jun 19, 2023
0520ea8
don't use line breaks, they make following changes harder since they …
poliorcetics Jun 19, 2023
f604f0a
expand upon targo caching scheme
poliorcetics Jun 19, 2023
d7830f3
propose alternate naming scheme to simply scaling in unresolved quest…
poliorcetics Jun 19, 2023
e79c752
underspecify naming scheme and use manifest instead of workspace dir,…
poliorcetics Jun 21, 2023
a245144
Clarify one advantage
poliorcetics Jun 21, 2023
a7af623
Add drawback about windows path length limits
poliorcetics Jun 25, 2023
001f27f
keeping local target dir even if the default changes
poliorcetics Jun 25, 2023
e3fda64
Typo fix
poliorcetics Jun 27, 2023
5d33e78
introduce backlinks and expand on them
poliorcetics Jul 8, 2023
46f4b80
mention garbage collection in future possibilities
poliorcetics Jul 8, 2023
10e2d19
nit: fix unclear text
poliorcetics Sep 3, 2023
c386c97
clarify text about setting `CARGO_TARGET_DIR` for cargo calls
poliorcetics Sep 3, 2023
a519e7c
clarify backlinks and forward links, I misunderstood them at first
poliorcetics Sep 4, 2023
50251e4
cleanup of RFC text to make less use of 'I', 'we', add a link to targ…
poliorcetics Sep 4, 2023
0e5d30e
feat: move to templating `CARGO_TARGET_DIR` instead of a new config `…
poliorcetics Feb 17, 2024
9e119ef
fix: add transition period notice
poliorcetics Feb 17, 2024
d4fd860
rename file to follow feature rename
poliorcetics Feb 18, 2024
9945d9a
nit: missed BASE_
poliorcetics Feb 18, 2024
d222d65
point to the discussion on garbage collection
poliorcetics Feb 18, 2024
aea0adb
expand on exposing template metadata
poliorcetics Feb 18, 2024
376c306
caching in cargo home discussion
poliorcetics Feb 18, 2024
befae7d
move forward links sections to Unresolved questions
poliorcetics Feb 18, 2024
07f899b
specify link-target-dir
poliorcetics Feb 18, 2024
fac2d9c
Explain the advantages of hashes over copied folder tree structure
poliorcetics Feb 19, 2024
23ae70a
fix: put section back where it belong, it was one level too deep
poliorcetics Feb 19, 2024
025b8ef
drawback: brace expansion
poliorcetics Feb 19, 2024
8d1ddc2
future possibilities: remove section about metadata since there is no…
poliorcetics Feb 19, 2024
b81b3ed
link to issue 1734 for other possible templates discussions
poliorcetics Feb 19, 2024
2ae536e
fix link to garbage collection issue
poliorcetics Feb 19, 2024
11ab0e3
delete section about providing backlinks
poliorcetics Feb 19, 2024
6540d11
expand a lot on forward links, adding them to the RFC and not just as…
poliorcetics Feb 19, 2024
1195538
default with template fix
poliorcetics Feb 19, 2024
3cead0d
nit: simplify section title for template variables
poliorcetics Mar 22, 2024
3a6fa3e
nit: no double negatives
poliorcetics Mar 22, 2024
645d9c4
nit: clarify the point about path length as an argument for hashes
poliorcetics Mar 23, 2024
05b96f8
clarify summary by being more explicit (and fix a typo)
poliorcetics Mar 23, 2024
226f9ec
nit: fix a forgotten 'target-base-dir' remnant
poliorcetics Mar 23, 2024
2802659
feat: only one new option, not two, handle concurrent builds predictably
poliorcetics Mar 23, 2024
d879751
feat: clarify template-as-default section in future possibilites
poliorcetics Mar 23, 2024
0f71cf5
nit: remove nonsense line
poliorcetics Mar 24, 2024
927226d
nit: move transition period section
poliorcetics Mar 24, 2024
a821d25
feat: expand on transition period
poliorcetics Mar 24, 2024
5dcfb29
feat: mention key for a future default cargo default target directory
poliorcetics Mar 25, 2024
774e328
chore: move Transition Period section to reference level explanation
poliorcetics Mar 25, 2024
81bac72
feat: clarify the 'Naming' section
poliorcetics Apr 19, 2024
0a38c07
nit: forgotten 'an'
poliorcetics Apr 19, 2024
d549cb1
feat: add future possibilities for templating other configs
poliorcetics Apr 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 45 additions & 5 deletions text/3371-cargo-target-directories.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ In practice, it should be rare to have two different projects using the same nam

In case the parent directory is `/` or `C:\`, the subdirectory name is implementation defined.

See "Rationale and alternatives" about conflicts in the naming scheme.

## Impact on `cargo ...` calls

When calling `cargo` where `CARGO_TARGET_DIRECTORIES` is active, `CARGO_TARGET_DIR` is set by all `cargo` calls that happen in a Cargo workspace, including calls to third-party tools.
epage marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -202,8 +204,12 @@ It is already possible today to use `CARGO_TARGET_DIR` to remap workspaces and p

1) If done globally, the `CARGO_TARGET_DIR` becomes a hodge-podge of every project, which is not often the goal.
2) If done per-project, it is very cumbersome to maintain.
3) [`targo`](https://github.com/sunshowers/targo) by @sunshowers
4) [rust-lang/cargo#11156](https://github.com/rust-lang/cargo/issues/11156)

`targo` and the cargo issue express a need for either remapping or a global target directory that is not shared between different Cargo workspaces.

For those reason, this option has not been retained.
For those reason, this option has not been retained and the `targo` tool is discussed more in details below.

## Using `XDG_CACHE_HOME` instead of a cargo-specific env-var

Expand All @@ -221,6 +227,32 @@ bringing the registry cache in their build directory for example.
I feel this require an hard-to-break naming scheme, which I don't have the skills nor motivation to design. Instead, I prefer explicitely telling the naming scheme is not to
be considered stable and allow more invested people to experiment with the feature and find something solid.

## Make the naming conflict less easily

The (possibly) conflicting naming scheme used here is something that can easily be fixed: instead of just `/cargo-caches/work-project`, use something like `/cargo-caches/work-project-<blake3 hash of full path to 'work-project' directory>`, like `targo` does. This approach has at least two defaults and one advantage:

- **Advantage**: conflicts are pretty much impossible while the naming scheme is still predictable
- **Disadvantage**: external tools now have to know about the naming scheme internal more than just "set `$CARGO_TARGET_DIRECTORIES` and append crate directory name" and changing the hash method or length could heavily break them (even with us specifying it as unstable, we all know how that goes in real life). We would also have to specify when is the hash resolved (before symlink resolution or after).
epage marked this conversation as resolved.
Show resolved Hide resolved
- **Disadvantage**: moving the crate directory implies a full rebuild because the hash has changed. This is especially impactful for CIs, where different steps could be executed in different temporary directories, preventing them from enjoying the benefits of such a cache. For CIs, this is strongly mitigated by using `$CARGO_TARGET_DIR` directly and so may not be that much of a disadvantage. For local builds, I expect users are not moving their crates all over the place often, although I only have myself as data on this.

Also, I don't expect people are using similarly-named directories for unrelated projects. I checked my machines, it's zero for all of them. I have two `rust-lang/rust` worktree, named `rust-lang-1` and `rust-lang-2`, and they would not interfere with each other if they used a system like `$CARGO_TARGET_DIRECTORIES` since their directory names are different.
epage marked this conversation as resolved.
Show resolved Hide resolved

## Just use `targo`
epage marked this conversation as resolved.
Show resolved Hide resolved

While a very nice tool, `targo` is not integrated with `cargo` and has a few shortcomings:

- It uses symlinks, which are not always handled well by other tools. Specifically, since it's not integrated inside `cargo`, it uses a `target` symlink to avoid having to remap `cargo`'s execution using `CARGO_TARGET_DIR` and such,making it less useful for external build tools that would use this functionality. Using such a symlink also means `cargo clean` does not work, it just removes the symlink and not the data.
- It completely ignores `CARGO_TARGET_DIR`-related options, which again may break workflows.
- It needs more metadata to work well, which means an external tool using it would have to understand that metadata too.
- It uses `$CARGO_HOME/targo` to place its cache, making it less useful for external build tools and people wanting to separate caches and configuration.
- It needs to intercept `cargo`'s arguments, making it more brittle than an integrated solution.
- It uses a hash-based naming scheme, making it less predictable and compatible with external build tools and moving directories, as seen above.

Some of those could be fixed of course, and I don't expect `cargo`'s `--target-dir` and `--manifest-path` to change or disappear anytime soon, but still, it could happen. An external tool like `targo` will never be able to
solve some of these or ensure forward compatibility as well as the solution proposed in this RFC.

On the other hand, `targo` is already here and working for at least one person, making it the most viable alternative for now.
epage marked this conversation as resolved.
Show resolved Hide resolved

# Prior art
[prior-art]: #prior-art

Expand All @@ -240,10 +272,17 @@ Note that while precedent set by other languages is some motivation, it does not
Please also take into consideration that rust sometimes intentionally diverges from common language features.
-->

- [`targo`](https://github.com/sunshowers/targo) by @sunshowers
- [rust-lang/cargo#11156](https://github.com/rust-lang/cargo/issues/11156)
## `bazel`

The [`bazel`] build system has a similar feature called the [`outputRoot`](https://docs.bazel.build/versions/5.4.0/output_directories.html), which is always active and has default directories on all major platforms (Linux, macOS, Windows).

The naming scheme is as follow: `<outputRoot>/_bazel_$USER/` is the `outputUserRoot`, used for all builds done by `$USER`. Below that, projects are identified by the MD5 hash of the path name of the workspace directory (computed after resolving symlinks).
epage marked this conversation as resolved.
Show resolved Hide resolved

The `outputRoot` can be overridden using `--output_base=...` (this is `$CARGO_TARGET_DIRECTORIES`, the subject of this RFC) and the `outputUserRoot` with `--output_user_root=...` (this is close to using `$CARGO_TARGET_DIR`, already possible in today's `cargo`).
epage marked this conversation as resolved.
Show resolved Hide resolved

**Conclusion**: `bazel` shows that a hash-based workflow seems to work well enough, making an argument for the use of it in `cargo` too. It also uses the current user, to prevent attacks by having compiled a program as root and making the directory accessible to other users later on by also compiling there for them. `cargo` could also do this, though I do not know what happens when `--output_user_root` is set to the same path for two different users.
poliorcetics marked this conversation as resolved.
Show resolved Hide resolved

Both express the needs for either remapping or a global target directory that is not shared between different Cargo workspaces.
*Note: I looked at Bazel 5.4.0, the latest stable version as of this writing, things may change in the future or be different for older versions.*

# Unresolved questions
[unresolved-questions]: #unresolved-questions
Expand All @@ -254,7 +293,8 @@ Both express the needs for either remapping or a global target directory that is
- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC?
-->

None I can think of.
- Should we use a hash-based solution or simply a directory-named based one ? `bazel` using hashes indicates this solution is viable for them, it probably would be for `cargo` too and if we use both the hash and the directory name, it would stay fairly human-readable but this solution also has disadvantages.
- Do we want to differentiate according to users ? `bazel` is a generic build tool, whereas `cargo` is not, so maybe differentiating on users is not necessary for us ?

# Future possibilities
epage marked this conversation as resolved.
Show resolved Hide resolved
[future-possibilities]: #future-possibilities
Expand Down