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.

Sign up for GitHub

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

Jump to bottom

Implement a file-path remapping feature in support of debuginfo and reproducible builds #41508

Merged
merged 5 commits into from
Apr 28, 2017
Merged

Implement a file-path remapping feature in support of debuginfo and reproducible builds #41508

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
39ffea3
Implement a file-path remapping feature in support of debuginfo and r…
michaelwoerister Apr 24, 2017
eb6308f
remap-path-prefix: Validate number of commandline arguments passed.
michaelwoerister Apr 25, 2017
5c62ce4
Make codegen test for remap-path-prefix more thorough.
michaelwoerister Apr 26, 2017
ab9691d
Add some documentation for -Zremap-path-prefix to the unstable book
michaelwoerister Apr 26, 2017
8ea050d
Disable path remapping test on Windows.
michaelwoerister Apr 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/doc/unstable-book/src/SUMMARY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- [Compiler flags](compiler-flags.md)
- [linker_flavor](compiler-flags/linker-flavor.md)
- [remap_path_prefix](compiler-flags/remap-path-prefix.md)
- [Language features](language-features.md)
- [abi_msp430_interrupt](language-features/abi-msp430-interrupt.md)
- [abi_ptx](language-features/abi-ptx.md)
Expand Down
37 changes: 37 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/remap-path-prefix.md
View file
Open in desktop
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"
```

Copy link
Contributor

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:

  • Is it delimited by path element boundaries, or is it operating on literal strings?
  • Is there any normalization performed before/after doing the transformation? (Redundant / elimination?)
  • Are the input paths absolute, relative, or whatever was provided to the compiler?
  • In this example, if the input path were /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?

Copy link
Member Author

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.

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does

Mappings specified later on the line will take precedence over earlier ones.

actually mean? Do you mean that if the -Zremap-path-prefix-from is identical it is replaced? Or does it more generally apply, such that a more specific path overrides a more general one?

For example, what's the mapping for:

rustc -Zremap-path-prefix-from=my-project/src -Zremap-path-prefix-to=/sources/my-project \
      -Zremap-path-prefix-from=my-project/src/vendored/otherproject -Zremap-path-prefix-to=/sources/otherproject

when applied to my-project/src/vendored/otherproject/src/lib.rs?

In other words, does the ordering of the -from options matter when multiple mappings could apply? Or is there some other way to disambiguate multiple matching mappings?

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if defining the matching as:

  1. later remap-path-prefix-from replaces a previous identical one
  2. mappings are matched from longest from prefix to shortest

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would differ from the current behaviour in the example a/b -> y, a -> x. I'm not sure which is better, but without a concrete argument that the new one is better I'd suggest to stick with the current behaviour which is simple to describe and implement. The other behaviour would require a trie-like data structure or other more complex thing.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 BTreeMap<String, String> instead of Vec<String, String>. It would be nice to have an algorithm that can have more efficient implementations (ie, not linear search), to cope with the case where there are lots of mappings (but that's definitely not a requirement for the first implementation).

Copy link
Contributor

@jsgf jsgf Apr 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@michaelwoerister

In my opinion, relying only on ordering is a good approach, since the rules are clear and without surprises

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).

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

@codyps codyps Apr 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

The 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).

Copy link
Contributor

Choose a reason for hiding this comment

The 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.