Stop adding .d files to outputs when dependency_file is disabled

[keith]

Previously if you disabled the dependency_file feature, your build would
fail because the .d file was still added to the outputs of the action
but would not be generated.

Fixes #7769

[keith]

@hlopko can you review this?

[hlopko]

Hi Keith,

I'll need to investigate this deeper, but my worry is that C++ compile actions are not correct without .d file (meaning we will not recompile on changed header). One quick workaround is to use a compiler wrapper, and postprocess .d file to remove absolute path prefix.

To make C++ rules work correctly without .d file we have to stop creating middleman artifact in compilation context and instead use nested set of headers for action inputs. @scentini can tell you more if you're interested.

[keith]

@hlopko I think you're right about system level headers, but in the common case those shouldn't be mutated right?

If this is required regardless, should we remove the ability to disable this feature?

[keith]

Actually without a .d file at all wouldn't this just mean it would have to re-check all system headers each time, which could lead to more work, but not an invalid build?

[keith]

@hlopko any other thoughts?

[hlopko]

So imagine a simple workspace:

BUILD:

cc_library(
  name = "foo",
  hdrs = [ "foo.h" ],
  srcs = [ "foo.cc" ],
)

foo.cc:

#include "foo.h"

foo.h:

int foo() { return 42; }

And you build it by bazel build //:foo --features=-dependency_file (with your fix). Now when you change foo.h to contain:

int bar() { return 42; }

and run Bazel, you'll see the library was not recompiled. This is what I mean by 'not correct', Bazel doesn't correctly identify which actions need to be reexecuted.

So I see 2 ways forward:

1. Adding a check that 'dependency_file' feature is enabled and if not show a nice error message
2. Stop using middleman artifacts in https://source.bazel.build/bazel/+/59fbdb3d5e993423ae641b1d4262eb5059f96bb6:src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java;l=760 when 'dependency_file' is not enabled. @scentini's help will be needed for this.

[kastiglione]

Forgive the uninformed question, but why does bazel not track hdrs like it does srcs? In your example, the build graph is knows the foo target depends on foo.h, a .d doesn't provide additional info in such a case.

[hlopko]

@kastiglione It does, but :)

There is an optimization in Bazel based on the hypothesis that substantial number of headers that are passed to compilation are actually unused. We see ~50% of transitive headers in compilation actions are not included by source file internally at Google. Now if any of the unused headers would change, we would reexecute the compilation action and discover that nothing changed. Plus backwards dependency edges consume a lot of memory at scale.

So we pass all the headers to the first compilation action, get the .d file, and only declare dependency edges on the pruned set of headers. Subsequent action executions work as you expect. All works fine if there is a .d file. If there is not, Bazel crashes. This PR tries to fix that, but the fix is a little bit more involved, but still doable.

[goldencz]

If it is important to keep bazel work correctly, why doens't just remove this feature? It's really confusing that it's said this is a feature, which implies it could be switch on/off. But actually it's not. Bazel just fail when this feature is off.