Fix per-file incremental analysis

[nettle]

Why: Incremental analysis doesn't work with the per-file rule

What: Make inputs dependent on headers only
Note: depending on headers only may mean that CTU wont work!

Test:

bazel clean --expunge
bazel build //test:code_checker_fail -s
vim test/src/pass.cc
bazel build //test:code_checker_fail -s

Addresses: #21

[Szelethus]

Do I understand correctly that in this case if a header changes, still all TUs are reanalized, considering that each .cpp file is dependent on all headers?

Or the idea is that the patch demonstrates that we are making bazel think of the analysis rules on a per-TU basis, where we can still refine the header set later down the line?

[nettle]

Do I understand correctly that in this case if a header changes, still all TUs are reanalized, considering that each .cpp file is dependent on all headers?

Well, yes, at least per target. We can say for one target each src depends on all headers within this target.
That's the idea.

Or the idea is that the patch demonstrates that we are making bazel think of the analysis rules on a per-TU basis, where we can still refine the header set later down the line?

Sure, it is per-TU basis, that's how we run CodeChecker analyze --file
The dependency on headers might be optimized and CTU is still the problem.

[Szelethus]

In terms of the high level stuff, this looks like a great step forward. Nice!

I see a few obstacles/future steps:

• We need some tests where we demonstate that changing a cpp file only triggers its own reanalysis, and one where changing a header (as of now) triggers a whole reanalysis
• I'd be happy to see the CI patches land (#37 or #28 at the very least) before this,
• We discussed in the related issue (#21) that in time, we should retrieve the true header dependency set. It sure sounds plausible (it really must be the case) that bazel knows the true header dependencies, even if the hdrs and srcs fields aren't the way to get them. Tibor mentioned some .d files that bazel generates but seem hard to access -- in loose, informal terms, how would you approach this problem? Make bazel list the header dependencies, should we have a compiler call, or ask the CodeChecker folks (which we will need to do for CTU anyways later on) to create a dry-run option?

[nettle]

• We need some tests where we demonstate that changing a cpp file only triggers its own reanalysis, and one where changing a header (as of now) triggers a whole reanalysis
• I'd be happy to see the CI patches land (Run tests in CI #37 or CI for unit tests #28 at the very least) before this,

Totally agree with those.

• We discussed in the related issue (Incremental analysis doesn't work with the per-file rule #21) that in time, we should retrieve the true header dependency set. It sure sounds plausible (it really must be the case) that bazel knows the true header dependencies, even if the hdrs and srcs fields aren't the way to get them.

Well, these discussions are valid and we should continue experimenting with different approaches and ideas.
However, meanwhile I think we should submit this simple improvement even it is suboptimal but at least it is more Bazel-compatible.

Tibor mentioned some .d files that bazel generates but seem hard to access -- in loose, informal terms, how would you approach this problem? Make bazel list the header dependencies, should we have a compiler call, or ask the CodeChecker folks (which we will need to do for CTU anyways later on) to create a dry-run option?

Bazel does not generate anything except artifacts described as Action output (clang -MMD/-MD/-MM/-M does).
So, we can describe an Action(s) to get whatever we need to know.
BTW, remember also that CodeChecker is just a wrapper around Clang.

[Szelethus]

This patch is indeed an improvement! I'm perfectly okay refining the later steps in subsequent patches, I don't see anything inherent that would make it more difficult.

So, we can describe an Action(s) to get whatever we need to know.

Do you have plans to work on that yourself or would you prefer if @furtib and I took a crack at it?

BTW, remember also that CodeChecker is just a wrapper around Clang.

And what a wrapper it is! :) But yes, you are correct, we should probably patch clang (likely clang-extdef-mapping) directly instead of CodeChecker. We can consider later on whether it'd be good design to consult with clang through CodeChecker regardless, but thats a discussion for another day.

[Szelethus]

LGTM! @nettle, I'd like to have your approval on this as well.

[Szelethus]

As there are no objections so far, lets land this. If the you-know-what hits the fan, we can still change course after the fact.