-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Implement a file-path remapping feature in support of debuginfo and reproducible builds #41508
Changes from 4 commits
39ffea3
eb6308f
5c62ce4
ab9691d
8ea050d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# `remap-path-prefix` | ||
|
||
The tracking issue for this feature is: [#41555](https://github.com/rust-lang/rust/issues/41555) | ||
|
||
------------------------ | ||
|
||
The `-Z remap-path-prefix-from`, `-Z remap-path-prefix-to` commandline option | ||
pair allows to replace prefixes of any file paths the compiler emits in various | ||
places. This is useful for bringing debuginfo paths into a well-known form and | ||
for achieving reproducible builds independent of the directory the compiler was | ||
executed in. All paths emitted by the compiler are affected, including those in | ||
error messages. | ||
|
||
In order to map all paths starting with `/home/foo/my-project/src` to | ||
`/sources/my-project`, one would invoke the compiler as follows: | ||
|
||
```text | ||
rustc -Zremap-path-prefix-from="/home/foo/my-project/src" -Zremap-path-prefix-to="/sources/my-project" | ||
``` | ||
|
||
Debuginfo for code from the file `/home/foo/my-project/src/foo/mod.rs`, | ||
for example, would then point debuggers to `/sources/my-project/foo/mod.rs` | ||
instead of the original file. | ||
|
||
The options can be specified multiple times when multiple prefixes should be | ||
mapped: | ||
|
||
```text | ||
rustc -Zremap-path-prefix-from="/home/foo/my-project/src" \ | ||
-Zremap-path-prefix-to="/sources/my-project" \ | ||
-Zremap-path-prefix-from="/home/foo/my-project/build-dir" \ | ||
-Zremap-path-prefix-to="/stable-build-dir" | ||
``` | ||
|
||
When the options are given multiple times, the nth `-from` will be matched up | ||
with the nth `-to` and they can appear anywhere on the commandline. Mappings | ||
specified later on the line will take precedence over earlier ones. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does
actually mean? Do you mean that if the For example, what's the mapping for:
when applied to In other words, does the ordering of the Is it guaranteed that at most a single mapping can be applied, or is there some notion of repeated application of mappings until nothing matches? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if defining the matching as:
would be better. I think this would make it deterministically always select more specific mappings over more general ones, rather than relying purely on ordering. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would differ from the current behaviour in the example I'm trying to standardise this behaviour across multiple compilers and GCC is already doing something very similar based on the ordering of command-line flags (and I have a patch pending to make it exactly match the behaviour being proposed in this PR). Changing this in the suggested way would make the spec for this standard even more complex. :( In real-world cases I don't think the issue will crop up, in rustc or anywhere else - it's unlikely that a child process inheriting a prefix-map from a parent process would want to add a less-specific mapping for a higher-level directory. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation is very simple: Walk all from/to pairs as provided on the commandline, starting at the last, and just stop at the first match. In my opinion, relying only on ordering is a good approach, since the rules are clear and without surprises. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current behaviour allows you to specify useless mappings (by putting more general before more specific in the reverse ordering), which might be inadvertent or unexpected. I don't think it would require very fancy structures; I think you could implement it pretty simply using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that's fine - I think the wording could do with clarification (ie, I managed to misinterpret what it meant until I looked at the source). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted. I'll provide an update in a subsequent PR and make sure to ping the usual suspects for review. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the ordering of flags for the lookup allows easy overriding of previous flags without needing to have a way to remove items from an accumulated list of flags (ex: presuming a variable in some higher level build env is accumulating RUST_FLAGS, using ordering means there is no need to parse all the accumulated flags to figure out which ones to remove when adding a higher priority remapping). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, that might be useful to have anyway, since an identity mapping is semantically different from no mapping at all. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ use lint; | |
use middle::cstore; | ||
|
||
use syntax::ast::{self, IntTy, UintTy}; | ||
use syntax::codemap::FilePathMapping; | ||
use syntax::parse::token; | ||
use syntax::parse; | ||
use syntax::symbol::Symbol; | ||
|
@@ -492,6 +493,14 @@ impl Options { | |
self.incremental.is_none() || | ||
self.cg.codegen_units == 1 | ||
} | ||
|
||
pub fn file_path_mapping(&self) -> FilePathMapping { | ||
FilePathMapping::new( | ||
self.debugging_opts.remap_path_prefix_from.iter().zip( | ||
self.debugging_opts.remap_path_prefix_to.iter() | ||
).map(|(src, dst)| (src.clone(), dst.clone())).collect() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a potentially confusing CLUI to me. At this point I would simply make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I know, it's not perfect. There's been quite some discussion about the CLUI in the initial issue (#38322). This seemed to be an acceptable compromise. It's an unstable feature at the moment. If our parser had some improvements before this is stabilized, we could certainly revisit this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that the values are paths, and most file systems do not like colon |
||
) | ||
} | ||
} | ||
|
||
// The type of entry function, so | ||
|
@@ -1012,6 +1021,10 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, | |
"Set the optimization fuel quota for a crate."), | ||
print_fuel: Option<String> = (None, parse_opt_string, [TRACKED], | ||
"Make Rustc print the total optimization fuel used by a crate."), | ||
remap_path_prefix_from: Vec<String> = (vec![], parse_string_push, [TRACKED], | ||
"add a source pattern to the file path remapping config"), | ||
remap_path_prefix_to: Vec<String> = (vec![], parse_string_push, [TRACKED], | ||
"add a mapping target to the file path remapping config"), | ||
} | ||
|
||
pub fn default_lib_output() -> CrateType { | ||
|
@@ -1319,7 +1332,7 @@ pub fn rustc_optgroups() -> Vec<RustcOptGroup> { | |
// Convert strings provided as --cfg [cfgspec] into a crate_cfg | ||
pub fn parse_cfgspecs(cfgspecs: Vec<String> ) -> ast::CrateConfig { | ||
cfgspecs.into_iter().map(|s| { | ||
let sess = parse::ParseSess::new(); | ||
let sess = parse::ParseSess::new(FilePathMapping::empty()); | ||
let mut parser = | ||
parse::new_parser_from_source_str(&sess, "cfgspec".to_string(), s.to_string()); | ||
|
||
|
@@ -1430,6 +1443,23 @@ pub fn build_session_options_and_crate_config(matches: &getopts::Matches) | |
output_types.insert(OutputType::Exe, None); | ||
} | ||
|
||
let remap_path_prefix_sources = debugging_opts.remap_path_prefix_from.len(); | ||
let remap_path_prefix_targets = debugging_opts.remap_path_prefix_from.len(); | ||
|
||
if remap_path_prefix_targets < remap_path_prefix_sources { | ||
for source in &debugging_opts.remap_path_prefix_from[remap_path_prefix_targets..] { | ||
early_error(error_format, | ||
&format!("option `-Zremap-path-prefix-from='{}'` does not have \ | ||
a corresponding `-Zremap-path-prefix-to`", source)) | ||
} | ||
} else if remap_path_prefix_targets > remap_path_prefix_sources { | ||
for target in &debugging_opts.remap_path_prefix_to[remap_path_prefix_sources..] { | ||
early_error(error_format, | ||
&format!("option `-Zremap-path-prefix-to='{}'` does not have \ | ||
a corresponding `-Zremap-path-prefix-from`", target)) | ||
} | ||
} | ||
|
||
let mut cg = build_codegen_options(matches, error_format); | ||
|
||
// Issue #30063: if user requests llvm-related output to one | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unclear from this how the remapping is actually operating. It should also answer:
/
elimination?)/home/foo/my-project/src-orig
, would this remap to/sources/my-project-orig
? Should there be a trailing/
to prevent this if its undesirable?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points. I'll provide some more documentation in a follow-up PR. The implementation as it is does as little as possible: (1) Take each string as it is provided to the compiler, (2) don't do any normalization, (3) do a simple string-level prefix replacement.