commit 36df7eddc16850d0974226ceef839d86eef35bdc
author lberki <lberki@google.com> Thu Jun 27 06:32:03 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Jun 27 06:33:18 2019 -0700
tree 1f1077866acb86f5c1dc640682e7cdeb3927f155
parent 04fc26c9d353aa6087976a228d0aeac73ea7a2fb

Remove the distinction between mandatory and non-mandatory artifact dependencies in ArtifactFunction.

This used to be used for implementing C++ include scanning: it's possible that a change both removes a header file and the reference to it. In the subsequent build, Blaze still needs to stat the removed file to verify whether the action cache entry is still valid; it's not, of course, since if the reference to the file is removed, at least one still-existing source file changes, but we don't know that at the time we check for an action cache hit.

So the action cache referencing a missing file should not be an error. Previously, this was implemented by a bit on the SkyKey of ArtifactFunction, which indicated whether an exception should be raised if the source file is missing.

However, it's just as simple to do this in ActionExecutionFunction, which has the benefit of limiting knowledge of this quirk to ActionExecutionFunction and requiring fewer Skyframe nodes and edges.

In certain cases, it can also result in performance benefits: if a discovered input is already statted before an action requiring it starts executing, no Skyframe restart is needed anymore.

Note that things are still not perfect: in particular, it's possible that when a reference to a discovered file is removed and that file is simultaneously replaced with symlink cycle (or made unreadable in some other way), we still raise an error and fail the build. This isn't new behavior introduced by this change, though.

RELNOTES: None.
PiperOrigin-RevId: 255388509