Track devirtualized filenames by pnkfelix · Pull Request #72767 · rust-lang/rust (original) (raw)

Split payload of FileName::Real to track both real and virtualized paths.

(Such splits arise from metadata refs into libstd; the virtualized paths look like /rustc/1.45.0/src/libstd/io/cursor.rs rather than /Users/felixklock/Dev/Mozilla/rust.git/src/libstd/io/cursor.rs)

This way, we can emit the virtual name into things like the like the StableSourceFileId (as was done back before PR #70642) that ends up in incremental build artifacts, while still using the devirtualized file path when we want to access the file.

Fix #70924

pnkfelix added beta-nominated

Nominated for backporting to the compiler in the beta channel.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

May 30, 2020

(and I want to discourage further use of it if possible.)

…ths.

Such splits arise from metadata refs into libstd.

This way, we can (in a follow on commit) continue to emit the virtual name into things like the like the StableSourceFileId that ends up in incremetnal build artifacts, while still using the devirtualized file path when we want to access the file.

Note that this commit is intended to be a refactoring; the actual fix to the bug in question is in a follow-on commit.

A note on the lack of a regression test for #70924: I don't have a regression test as part of the PR, because I'm not sure how to make one. my local replication relies on either adding/removing the rust-src component in rustup , or actually renaming the rustlib/src/rust subdirectory in my build directory.

(Either of these options don't seem to lend themselves to a test in our infrastructure, no?)

I'm going to unilaterally tag this as beta-accepted, based on the conversation in today's T-compiler meeting

However, as noted in my recent chat with @Mark-Simulacrum: please do wait until after this has been reviewed before backporting it into beta and/or stable. I don't want my unilateral beta-backport approval to inadvertently sidestep the review process entirely.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with or without extra long-term/low-priority FIXME (for removing the /rustc/.../ encoding into paths altogether)

📌 Commit a8e4236 has been approved by eddyb

bors added the S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

label

May 30, 2020

@bors rollup=never

(this change is subtle and hard to test, and I want to maximize our ability to bisect to it if necessary.)

@bors p=1

this is a bugfix to a subtle incremental compilation bug that I have seen come up in my own workflow for building rustc itself (ironically, it happened while I was double-checking the intermediate commits of this PR itself were buildable). So I am upping the priority slightly, but not more than the PR's that have also had their priorities increased.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 1, 2020

…imulacrum

[stable] 1.44 release

This includes a release notes update as usual and a backport of rust-lang#72767.

r? @ghost

A note on the lack of a regression test for #70924: I don't have a regression test as part of the PR, because I'm not sure how to make one. my local replication relies on either adding/removing the rust-src component in rustup , or actually renaming the rustlib/src/rust subdirectory in my build directory.

(Either of these options don't seem to lend themselves to a test in our infrastructure, no?)

I just realized that I might be able to test this by invoking rustc while overriding the sysroot via --sysroot. I'll look into that, and if it bears fruit, then I'll make a regression test for #70924 in a separate PR.

This was referenced

Jun 3, 2020

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request

Jun 5, 2020