Prefer remapping the relative library/ and compiler/ directories
#150110
Conversation
This is done to avoid leaking the relative paths to the standard library after the overall of filenames. Noted that the paths were already leaking before, but to a lesser extent since the paths embedded in the distributed `rlib` were absolute. In general Cargo compiles workspace members with relative paths, so it's better anyway to remap the relative path. cf https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/remapping.20of.20the.20standard.20library/near/564093571
rustbot
added
S-waiting-on-review
|
@bors try |
Prefer remapping the relative `library/` and `compiler/` directories
This comment has been minimized.
This comment has been minimized.
|
@bors r+ rollup=never |
bors
added
S-waiting-on-bors
Prefer remapping the relative `library/` and `compiler/` directories This is done to avoid leaking the relative paths to the standard library after the overall of filenames in #149709. Noted that the paths were already leaking before, but to a lesser extent since most (but not all) the paths embedded in the distributed `rlib` were absolute. In general Cargo compiles workspace members with relative paths, so it's better anyway to remap the relative path. In addition to our tests I have manually confirmed that it also works as expected for the printed diagnostics paths. cf. https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/remapping.20of.20the.20standard.20library/near/564093571
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
Huh. |
|
Those tests failures are interesting, they show the remapped paths, despite I have manually checked and the above prefix change works as expected for imported files (ie files described in Instead they come from the compiled artifact it-self, and more specifically the debuginfo, which is retrived by the backtraces and shown on those tests. (yes, even in the The reason this wasn't an issue before is because were only producing the relative paths, which are not remapped before this PR. As for why those tests didn't fail on PR CI, I have no idea. In any-case we well need a mechanism that supplants the current This is even complicated by the fact depending on whenever the compiler/std was compiled with remapping on or off changes the kind of paths we get, as with remapping on we will get absolute paths (starting with Given that all of those test already had custom normalization for those backtrace paths, I have opted to add a simple |
|
@bors try jobs=dist-x86_64-linux |
This comment has been minimized.
This comment has been minimized.
Prefer remapping the relative `library/` and `compiler/` directories try-job: dist-x86_64-linux
|
@bors r+ |
bors
added
S-waiting-on-bors
5 participants
This is done to avoid leaking the relative paths to the standard library after the overall of filenames in #149709.
Noted that the paths were already leaking before, but to a lesser extent since most (but not all) the paths embedded in the distributed
rlibwere absolute.In general Cargo compiles workspace members with relative paths, so it's better anyway to remap the relative path.
In addition to our tests I have manually confirmed that it also works as expected for the printed diagnostics paths.
cf. https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/remapping.20of.20the.20standard.20library/near/564093571
r? @jieyouxu