-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
base: master
Are you sure you want to change the base?
RFC: Templating CARGO_TARGET_DIR
to make it the parent of all target directories
#3371
Conversation
What I would like to see is a "global" target directory for all projects, that way, common dependency artifacts would be shared. (Not all projects would need to compile syn and other common dependencies over and over again) I think that would solve the problems that this RFC tries to solve. But I realise this has other challenges (different features set of dependencies, when/how to clean that cache, ...) |
Already possible by setting |
If this is ever slated to merge I'll squash then, while the RFC is ongoing I'll make new commits to allow people to follow the evolution more easily :) |
Sorry it took me a long time to answer to comments |
Thanks epage for the suggestions.
While unlikely, the transition period may break workflows by introducing errors for previously valid target directories. | ||
Several alternatives exist for this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be worth being explicit about what happens if the config option is turned on and off, or if older and newer Cargo versions are invoked with e.g. cargo +version build
. And to consider the behavior both with and without forward links. For example, let's say Cargo version 1.500 supports templating:
- Fresh build with Cargo 1.500 with option turned on
- Fresh build with 1.500 with option turned on and then off
- Fresh build with 1.500 with option turned off and then on
- Fresh build with 1.500 with option turned on and then with 1.499
- Fresh build with 1.499 and then with Rust 1.500 with option turned on
cargo +1.499 clean
cargo +1.500 clean
with option turned off
Old Rust versions will be around for a while, and typical user behavior would be to set it in the global env or .cargo/config, and then forget about it.
For comparison, targo needs to be invoked once to create the symlink, and then most cargo commands work fine as usual regardless of version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be worth being explicit
On the contrary, I don't think it's worth it: that would be better determined by experimentation I think, and once we have something of a consensus by testers we can finalize and write a definite table for it.
There are effectively two options:
- Templating
- Forwarding
The second does not matter if the first is inactive.
If templating is active it means CARGO_TARGET_DIR
is set in some way, in which case the behaviour is known: cargo
creates a directory as directed, so CARGO_TARGET_DIR=/tmp/{target}
would create exactly /tmp/{target}
, as it does today already.
I have a few rules I would like to see because they're also how cargo
operates today (for the non-templating rules):
- No
CARGO_TARGET_DIR
setting: work with./target
, regardless of if it's a symlink or directory, as is done today - Using
CARGO_TARGET_DIR=/no-template
, ignore./target
completely, as is done today - Using
CARGO_TARGET_DIR=/tmp/{manifest-path-hash}
1.499
: ignore./target
completely, as is done today, works in exactly/tmp/{manifest-path-hash}
1.500
: works in/tmp/<hash path>
- Forwarding:
./target
is symlink: replace it, work with it./target
is not symlink: ignore it completely
- No forwarding: ignore
./target
completely
- Forwarding:
Subsequent commands where the target dir (regardless of position) already exist would just work with it, as is done today: I can build with 1.76 and 1.77 in the same repo, it will just work in the same target dir and do whatever it wants with the artifacts and such.
But all of those rules (barring those that are already in action in released versions of cargo
) shouldn't be set in stone IMO, I would prefer having users coming to us to tell us what they (didn't) like about them during the unstable period.
If enough people want to see them in the RFC then I can them though, since it will likely mean there is some consensus on them already
@rfcbot resolve cargo-target-dir |
|
||
## Definition of `{manifest-path-hash}` | ||
|
||
In the example in the previous section, `{manifest-path-hash}` was replaced with a relative path. This relative path is computed from the full and canonicalized path to the manifest for the workspace `Cargo.toml` (or the `script.rs` file directly for cargo-scripts). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be the very first templating supported in .cargo/config.toml
. Is the syntax flexible enough to extend to other configuration like [build.rustflags]
, [env]
. I can see it useful in other issues like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no reason for it to be limited.
{name}
is not used anywhere currently IIRC and the most likely places it can crop up unexpectedly are paths, where this RFC already discuss how unlikely that is.
I suppose anything where cargo shells out to another program can also have it come up, but I'm not sure we can even design for every possible target.TTT.runner
in eternity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Thanks for the reply. Maybe it is worth putting this in future posibilities in some way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in d549cb1 :) I tried noting possible future problems I could find in 10 minutes of thinking and why they're not limiting
Cargo has these dilemmas about the location of the target dir, because Cargo has conflated two concepts:
Users want 1 in a centralized location to deduplicate/reuse intermediate products (files they never touch). And users want 2 in ./target to run or copy the files easily, from a convenient location that has no global naming clashes. As long as Cargo will continue to dump both kinds of products into the same directory, it will get complaints about making build results difficult to reach, or even overwriting each other, while at the same time getting requests to unify the directory. Adding a hash to the path is not solving either of these problems. The hash keeps common dependencies built redundantly and puts the final artifacts in an inconvenient location. |
This isn't a full solution to this problem, but (having used targo full time for over a year) it solves several other problems that are roughly as important. |
I think you misunderstood the RFC. Point
That is already possible, always has been I think, just by setting a
Because the problems you're describing are not what this RFC is proposing to solve. What this RFC is proposing is to gather all target directories under a common "base" path while keeping them separate for each project, for the purposes of caching, cleaning local repositories, sharing build artifacts via NFS or any other usage that people will think of or have mentioned in the original issue. This RFC proposes an opt-in option: it won't make life harder for people that don't use it, just like lots of people don't override their Separating intermediate and final build artifacts or making them (either or) easy to access are not goals: those are either cargo issues or RFCs already. |
To add to this, I want to eventually change this but I feel its dependent on finalizing |
What I'm trying to say that if Cargo redefined To avoid hijacking this thread, I've posted: rust-lang/cargo#14125 |
Just to clarify, would this RFC allow a use case when you compile the same project with, for example, different RUSTFLAGS alternatingly and it will not invalidate cache every time, but keep 2 of them and use appropriate? |
Nop, only the location of the cache would be affected, not its workings |
@rfcbot cancel |
@epage proposal cancelled. |
@rfcbot postpone We discussed this, rust-lang/cargo#6790, and rust-lang/cargo#14125 in the Cargo team meeting today. In that discussion, we came up with an idea that merges these different ideas, see rust-lang/cargo#14125 (comment). However, we recognize that we don't want an "ideal" to forever block people getting functionality now and we were thinking of time boxing that effort for 1 year after which we could re-evaluate whether we should go with a "short term solution" of this RFC. @poliorcetics thank you for all of your work on this proposal. You've put in a lot of work on this and this RFC serves as the basis for this alternative idea we are exploring. If you'd like to continue with that as an extension of this work, we'd welcome your help! I hope you don't see this as "a rejection" or "this was thrownaway". |
Team member @epage has proposed to postpone this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Introduce a new templating option for
CARGO_TARGET_DIR
to tell it to move the crate/workspace's targetdirectory into a crate/workspace-specific subdirectory of the configured path, allowing for easy exclusion from save tools for example.
Rendered
FCP (previous FCP)