Skip to content
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

remap-path-prefix appears to be applied in a reverse order #82108

Closed
nagisa opened this issue Feb 14, 2021 · 3 comments
Closed

remap-path-prefix appears to be applied in a reverse order #82108

nagisa opened this issue Feb 14, 2021 · 3 comments
Labels
A-reproducibility Area: Reproducible / deterministic builds D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nagisa
Copy link
Member

nagisa commented Feb 14, 2021

It appears that if multiple--remap-path-prefix arguments are supplied on the CLI, they will be applied in the reverse order, rather than in the CLI-occurence order.

To reproduce:

$ mkdir -p apple/banana
$ echo 'fn main(){ panic!() }' > apple/banana/chaenomeles.rs
$ rustc apple/banana/chaenomeles.rs --remap-path-prefix=apple/banana/=/first/ --remap-path-prefix=apple/=/second/
$ ./chaenomeles
thread 'main' panicked at 'explicit panic', /second/banana/chaenomeles.rs:1:12
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Note that only the latter --remap-path-prefix got invoked and the panic path became /second/banana/chaenomeles.rs. Instead users need to specify the order of remappings in reverse:

$ rustc apple/banana/chaenomeles.rs --remap-path-prefix=apple/=/first/ --remap-path-prefix=apple/banana/=/second/
$ ./chaenomeles
thread 'main' panicked at 'explicit panic', /second/chaenomeles.rs:1:12
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

(in contrast clang/gcc will apply remappings front-to-back)

@nagisa nagisa added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-reproducibility Area: Reproducible / deterministic builds D-papercut Diagnostics: An error or lint that needs small tweaks. labels Feb 14, 2021
@nagisa
Copy link
Member Author

nagisa commented Feb 14, 2021

Unclear to me if we can fix this, however, so perhaps we should warn if people write more specific paths first and less specific ones later?

@cbeuw
Copy link
Contributor

cbeuw commented Mar 30, 2021

This seems like a deliberate design choice:

/// Applies any path prefix substitution as defined by the mapping.
/// The return value is the remapped path and a boolean indicating whether
/// the path was affected by the mapping.
pub fn map_prefix(&self, path: PathBuf) -> (PathBuf, bool) {
// NOTE: We are iterating over the mapping entries from last to first
// because entries specified later on the command line should
// take precedence.
for &(ref from, ref to) in self.mapping.iter().rev() {
if let Ok(rest) = path.strip_prefix(from) {
return (to.join(rest), true);
}
}
(path, false)
}

.rev() has been there since when path remapping was first introduced back in 2017: 39ffea3

Indeed by simply removing .rev() it gives the same behaviour as clang/gcc

$ build/x86_64-unknown-linux-gnu/stage1/bin/rustc apple/banana/chaenomeles.rs --remap-path-prefix=apple/banana/=/first/ --remap-path-prefix=apple/=/second/
$ ./chaenomeles thread 'main' panicked at 'explicit panic', /first/chaenomeles.rs:1:12
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The problem here is that is this a bug or a feature? It is working as intended by design, but the design was unusual when compared against clang/gcc

@nagisa
Copy link
Member Author

nagisa commented Mar 30, 2021

Yeah, I opened this issue basically to figure out an answer to the last question. The source you linked seems to suggest to me its intended and given that this behaviour is long-standing I doubt we are in a position to change it anyway.

Thank you for looking into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-reproducibility Area: Reproducible / deterministic builds D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants