Allow ninja_build .d file processing to handle undeclared inputs

This introduces a new Artifact type, "NinjaMysteryArtifact", which represents a generated artifact that was not explicitly declared as any action output, though it is known to have been generated as a side effect due to other info in the graph (for example, post-execution .d files of other actions).

RELNOTES: None.
PiperOrigin-RevId: 307908968

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -185,7 +185,7 @@ public abstract class Artifact
   public static SkyKey key(Artifact artifact) {
     if (artifact.isTreeArtifact()
         || artifact.isMiddlemanArtifact()
-        || artifact.isSourceArtifact()) {
+        || !artifact.hasKnownGeneratingAction()) {
       return artifact;
     }
 
@@ -279,6 +279,50 @@ private Artifact(ArtifactRoot root, PathFragment execPath, boolean contentBasedP
     this.contentBasedPath = contentBasedPath;
   }
 
+  /**
+   * An artifact corresponding to a file in the output tree, generated by an {@link Action}, but for
+   * which the generating action is unknown (may have been created as an undeclared "side effect" of
+   * another action).
+   *
+   * <p>This artifact type defined for compatibility with the Ninja build system.
+   *
+   * <p>Only use this type in rare cases, and only when there is certainty that the artifact is
+   * actually generated by an action in the build (for example, when another action declares it as
+   * an explicit dependency in a post-execution .d file). These artifacts are indicative of
+   * underspecified, incomplete builds, and may in worst cases be indicative of incorrect builds.
+   */
+  @Immutable
+  public static final class NinjaMysteryArtifact extends Artifact {
+    public NinjaMysteryArtifact(ArtifactRoot root, PathFragment execPath) {
+      super(root, execPath, false);
+    }
+
+    @Override
+    public PathFragment getRootRelativePath() {
+      return getExecPath().relativeTo(getRoot().getExecPath());
+    }
+
+    @Override
+    boolean ownersEqual(Artifact other) {
+      return true;
+    }
+
+    @Override
+    public ArtifactOwner getArtifactOwner() {
+      return ArtifactOwner.NullArtifactOwner.INSTANCE;
+    }
+
+    @Override
+    public Label getOwnerLabel() {
+      return ArtifactOwner.NullArtifactOwner.INSTANCE.getLabel();
+    }
+
+    @Override
+    public boolean hasKnownGeneratingAction() {
+      return false;
+    }
+  }
+
   /** An artifact corresponding to a file in the output tree, generated by an {@link Action}. */
   @AutoCodec
   public static class DerivedArtifact extends Artifact {
@@ -531,6 +575,13 @@ public final boolean isSourceArtifact() {
     return root.isSourceRoot();
   }
 
+  /**
+   * Returns true iff this artifact has a generating action, and that generating action is known.
+   */
+  public boolean hasKnownGeneratingAction() {
+    return !isSourceArtifact();
+  }
+
   /**
    * Returns true iff this is a middleman Artifact as determined by its root.
    *

src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java

@@ -177,7 +177,9 @@ interface Singleton {}
 
   public static FileArtifactValue createForSourceArtifact(Artifact artifact, FileValue fileValue)
       throws IOException {
-    Preconditions.checkState(artifact.isSourceArtifact());
+    // Artifacts with known generating actions should obtain the derived artifact's SkyValue
+    // from the generating action, instead.
+    Preconditions.checkState(!artifact.hasKnownGeneratingAction());
     Preconditions.checkState(!artifact.isConstantMetadata());
     boolean isFile = fileValue.isFile();
     return create(

src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaAction.java

@@ -21,7 +21,9 @@
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.NinjaMysteryArtifact;
 import com.google.devtools.build.lib.actions.ArtifactResolver;
+import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.CommandLines;
 import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
@@ -51,6 +53,7 @@ public class NinjaAction extends SpawnAction {
   private final Root sourceRoot;
   @Nullable private final Artifact depFile;
   private final ImmutableMap<PathFragment, Artifact> allowedDerivedInputs;
+  private final ArtifactRoot derivedOutputRoot;
 
   public NinjaAction(
       ActionOwner owner,
@@ -63,7 +66,8 @@ public NinjaAction(
       ImmutableMap<String, String> executionInfo,
       RunfilesSupplier runfilesSupplier,
       boolean executeUnconditionally,
-      @Nullable Artifact depFile) {
+      @Nullable Artifact depFile,
+      ArtifactRoot derivedOutputRoot) {
     super(
         /* owner= */ owner,
         /* tools= */ tools,
@@ -92,6 +96,7 @@ public NinjaAction(
       }
     }
     this.allowedDerivedInputs = allowedDerivedInputsBuilder.build();
+    this.derivedOutputRoot = derivedOutputRoot;
   }
 
   private static CharSequence createProgressMessage(List<? extends Artifact> outputs) {
@@ -149,12 +154,18 @@ private void updateInputsFromDepfile(ActionExecutionContext actionExecutionConte
           inputArtifact = allowedDerivedInputs.get(execRelativePath);
         }
         if (inputArtifact == null) {
-          // Not a predeclared generated input, so it must be a source file.
           RepositoryName repository =
               PackageIdentifier.discoverFromExecPath(
                       execRelativePath, false, siblingRepositoryLayout)
                   .getRepository();
-          inputArtifact = artifactResolver.resolveSourceArtifact(execRelativePath, repository);
+          if (execRelativePath.startsWith(derivedOutputRoot.getExecPath())) {
+            // This input is a generated file which was not declared in the original inputs for
+            // this action.
+            inputArtifact = new NinjaMysteryArtifact(derivedOutputRoot, execRelativePath);
+          } else {
+            // Source file input.
+            inputArtifact = artifactResolver.resolveSourceArtifact(execRelativePath, repository);
+          }
         }
 
         if (inputArtifact == null) {

src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaActionsHelper.java

@@ -194,7 +194,8 @@ private void createNinjaAction(NinjaTarget target) throws GenericParsingExceptio
             executionInfo,
             EmptyRunfilesSupplier.INSTANCE,
             isAlwaysDirty,
-            depFile));
+            depFile,
+            artifactsHelper.getDerivedOutputRoot()));
   }
 
   /** Returns true is the action shpould be marked as always dirty. */

src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraphArtifactsHelper.java

@@ -97,6 +97,10 @@ DerivedArtifact createOutputArtifact(PathFragment pathRelativeToWorkingDirectory
     return ruleContext.getDerivedArtifact(execPath.relativeTo(outputRootPath), derivedOutputRoot);
   }
 
+  ArtifactRoot getDerivedOutputRoot() {
+    return derivedOutputRoot;
+  }
+
   Artifact getInputArtifact(PathFragment workingDirectoryPath) throws GenericParsingException {
     if (symlinkPathToArtifact.containsKey(workingDirectoryPath)) {
       return symlinkPathToArtifact.get(workingDirectoryPath);

src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java

@@ -82,7 +82,9 @@ public ArtifactFunction(Supplier<Boolean> mkdirForTreeArtifacts) {
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws ArtifactFunctionException, InterruptedException {
     Artifact artifact = (Artifact) skyKey;
-    if (artifact.isSourceArtifact()) {
+    if (!artifact.hasKnownGeneratingAction()) {
+      // If the artifact has no known generating action, it is either a source artifact, or a
+      // NinjaMysteryArtifact, which undergoes the same handling here.
       try {
         return createSourceValue(artifact, env);
       } catch (IOException e) {

src/test/shell/bazel/ninja_build_test.sh

@@ -52,8 +52,7 @@ build out/test/output.txt: cattool test/cattool.sh
 EOF
 
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/{one,two} > $OUTPUT
@@ -159,8 +158,7 @@ EOF
 function test_depfile_junk() {
   setup_basic_ninja_build
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/{one,two} > $OUTPUT
@@ -179,8 +177,7 @@ EOF
 function test_depfile_invalid_file() {
   setup_basic_ninja_build
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/{one,two} > $OUTPUT
@@ -210,8 +207,7 @@ build out/test/output.txt: cattool test/cattool.sh out/test/generated
 EOF
 
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/one out/test/generated  > $OUTPUT
@@ -249,8 +245,7 @@ build out/test/output.txt: cattool test/cattool.sh test/one
 EOF
 
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/one out/test/generated  > $OUTPUT
@@ -269,8 +264,7 @@ EOF
 function test_depfile_not_generated() {
   setup_basic_ninja_build
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 
 cat test/{one,two} > $OUTPUT
 EOF
@@ -294,8 +288,7 @@ build out/test/generated: filecopy test/two
 build out/test/output.txt: cattool test/cattool.sh test/one out/test/generated
 EOF
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/one  > $OUTPUT
@@ -321,8 +314,7 @@ EOF
   expect_log "INFO: 1 process"
 
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/one out/test/generated > $OUTPUT
@@ -370,8 +362,7 @@ build out/test/output.txt: cattool test/cattool.sh test/one external/two
 EOF
 
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/one external/two > $OUTPUT
@@ -436,8 +427,7 @@ build out/test/output.txt: cattool test/cattool.sh test/one
 EOF
 
   cat > test/cattool.sh <<'EOF'
-for i in $@; do :; done
-OUTPUT=$i
+OUTPUT=${!#}
 DEPFILE="$(dirname $OUTPUT)/depfile.d"
 
 cat test/one external/foo/two > $OUTPUT
@@ -479,4 +469,94 @@ function test_basic_depfile_processing() {
   expect_log "HELLOb"
 }
 
+function test_depfile_undeclared_generated_inputs() {
+  setup_basic_ninja_build
+
+  cat > test/build.ninja <<'EOF'
+rule filecopy_with_side_effect
+  command = test/filecopy_with_side_effect.sh ${in} ${out}
+rule cattool
+  depfile = out/test/depfile.d
+  command = ${in} ${out}
+build out/test/generated: filecopy_with_side_effect test/one
+build out/test/output.txt: cattool test/cattool.sh out/test/generated
+EOF
+
+  cat > test/filecopy_with_side_effect.sh <<'EOF'
+cat $1 > $2
+cat $1 > out/side_effect
+EOF
+  chmod +x test/filecopy_with_side_effect.sh
+
+  cat > test/cattool.sh <<'EOF'
+OUTPUT=${!#}
+DEPFILE="$(dirname $OUTPUT)/depfile.d"
+
+cat out/test/generated out/side_effect > "$OUTPUT"
+
+echo "$1 : out/test/generated out/side_effect" > "$DEPFILE"
+EOF
+
+  printf "HELLO" > test/one
+  bazel build //test:ninjabuild --experimental_ninja_actions &> "$TEST_log" \
+      || fail "should have generated output successfully"
+
+  cat bazel-workspace/out/test/output.txt &> "$TEST_log"
+  expect_log "HELLOHELLO"
+
+  printf "GOODBYE" > test/one
+
+  bazel build //test:ninjabuild --experimental_ninja_actions &> "$TEST_log" \
+      || fail "should have generated output successfully"
+
+  cat bazel-workspace/out/test/output.txt &> "$TEST_log"
+  expect_log "GOODBYEGOODBYE"
+}
+
+function test_depfile_modify_side_effect_file() {
+  setup_basic_ninja_build
+
+  cat > test/build.ninja <<'EOF'
+rule filecopy_with_side_effect
+  command = test/filecopy_with_side_effect.sh ${in} ${out}
+rule cattool
+  depfile = out/test/depfile.d
+  command = ${in} ${out}
+build out/test/generated: filecopy_with_side_effect test/one
+build out/test/output.txt: cattool test/cattool.sh out/test/generated
+EOF
+
+  cat > test/filecopy_with_side_effect.sh <<'EOF'
+cat $1 > $2
+cat $1 > out/side_effect
+EOF
+  chmod +x test/filecopy_with_side_effect.sh
+
+  cat > test/cattool.sh <<'EOF'
+OUTPUT=${!#}
+DEPFILE="$(dirname $OUTPUT)/depfile.d"
+
+cat out/test/generated out/side_effect > "$OUTPUT"
+
+echo "$1 : out/test/generated out/side_effect" > "$DEPFILE"
+EOF
+
+  printf "HELLO" > test/one
+  bazel build //test:ninjabuild --experimental_ninja_actions &> "$TEST_log" \
+      || fail "should have generated output successfully"
+
+  cat bazel-workspace/out/test/output.txt &> "$TEST_log"
+  expect_log "HELLOHELLO"
+
+  printf "GOODBYE" > bazel-workspace/out/side_effect
+
+  bazel build //test:ninjabuild --experimental_ninja_actions &> "$TEST_log" \
+      || fail "should have generated output successfully"
+
+  cat bazel-workspace/out/test/output.txt &> "$TEST_log"
+  # This verifies that changing out/side_effect retriggers the action.
+  # ("HELLO" is from out/test/generated, "GOODBYE" is from out/side_effect)
+  expect_log "HELLOGOODBYE"
+}
+
 run_suite "ninja_build rule tests"