-
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
Open
poliorcetics
wants to merge
67
commits into
rust-lang:master
Choose a base branch
from
poliorcetics:cargo-target-directories
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+435
−0
Open
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 0a0df4c
chore: use PR number in file name
poliorcetics 0b53b39
feat: Improve "alternatives" and "prior art" sections following review.
poliorcetics 6082243
nit: clarify some lines
poliorcetics ed55d46
fix: improve bazel section, fix missing link
poliorcetics 5f869bc
feat: use hashing in the naming scheme
poliorcetics aafee1c
feat: discuss remapping
poliorcetics 4c89e34
nit: missing `<hash>`
poliorcetics a3b8c7c
fix: decisively declare non-stability of naming scheme
poliorcetics 4ea813c
feat: talk about symlinks resolving
poliorcetics a8fae16
nit: clarify `cargo clean` behaviour explanation
poliorcetics f2a8448
fix: link to targo and add backlinks subsection
poliorcetics bd8b690
Expand on using `CARGO_TARGET_DIRECTORIES` as the default
poliorcetics 7a1ee20
Clarify we hash on the symlink-resolved form of the path
poliorcetics 35e99f6
rename CARGO_TARGET_DIRECTORIES to CARGO_TARGET_BASE_DIR
poliorcetics c2300bb
rename file to follow feature renaming
poliorcetics 4a6cb64
question: how do we handle possibly thousand of directories in the sa…
poliorcetics 7ac2013
clarify situation for third party cargo tools
poliorcetics b1aa45e
typo: dire -> directory
poliorcetics 3676816
clarify CARGO_HOME as target base dir
poliorcetics 0520ea8
don't use line breaks, they make following changes harder since they …
poliorcetics f604f0a
expand upon targo caching scheme
poliorcetics d7830f3
propose alternate naming scheme to simply scaling in unresolved quest…
poliorcetics e79c752
underspecify naming scheme and use manifest instead of workspace dir,…
poliorcetics a245144
Clarify one advantage
poliorcetics a7af623
Add drawback about windows path length limits
poliorcetics 001f27f
keeping local target dir even if the default changes
poliorcetics e3fda64
Typo fix
poliorcetics 5d33e78
introduce backlinks and expand on them
poliorcetics 46f4b80
mention garbage collection in future possibilities
poliorcetics 10e2d19
nit: fix unclear text
poliorcetics c386c97
clarify text about setting `CARGO_TARGET_DIR` for cargo calls
poliorcetics a519e7c
clarify backlinks and forward links, I misunderstood them at first
poliorcetics 50251e4
cleanup of RFC text to make less use of 'I', 'we', add a link to targ…
poliorcetics 0e5d30e
feat: move to templating `CARGO_TARGET_DIR` instead of a new config `…
poliorcetics 9e119ef
fix: add transition period notice
poliorcetics d4fd860
rename file to follow feature rename
poliorcetics 9945d9a
nit: missed BASE_
poliorcetics d222d65
point to the discussion on garbage collection
poliorcetics aea0adb
expand on exposing template metadata
poliorcetics 376c306
caching in cargo home discussion
poliorcetics befae7d
move forward links sections to Unresolved questions
poliorcetics 07f899b
specify link-target-dir
poliorcetics fac2d9c
Explain the advantages of hashes over copied folder tree structure
poliorcetics 23ae70a
fix: put section back where it belong, it was one level too deep
poliorcetics 025b8ef
drawback: brace expansion
poliorcetics 8d1ddc2
future possibilities: remove section about metadata since there is no…
poliorcetics b81b3ed
link to issue 1734 for other possible templates discussions
poliorcetics 2ae536e
fix link to garbage collection issue
poliorcetics 11ab0e3
delete section about providing backlinks
poliorcetics 6540d11
expand a lot on forward links, adding them to the RFC and not just as…
poliorcetics 1195538
default with template fix
poliorcetics 3cead0d
nit: simplify section title for template variables
poliorcetics 3a6fa3e
nit: no double negatives
poliorcetics 645d9c4
nit: clarify the point about path length as an argument for hashes
poliorcetics 05b96f8
clarify summary by being more explicit (and fix a typo)
poliorcetics 226f9ec
nit: fix a forgotten 'target-base-dir' remnant
poliorcetics 2802659
feat: only one new option, not two, handle concurrent builds predictably
poliorcetics d879751
feat: clarify template-as-default section in future possibilites
poliorcetics 0f71cf5
nit: remove nonsense line
poliorcetics 927226d
nit: move transition period section
poliorcetics a821d25
feat: expand on transition period
poliorcetics 5dcfb29
feat: mention key for a future default cargo default target directory
poliorcetics 774e328
chore: move Transition Period section to reference level explanation
poliorcetics 81bac72
feat: clarify the 'Naming' section
poliorcetics 0a38c07
nit: forgotten 'an'
poliorcetics d549cb1
feat: add future possibilities for templating other configs
poliorcetics File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
nit: move transition period section
- Loading branch information
commit 927226dbd2abd0b6be700b524f3e887ea3841f62
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'd suggest that we error for any unrecognized value in braces
{...}
, and allow explicit braces though double braces{{
similar to how Rust's format strings work.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.
Wouldn't this deviate from our other "templating" systems in cargo? For myself, I lean towards consistency but might be convinced otherwise.
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 think that's better answered at the implementation level, once we have some experience with how people use it.
Escaping is always nice, but so is consistency, and I'm really not convinced there is any "non test" path in the world with
{
in it that would appear in a human controlled config for target directories