[PATCH] D36391: [llvm-cov] Add an option which maps the location of source directories on another machine to your local copies

[Sean Eveson via Phabricator via llvm-commits]

seaneveson added a comment.

Thanks for your feedback, I think I've addressed all your comments (or replied to them).



================
Comment at: test/tools/llvm-cov/scan-directory.test:11
 
-RUN: llvm-cov show /dev/null -instr-profile /dev/null -dump-collected-paths -filename-equivalence %t a b c d e f | FileCheck %s --check-prefix=EQUIV
+// NEEDS FIX.
+RUN: llvm-cov show /dev/null -instr-profile /dev/null -dump-collected-paths -path-equivalence=/tmp,%S %t a b c d e f | FileCheck %s --check-prefix=EQUIV
----------------
I'm not sure what the expected output should be now. The actual output is:

```
error: a: Missing source file
error: b: Missing source file
error: c: Missing source file
error: d: Missing source file
error: e: Missing source file
error: f: Missing source file
D:\Test\x\a\b\c.tmp
D:\Test\x\a\d.tmp
```


================
Comment at: tools/llvm-cov/CodeCoverage.cpp:822
+        if (NativeFilename.startswith(PathRemapping.first)) {
+          RemappedFilenames[Filename] =
+              PathRemapping.second +
----------------
vsk wrote:
> You should only need to call native() on the string Filename is mapped to.
I think you do have to call native on both paths and the Filename to cover the case where the coverage data was generated on linux and llvm-cov is being run on windows (like in the lit tests).

Since the RemapFrom path and the Filename path are being compared it is important they are both native. Otherwise the RemapFrom path could be anything the user typed in, and we can't be sure about Filename because the data could come from another platform.

Given that the Filename path is then already native, it is quicker and easier to make RemapTo native and then just append the NativeFilename.


https://reviews.llvm.org/D36391