feat: Stablize CARGO_RUSTC_CURRENT_DIR
#13644
Conversation
This provides what cargo sets as the `current_dir` for the `rustc` process. While `std::file!` is unspecified in what it is relative to, it is relatively safe, it is generally relative to `rustc`s `current_dir`. This can be useful for snapshot testing. For example, `snapbox` has been using this macro on nightly since assert-rs/snapbox#247, falling back to finding a parent of `CARGO_MANIFEST_DIR`, if present. This has been in use in Cargo since rust-lang#13441. This was added in rust-lang#12996. Relevant points discussed in that issue: - This diverged from the original proposal from the Cargo team of having a `CARGO_WORKSPACE_DIR` that is the "workspace" of the package being built (ie registry packages would map to `CARGO_MANIFEST_DIR`). In looking at the `std::file!` use case, `CARGO_MANIFEST_DIR`, no matter how we defined it, would only sort of work because no sane definition of that maps to `rustc`'s `current_dir`.a This instead focuses on the mechanism currently being used. - Using "current dir" in the name is meant to be consistent with `std::env::current_dir`. - I can go either way on `CARGO_RUSTC` vs `RUSTC`. Existing related variables: - `RUSTC` - `RUSTC_WRAPPER` - `RUSTC_WORKSPACE_WRAPPER` - `RUSTFLAGS` (no `C`) - `CARGO_CACHE_RUSTC_INFO` Note that rust-lang#3946 was overly broad and covered many use cases. One of those was for packages to look up information on their dependents. Issue rust-lang#13484 is being left open to track that. Fixes rust-lang#3946
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
rustbot
added
A-build-execution
|
@rfcbot fcp merge |
|
Team member @epage has proposed to merge 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. |
rfcbot
added
proposed-final-comment-period
| base.get_cwd() | ||
| .map(|c| c.display().to_string()) | ||
| .unwrap_or(String::new()), |
There was a problem hiding this comment.
Wonder if it could just be.
| base.get_cwd() | |
| .map(|c| c.display().to_string()) | |
| .unwrap_or(String::new()), | |
| base.get_cwd().unwrap_or_default(), |
(and ditto for CARGO_TARGET_TMPDIR? though that is not relevant to this PR)
| @@ -265,7 +265,7 @@ corresponding environment variable is set to the empty string, `""`. | |||
| where integration tests or benchmarks are free to put any data needed by | |||
| the tests/benches. Cargo initially creates this directory but doesn't | |||
| manage its content in any way, this is the responsibility of the test code. | |||
| * `CARGO_RUSTC_CURRENT_DIR` --- This is a path that `rustc` is invoked from **(nightly only)**. | |||
| * `CARGO_RUSTC_CURRENT_DIR` --- This is a path that `rustc` is invoked from | |||
There was a problem hiding this comment.
Still feel like people might overlook this, as it isn't an obvious thing easy to relate to their problems. That said, I am fine with the previous explanation. Just forward it here if people have the same question: #12996 (comment).
When we talked about this, Eric was hesitant about locking down the behavior of
file!so I left out that use case. It should work and I can't imagine rustc doing anything but absolute or relative tostd::env::current_dirbut I'm leaving that to people to make the choice that they want to make those assumptions, rather than us advertising it.
|
Is there some information about the use cases for this? How is it expected to be used? |
One of the requests in #3946 is a way to be able to resolve For example, snapbox::path::current_dir!) is useful for
Possible workarounds
As you mentioned the last time the Cargo team discussed this, For a summary of the team's last discussion on this, see #3946 (comment) |
|
I am concerned that if the primary (or only?) way to use this environment variable is with Were there other considerations made, such as adding something like Have there been any discussions with the libs team, and what their thoughts are on making
Is this limited to only test/bench? |
This provides what cargo sets as the
current_dirfor therustcprocess.While
std::file!is unspecified in what it is relative to, it is relatively safe, it is generally relative torustcscurrent_dir.This can be useful for snapshot testing.
For example,
snapboxhas been using this macro on nightly since assert-rs/snapbox#247, falling back to finding a parent ofCARGO_MANIFEST_DIR, if present.This has been in use in Cargo since #13441.
This was added in #12996.
Relevant points discussed in that issue:
CARGO_WORKSPACE_DIRthat is the "workspace" of the package being built (ie registry packages would map toCARGO_MANIFEST_DIR). In looking at thestd::file!use case,CARGO_MANIFEST_DIR, no matter how we defined it, would only sort of work because no sane definition of that maps torustc'scurrent_dir.a This instead focuses on the mechanism currently being used.std::env::current_dir.CARGO_RUSTCvsRUSTC. Existing related variables:RUSTCRUSTC_WRAPPERRUSTC_WORKSPACE_WRAPPERRUSTFLAGS(noC)CARGO_CACHE_RUSTC_INFONote that #3946 was overly broad and covered many use cases. One of those was for packages to look up information on their dependents.
Issue #13484 is being left open to track that.
Fixes #3946