fix_includes and relative paths

[pfaffe]

fix_includes.py can't deal with relative paths in the iwyu output if these paths are not relative to the cwd. This occurs if iwyu and fix_includes are run from different working directories, using e.g. a temporary file to store the output, or, more importantly when using iwyu_tool.py: When the compilation database contains "directory" entries that aren't the cwd or when we're using the -p argument.

Currently I have two work-arounds for this:

1. Have iwyu always output absolute paths. The change is small (absolutize the filename in IwyuFileInfo::CalculateAndReportIwyuViolations), but it basically breaks all the tests.
2. Add a -p option to fix_includes.py, in accordance to the iwyu_tool option. This change is much less invasive, but won't cover every situation in which fix_includes will see a relative filename.

[kimgr]

I don't use fix_includes.py myself. Can you be more specific around "can't deal with"? Which filenames are we talking about here, I'm guessing they're not include names but mentions of the file under analysis?

[pfaffe]

I'm guessing they're not include names but mentions of the file under analysis

Quite right. fix_includes will read the analyzed file to create a diff. It can't do that in this situation, because the path is wrong.

Consider this compilation db entry:

{
  "directory": "/home/pfaffe/code/iwyu/build",
  "command": "/home/pfaffe/.bin/c++   -DIWYU_GIT_REV=\\\"61c977d\\\" -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS  -fno-rtti -std=c++11 -I../llvm/include    -o CMakeFiles/include-what-you-use.dir/iwyu_preprocessor.cc.o -c /home/pfaffe/code/iwyu/iwyu_preprocessor.cc",
  "file": "/home/pfaffe/code/iwyu/iwyu_preprocessor.cc"
}

IWYU will report includes as ../llvm/include/llvm/Support/raw_ostream.h, which in this example is only valid relative to the build directory.

[kimgr]

1. Add a -p option to fix_includes.py, in accordance to the iwyu_tool option. This change is much less invasive, but won't cover every situation in which fix_includes will see a relative filename.

I like this better of the two options. Can you explain more on "won't cover every situation"?

[pfaffe]

In the case that there are different "directory" entries in the compilation db, relative paths may refer to different things.

[vsapsai]

@pfaffe, I don't understand the situation entirely. You mentioned that IWYU outputs relative paths for file(s) under analysis and IWYU outputs ../llvm/include/llvm/Support/raw_ostream.h. Why IWYU is giving recommendations for ../llvm/include/llvm/Support/raw_ostream.h when you are analysing iwyu_preprocessor.cc?

[pfaffe]

@vsapsai Looks like I might have botched that example, I made that one up. Of course the only time iwyu will request changes to a header file is for associated headers.

Here is some real output for a project of mine, for the associated header of the file Statistics.cc

../include/Statistics.h should add these lines:

../include/Statistics.h should remove these lines:
- #include <cstdlib>  // lines 3-3

The full include-list for ../include/Statistics.h:
#include <iosfwd>  // for ostream
---

[vsapsai]

Thanks for example, now I understand the situation better.

I assume in compilation DB entry for Statistics.cc "file" is absolute path and "command" uses relative path. Is it correct?

[pfaffe]

Yes, exactly. "command" looks something like

"command": "/usr/bin/c++ -std=c++14 -Wall -fPIC -I../include [...]"

[Flamefire]

My five cents: IWYU should output absolute paths. So it says /home/user/foo/include/Statistics.h should add these lines: This makes it more stable, as the file can't be ambiguous. As the tests expect relative paths in their verification I would add a macro like think to the tests that gets replaced by the current dir during the checks (e.g. <cwd> or <iwyuRoot>) Background: There are currently no tests for cc files given by absolute paths or by using something like ../src/sub/file.cc However those should be tested (see #276) Without the macro the verification is pretty hard, so if it needs to be done anyway, why not do it now?

We could additionally add a --strip-prefix or --relative-paths option to IWYU that removes a given prefix from the absolute path. This is even more usefull. Example: CMake project with external build dir (as it should be), files are compiled by e.g. ../src/sub/file.cc And I may want to strip everything including the src so all files are relative to the source directory when inspecting the results manually.