-
Notifications
You must be signed in to change notification settings - Fork 19
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
file_absolute!
macro
#478
Comments
We discussed this in last week's libs-api meeting. We're very hesitant to add absolute paths to the source files as a feature in the standard libarry. As you mention above, it's an open question how this will work with the "trim paths" feature. It's quite possible that trim-paths will become a often used or even default behaviour in the future, so adding a macro that is incompatible with that (e.g. errors/warns), seems like a step in the wrong direction. There seem to be other possible solutions that can solve your problem without relying on absolute paths being hardcoded in binaries, so we'd like to see more exporation in those directions. (E.g. getting the relevant information from cargo, as you mention above.) |
We previously explored these options already before I came here, including having rust-lang/cargo#13644 for stabilizing it. A core problem is that If we could specify CC @ehuss |
We've had to add awkward work-arounds for this in rust-lang/rust#133533, which also turned out to be problematic so rust-lang/rust#132390 has to use a different work-around. The current situation is that a libs-api member requested to not add a new cargo env var for this, and suggested |
@rustbot label +I-libs-api-nominated |
(Speaking for myself, not the Cargo or libs-api team.) @RalfJung Sympathy for the net outcome of mixed messaging here. The previously proposed approach on the Cargo side seems like it would have involved Cargo attempting to expose the current directory that rustc used as a base for I don't think using absolute paths is the right approach either. I would propose that we should have a "base" directory passed into rustc, so that cargo and other build tools can always consistently pass in a standard base (e.g. the crate root, even when in a workspace). And cases like Rust-for-Linux could (for instance) pass in the root of the kernel tree. We could then have a |
well, if the path should be based on the passed-in base directory, how about |
"Cargo attempting to expose the current directory that rustc used as a base for
Judging based on rust-lang/cargo#13644 (comment), that stabilization stalled out on the fact that It makes sense to me that it would be up to Cargo to supply context about Cargo's choice of rustc execution directory because the expectation that we can bake a source's absolute path into compiled code (the
|
As |
Proposal
Problem statement
Being able to bless a test failure as expected and have the expected value updated (ie snapshot testing) is a big productivity boon. For example, the Cargo team just put in a lot of work to switch thousands of tests to snapshot testing (rust-lang/cargo#14039). Having that expected value inside the source code makes it easier to manage (rather than dealing with thousands of snapshots that may or may not be used anymore) and allows you to more easily inspect what condition is being tested. You can look at the reverse dependencies of snapbox, expect-test, and insta. The first two primarily work through inline snapshots. I can't say how often people use inline snapshots with
insta
.Similarly, it is can be helpful to be able to look up resources relative to the source file, rather than relative to the manifest root (via
CARGO_MANIFEST_DIR
). These can includeToday this done by using
file!
(which is unspecified) and assuming it is relative to the workspace root (which isn't always true), and somehow discovering the workspace root. The "somehow" is usually a mixture of.cargo/config.toml
with an[env]
that is the workspace root (remember, as I said,file!
isn't always relative to workspace root), https://github.com/rust-lang/cargo/blob/286b6d0efa1db67c17c1fbed1385e9829f09c1e6/.cargo/config.toml#L7-L10The Cargo team considered adding a
CARGO_RUSTC_CURRENT_DIR
(rust-lang/cargo#12996) that would solve this problem but there were concerns over this existing purely forfile!
andfile!
being unspecified, causing us to push people towards relying on unspecified behavior.Motivating examples or use cases
Normally, this will be abstracted away in a test like:
This relies on macros like
or
Solution sketch
Like
file!
provide afile_absolute!
that fills the role of the above$crate::utils::current_rs!()
(for which a$crate::utils::current_dir!()
can also be made).Likewise, we should probably make the expectations for
file!
more explicitOpen questions
file!
be "unspecified, for display purposes only" or be expected to be relative to a certain location (barring trim paths)Alternatives
CARGO_RUSTC_CURRENT_DIR
(feat: AddCARGO_RUSTC_CURRENT_DIR
(unstable) cargo#12996)Links and related work
What happens now?
This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
Second, if there's a concrete solution:
The text was updated successfully, but these errors were encountered: