Tracking Issue for trim-paths RFC 3127

[ehuss]

This is a tracking issue for the RFC 3127 (rust-lang/rfcs#3127).

This enhancement adds the --remap-path-scope command-line flag to control the scoping of how paths get remapped in the resulting binary.

Issues: F-trim-paths Feature: trim-paths
Documentation (rustc): https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/remap-path-scope.html
Documentation (cargo): https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#profile-trim-paths-option
Cargo tracking issue: rust-lang/cargo#12137

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

• Implement the RFC (Implement rustc part of RFC 3127 trim-paths #115214)
• Adjust documentation (see instructions on rustc-dev-guide)
• Stabilization PR (see instructions on rustc-dev-guide)

Unresolved Questions

• Should we use a slightly more complex remapping rule, like distinguishing packages from registry, git and path, as proposed in Enable --remap-path-prefix for absolute paths by default #40552?
• What concerns are there regarding the use of relative paths causing issues with backtraces and debuggers?
• Should we treat the current package the same as other packages?
• Will these cover all potentially embedded paths? Have we missed anything?
• What exactly is the intended use case for the "diagnostics" scope, and when would someone want to turn it on? Is that something someone would really want to enable in cargo? See --remap-path-prefix no longer works for compiler messages #87745.
• Should rustdoc support remapping (like for diagnostics)? Does remapping matter for doctests?
  • doctest has supported remapping: Simplify trim-paths feature by merging all debuginfo options together #122450
  • rustdoc diagnostics as well: rustdoc: use remap-path-prefix #69264 (comment)
• On Windows PDB hasn't yet got sanitized. See refactor: use Path::push to construct remap-path-prefix cargo#14908.
• Consider the impact of back versus forward slashes, and whether or not that will work with either. See refactor: use Path::push to construct remap-path-prefix cargo#14908 (review)

Implementation history

• Implement rustc part of RFC 3127 trim-paths #115214
• Simplify trim-paths feature by merging all debuginfo options together #122450
• feat: implement RFC 3127 -Ztrim-paths cargo#12625

[weihanglo]

I was trying to remove OSO symbols on macOS. Did a research and summarized in this issue: #116948 (comment). Now, I can see that there is no way for trim-paths to really trim all paths unless rustc does some dirty trick for older macOS platforms. Would it be a blocker if we want to stabilize this feature, say, in 2024 edition?

cc @Urgau you might be interested.

[michaelwoerister]

What concerns are there regarding the use of relative paths causing issues with backtraces and debuggers?

I talked about this with @danielframpton the other day and he noticed that the current [package name]-[version] rule for remapping might make things unnecessarily complicated for debuggers. For making the debugging experience easy to set up, it would be beneficial to require as little configuration as possible. The RFC does not explicitly make file system paths (as used by Cargo) match with how things are renamed, so in the general case the debugger needs a separate source file search path for each crate involved.

However, if we prescribed that each crate from was remapped to the relative path Cargo uses inside its $CARGO_HOME/registry/src and $CARGO_HOME/git/checkouts directories, then we have a fixed number of source directories a debugger needs to know about. The renaming would then be [registry]/[package name]-[version] for registry crates and [package name]-[sha] for git dependencies.

The current [package name]-[version] rule is already pretty close, but that seems to be by accident. It would be good to make this something that can be relied on.

This might also help with backtraces?

I don't know if there is something similarly useful we can do for path dependencies.

Should we treat the current package the same as other packages?

This also came up. Unconditionally stripping the path to the current package might be too inflexible in some cases. E.g. when building a staticlib, one might want to make the paths coming from the current package identifiable in some way. If they all start with src/.. then they might clash with the paths of the codebase that uses the staticlib as a dependency.

[weihanglo]

However, if we prescribed that each crate from was remapped to the relative path Cargo uses inside its $CARGO_HOME/registry/src and $CARGO_HOME/git/checkouts directories, then we have a fixed number of source directories a debugger needs to know about. The renaming would then be [registry]/[package name]-[version] for registry crates and [package name]-[sha] for git dependencies.

This plan sounds really tenable to me! Will do a change on cargo side for this, and see how people think about it.

For path dependencies, maybe we could have a common prefix for them, like cargo_deps@local/<pkg>-<version>. Might not be useful as others dep kinds, but at least people can have a single location for path dependencies?

then they might clash with the paths of the codebase that uses the staticlib as a dependency.

Sorry I don't fully understand. Could you give a more concrete example of this?

[michaelwoerister]

For path dependencies, maybe we could have a common prefix for them, like cargo_deps@local/-. Might not be useful as others dep kinds, but at least people can have a single location for path dependencies?

Maybe, yes. It's definitely worth thinking about this some more (and collecting ideas from other people).

Sorry I don't fully understand. Could you give a more concrete example of this?

As I understand it, the RFC states that paths from the workspace being compiled are trimmed by simply making them relative to the workspace root, that is, all file paths from workspace crates would look like src/foo/bar.rs or some_crate_in_the_workspace/src/foo/bar.rs. Those paths don't contain anything that would allow disambiguating them from other paths that are purely relative. One example where this is sub-optimal is when one has multiple Rust projects, each of which gets compiled into a staticlib that is then consumed within another, non-Rust project. Each of these Rust projects would have debuginfo file paths that just start with src/ and the paths for some of the files might clash , e.g. src/lib.rs. The debugger would have no way to know which src/lib.rs file to pick, even if both are in its source search path.

Therefore it would be good, if there was some way to avoid this problem. Allowing to prepend a prefix to the relative paths might be an option.

[weihanglo]

@michaelwoerister. Thank you for your feedback! I just opened a new issue rust-lang/cargo#13171 for further discussion about remap rules in Cargo.

[weihanglo]

Not entirely sure on rustc side of this, but I feel like it is waiting for Cargo to report back the integration status. See also: rust-lang/cargo#12137 (comment)

[estebank]

Random question: are these flags meant to affect the output of diagnostics as well? One could argue that they should.

[Urgau]

Yes, that's the diagnostics scope, see the --remap-path-scope flag for more details.

[teknalb]

Looks like doesn't work when a dependency uses generated bindings.

I see two full project paths in a binary compiled with --release and trim-paths = true

D:\\dev\my-app\\target\\release\\build\\glutin_wgl_sys-8d26fb50290274cc\\out/wgl_extra_bindings.rs
D:\\dev\\my-app\\target\\release\\build\\glutin_egl_sys-a91455d827c5e0e9\\out/egl_bindings.rs

This is what's in cargo.toml

cargo-features = ["trim-paths"]

[package]
name = "my-app"
version = "0.1.0"
edition = "2021"
build = "build.rs"

[dependencies]
slint = "1.8"

[build-dependencies]
slint-build = "1.8"

[profile.release]
strip = true
trim-paths = true
panic = "abort"

[weihanglo]

It is interesting that include!(concat!(env!("OUTDIR"), "…")) combo got leaked, though I can't reproduce it on Linux with the small reproducer below, even for a debug build. @teknalb Does it only happen on Windows?

Details

// build.rs
fn main() {
    let dest = std::path::PathBuf::from(std::env::var("OUT_DIR").unwrap());
    std::fs::write(
        dest.join("bindings.rs"),
        r#"pub fn yes(n: u64) { println!("yes {n}!!", ); }"#,
    )
    .unwrap();
}

// src/lib.rs
include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

// src/main.rs
fn main() {
    bar::yes(2);
}

[teknalb]

@weihanglo
Did not happen on Linux.

[weihanglo]

@teknalb It might be worth opening a separate issue report with a full reproducible example, including how you figured out those embedded strings. profile.trim-paths = true is inherently the stable --remap-path-prefix flag without any scope, so it might be a regression or bug. I had a hard time reproducing with the minimal reproducer here #111540 (comment) on Windows with strings from Sysinternals. Mind trying out the minimal reproducer and reporting back the result? Maybe my reproducer is not a good one.

There are indeed absolute paths in pdb file itself, but not in the final executables. Despite that it sounds like a separate issue.

I feel like it's not possible to remove absolute paths in pdb, which --remap-path-scope=debuginfo ought to support. However, I saw the PathMap flag in C# compiler, so we still have hope. @ChrisDenton Do you have any suggestion?

[teknalb]

@weihanglo

Path leaks I mentioned happened on release mode, here is a minimal reproducible project: path-leak.zip
One way to find leaked strings is to use this little utility strings, to find specific path strings I mentioned you can use this command: strings path-leak.exe | findstr /i bindings