[PATCH] D151283: [llvm-cov] Support directory layout in coverage reports

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/tools/llvm-cov/CoverageReport.cpp:578
+
+    // What if we report a Windows exe on Linux? In this case, all files will
+    // be considered be directly in the project root directory as Linux allows
----------------
AtomicGu wrote:
> gulfem wrote:
> > AtomicGu wrote:
> > > @gulfem @phosek Any advises for this problem?
> > Can you use `sys::path::native` to convert the path to native path? I looked at `native_separators.c` test: 
> > https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-cov/native_separators.c
> > 
> > It's a Windows test, but its `*.covmapping` file input does not include backslashes for the filenames because it converts the path read to native path:  
> > https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-cov/Inputs/native_separators.covmapping
> > 
> > So, what I was thinking is that you can have two separate tests `directory_coverage.linux.test` and `directory_coverage.win.test`, but they use the same inputs for *.profdata and *.covmapping files. Do you think that would work? If not, having two separate files with their own inputs should be fine.
> Actually there's another difference between the two tests: the linux test uses absolute path in the .covmapping file but the windows test uses relative path.
> 
> The key problem here is: do we want to support generating reports for inputs of different platforms? I think `native_separators.c` answers "yes", so I will follow it.
Yes, I think so, we don't currently restrict handling of inputs to a particular platform so this would be a regression. I agree that using `sys::path::native` to convert all paths to native format is likely the right approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151283/new/

https://reviews.llvm.org/D151283