Rename ActionExecutionValue#createSimpleFileArtifactValue and make it an instance method. The name of the method is misleading because it doesn't create anything new, just gets it from a map. It also asserts that it exists, so rename it to getExistingFileArtifactValue. Also, make it an instance method. A static method that requires passing in an instance of the class seems awkward. RELNOTES: None. PiperOrigin-RevId: 312712401
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 91b428d..291f42a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -39,6 +39,7 @@ import com.google.devtools.build.lib.actions.ActionRewoundEvent; import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.DiscoveredInputsEvent; import com.google.devtools.build.lib.actions.FileArtifactValue; @@ -1049,8 +1050,8 @@ } else if (retrievedMetadata instanceof ActionExecutionValue) { inputData.putWithNoDepOwner( input, - ActionExecutionValue.createSimpleFileArtifactValue( - (Artifact.DerivedArtifact) input, (ActionExecutionValue) retrievedMetadata)); + ((ActionExecutionValue) retrievedMetadata) + .getExistingFileArtifactValue((DerivedArtifact) input)); } else if (retrievedMetadata instanceof MissingFileArtifactValue) { inputData.putWithNoDepOwner(input, FileArtifactValue.MISSING_FILE_MARKER); } else if (retrievedMetadata instanceof FileArtifactValue) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java index 1a2587e..7116a0a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; @@ -117,19 +118,22 @@ } /** - * Create {@link FileArtifactValue} for artifact that must be non-middleman non-tree derived - * artifact. + * Retrieves a {@link FileArtifactValue} for a regular (non-middleman, non-tree) derived artifact. + * + * <p>The value for the given artifact must be present. */ - static FileArtifactValue createSimpleFileArtifactValue( - Artifact.DerivedArtifact artifact, ActionExecutionValue actionValue) { - Preconditions.checkState(!artifact.isMiddlemanArtifact(), "%s %s", artifact, actionValue); - Preconditions.checkState(!artifact.isTreeArtifact(), "%s %s", artifact, actionValue); + FileArtifactValue getExistingFileArtifactValue(DerivedArtifact artifact) { + Preconditions.checkState( + !artifact.isMiddlemanArtifact() && !artifact.isTreeArtifact(), + "Cannot request %s from %s", + artifact, + this); return Preconditions.checkNotNull( - actionValue.getArtifactValue(artifact), - "%s %s %s", + getArtifactValue(artifact), + "Missing artifact %s (generating action key %s) in %s", artifact, artifact.getGeneratingActionKey(), - actionValue); + this); } /**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java index 9e15389..9c7a570 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
@@ -22,6 +22,8 @@ import com.google.devtools.build.lib.actions.ActionLookupValue; import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; +import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; @@ -51,7 +53,7 @@ inputMap.put(artifact, entry.second, /*depOwner=*/ key); if (artifact.isFileset()) { ImmutableList<FilesetOutputSymlink> expandedFileset = - getFilesets(env, (Artifact.SpecialArtifact) artifact); + getFilesets(env, (SpecialArtifact) artifact); if (expandedFileset != null) { filesetsInsideRunfiles.put(artifact, expandedFileset); } @@ -87,11 +89,10 @@ } else if (value instanceof ActionExecutionValue) { inputMap.put( key, - ActionExecutionValue.createSimpleFileArtifactValue( - (Artifact.DerivedArtifact) key, (ActionExecutionValue) value), + ((ActionExecutionValue) value).getExistingFileArtifactValue((DerivedArtifact) key), key); if (key.isFileset()) { - topLevelFilesets.put(key, getFilesets(env, (Artifact.SpecialArtifact) key)); + topLevelFilesets.put(key, getFilesets(env, (SpecialArtifact) key)); } } else { Preconditions.checkState(value instanceof FileArtifactValue); @@ -100,7 +101,7 @@ } static ImmutableList<FilesetOutputSymlink> getFilesets( - Environment env, Artifact.SpecialArtifact actionInput) throws InterruptedException { + Environment env, SpecialArtifact actionInput) throws InterruptedException { Preconditions.checkState(actionInput.isFileset(), actionInput); ActionLookupData generatingActionKey = actionInput.getGeneratingActionKey(); ActionLookupKey filesetActionLookupKey = generatingActionKey.getActionLookupKey(); @@ -113,8 +114,8 @@ ActionLookupData filesetActionKey; if (generatingAction instanceof SymlinkAction) { - Artifact.DerivedArtifact outputManifest = - (Artifact.DerivedArtifact) generatingAction.getInputs().getSingleton(); + DerivedArtifact outputManifest = + (DerivedArtifact) generatingAction.getInputs().getSingleton(); ActionLookupData manifestGeneratingKey = outputManifest.getGeneratingActionKey(); Preconditions.checkState( manifestGeneratingKey.getActionLookupKey().equals(filesetActionLookupKey), @@ -125,8 +126,8 @@ manifestGeneratingKey); ActionAnalysisMetadata symlinkTreeAction = filesetActionLookupValue.getAction(manifestGeneratingKey.getActionIndex()); - Artifact.DerivedArtifact inputManifest = - (Artifact.DerivedArtifact) symlinkTreeAction.getInputs().getSingleton(); + DerivedArtifact inputManifest = + (DerivedArtifact) symlinkTreeAction.getInputs().getSingleton(); ActionLookupData inputManifestGeneratingKey = inputManifest.getGeneratingActionKey(); Preconditions.checkState( inputManifestGeneratingKey.getActionLookupKey().equals(filesetActionLookupKey),
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java index c582e7b..e2c4621 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -216,8 +216,7 @@ for (TreeFileArtifact treeFileArtifact : treeFileArtifacts) { FileArtifactValue value = - ActionExecutionValue.createSimpleFileArtifactValue( - treeFileArtifact, actionExecutionValue); + actionExecutionValue.getExistingFileArtifactValue(treeFileArtifact); if (FileArtifactValue.OMITTED_FILE_MARKER.equals(value)) { omitted = true; } else { @@ -333,8 +332,8 @@ fileInputsBuilder.add( Pair.of( input, - ActionExecutionValue.createSimpleFileArtifactValue( - (DerivedArtifact) input, (ActionExecutionValue) inputValue))); + ((ActionExecutionValue) inputValue) + .getExistingFileArtifactValue((DerivedArtifact) input))); } else if (inputValue instanceof TreeArtifactValue) { directoryInputsBuilder.add(Pair.of(input, (TreeArtifactValue) inputValue)); } else {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java index d636b21..f260515 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
@@ -22,6 +22,7 @@ import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileStateType; @@ -278,9 +279,8 @@ fsVal = (HasDigest) value; } else if (value instanceof ActionExecutionValue) { fsVal = - Preconditions.checkNotNull( - ActionExecutionValue.createSimpleFileArtifactValue( - (Artifact.DerivedArtifact) artifact, (ActionExecutionValue) value)); + ((ActionExecutionValue) value) + .getExistingFileArtifactValue((DerivedArtifact) artifact); } else { return NON_EXISTENT_FILE_INFO; }