commit 9de2ea51187bf11a06cc7089b5c6fddcfda555f6
author ulfjack <ulfjack@google.com> Wed Oct 17 09:23:23 2018 -0700
committer Copybara-Service <copybara-piper@google.com> Wed Oct 17 09:24:50 2018 -0700
tree 891f533da91ad92db54cf6c73bebc484b995eacc
parent 03fac42db6629ac0574086a5b955f6c7193f6f09

Make ActionMetadataHandler stricter

Don't store metadata for output artifacts that are not declared outputs
of the action.

This solves the issue with https://github.com/bazelbuild/bazel/commit/a6255612e4892729d3758775c76085b26b9bc584.

The ActionExecutionFunction uses a per-build cache for the entire build,
and a per-action cache for each action. It runs each action with the
DelegatingPairFileCache, which first asks the the per-action cache, and then
the per-build cache for data.

When we do action input discovery (aka include scanning), the per-action
cache does not contain input header files, including input header files
that are outputs of dependent actions.

The (Google-internal) remote execution plugin first asks the cache for
metadata for each input file to construct the remote execution request.
The initial request may fail if the remote system does not have one or
more of the input files. In that case, it reports the metadata of the
missing input files, _but not the names_. The plugin then looks up the
name from the metadata from a local hash map. However, it requires the
ActionInput object in order to be able to upload the file. In order to
find the ActionInput, it asks the cache for the object using the name.

Before https://github.com/bazelbuild/bazel/commit/a6255612e4892729d3758775c76085b26b9bc584, the PerActionFileCache consulted the
ActionInputMap first to get the metadata and later to resolve the
ActionInput from the name.

After https://github.com/bazelbuild/bazel/commit/a6255612e4892729d3758775c76085b26b9bc584, the ActionMetadataHandler does a more involved
lookup. In particular, if the ActionInput corresponds to an output
Artifact, then it first consults the ActionInputMap, but if the output
is not there, it assumes that the ActionInput is an output of the
current action and stats the file on disk to obtain the metadata.

This would not be a problem, except that the DelegatingPairFileCache
uses the presence of metadata in the ActionInput lookup call to decide
which of the two caches to consult. If the ActionMetadataHandler has
metadata, then the DelegatingPairFileCache also asks the
ActionMetadataHandler for the ActionInput. However, the
ActionMetadataHandler only consults the ActionInputMap to look up the
ActionInput from the file name, which doesn't know about the Artifact
since it's neither an input nor an output of the current action.

By making the ActionMetadataHandler stricter, we avoid this situation -
if it's asked for metadata for an artifact that is neither an input nor
an output of the current action, it simply returns null unless we're in
strict mode, i.e., outside of include scanning. In the latter case, it
throws an exception.

PiperOrigin-RevId: 217531155