Remove references to MiddlemanType.AGGREGATING_MIDDLEMAN.
Replace some occurrences with RUNFILES_MIDDLEMAN, and remove code where applicable.
PiperOrigin-RevId: 382262465
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 4e6afaf..adc5992 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
@@ -75,9 +75,13 @@
Environment env,
MetadataConsumerForMetrics consumer)
throws InterruptedException {
- if (value instanceof AggregatingArtifactValue) {
- AggregatingArtifactValue aggregatingValue = (AggregatingArtifactValue) value;
- for (Pair<Artifact, FileArtifactValue> entry : aggregatingValue.getFileArtifacts()) {
+ if (value instanceof RunfilesArtifactValue) {
+ // Note: we don't expand the .runfiles/MANIFEST file into the inputs. The reason for that
+ // being that the MANIFEST file contains absolute paths that don't work with remote execution.
+ // Instead, the way the SpawnInputExpander expands runfiles is via the Runfiles class
+ // which contains all artifacts in the runfiles tree minus the MANIFEST file.
+ RunfilesArtifactValue runfilesArtifactValue = (RunfilesArtifactValue) value;
+ for (Pair<Artifact, FileArtifactValue> entry : runfilesArtifactValue.getFileArtifacts()) {
Artifact artifact = entry.first;
inputMap.put(artifact, entry.getSecond(), /*depOwner=*/ key);
if (artifact.isFileset()) {
@@ -91,7 +95,7 @@
consumer.accumulate(entry.getSecond());
}
}
- for (Pair<Artifact, TreeArtifactValue> entry : aggregatingValue.getTreeArtifacts()) {
+ for (Pair<Artifact, TreeArtifactValue> entry : runfilesArtifactValue.getTreeArtifacts()) {
expandTreeArtifactAndPopulateArtifactData(
entry.getFirst(),
Preconditions.checkNotNull(entry.getSecond()),
@@ -103,20 +107,7 @@
}
// We have to cache the "digest" of the aggregating value itself, because the action cache
// checker may want it.
- inputMap.put(key, aggregatingValue.getMetadata(), /*depOwner=*/ key);
- // While not obvious at all this code exists to ensure that we don't expand the
- // .runfiles/MANIFEST file into the inputs. The reason for that being that the MANIFEST
- // file contains absolute paths that don't work with remote execution.
- // Instead, the way the SpawnInputExpander expands runfiles is via the Runfiles class
- // which contains all artifacts in the runfiles tree minus the MANIFEST file.
- // TODO(buchgr): Clean this up and get rid of the RunfilesArtifactValue type.
- if (!(value instanceof RunfilesArtifactValue)) {
- ImmutableList.Builder<Artifact> expansionBuilder = ImmutableList.builder();
- for (Pair<Artifact, FileArtifactValue> pair : aggregatingValue.getFileArtifacts()) {
- expansionBuilder.add(Preconditions.checkNotNull(pair.getFirst()));
- }
- expandedArtifacts.put(key, expansionBuilder.build());
- }
+ inputMap.put(key, runfilesArtifactValue.getMetadata(), /*depOwner=*/ key);
} else if (value instanceof TreeArtifactValue) {
TreeArtifactValue treeArtifactValue = (TreeArtifactValue) value;
expandTreeArtifactAndPopulateArtifactData(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java
deleted file mode 100644
index e9978fb..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java
+++ /dev/null
@@ -1,85 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.skyframe;
-
-import com.google.common.base.MoreObjects;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.FileArtifactValue;
-import com.google.devtools.build.lib.util.Pair;
-import com.google.devtools.build.skyframe.SkyValue;
-import java.util.Collection;
-
-/** Value for aggregating artifacts, which must be expanded to a set of other artifacts. */
-class AggregatingArtifactValue implements SkyValue {
- private final ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs;
- private final ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs;
- private final FileArtifactValue metadata;
-
- AggregatingArtifactValue(
- ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs,
- ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs,
- FileArtifactValue metadata) {
- this.fileInputs = Preconditions.checkNotNull(fileInputs);
- this.directoryInputs = Preconditions.checkNotNull(directoryInputs);
- this.metadata = Preconditions.checkNotNull(metadata);
- }
-
- /** Returns the none tree artifacts that this artifact expands to, together with their data. */
- Collection<Pair<Artifact, FileArtifactValue>> getFileArtifacts() {
- return fileInputs;
- }
-
- /**
- * Returns the tree artifacts that this artifact expands to, together with the information
- * to which artifacts the tree artifacts expand to.
- */
- Collection<Pair<Artifact, TreeArtifactValue>> getTreeArtifacts() {
- return directoryInputs;
- }
-
- /** Returns the data of the artifact for this value, as computed by the action cache checker. */
- FileArtifactValue getMetadata() {
- return metadata;
- }
-
- @SuppressWarnings("EqualsGetClass") // RunfilesArtifactValue not equal to Aggregating.
- @Override
- public boolean equals(Object o) {
- if (this == o) {
- return true;
- }
- if (o == null || getClass() != o.getClass()) {
- return false;
- }
- AggregatingArtifactValue that = (AggregatingArtifactValue) o;
- return metadata.equals(that.metadata)
- && fileInputs.equals(that.fileInputs)
- && directoryInputs.equals(that.directoryInputs);
- }
-
- @Override
- public int hashCode() {
- return 31 * 31 * directoryInputs.hashCode() + 31 * fileInputs.hashCode() + metadata.hashCode();
- }
-
- @Override
- public String toString() {
- return MoreObjects.toStringHelper(this)
- .add("fileInputs", fileInputs)
- .add("directoryInputs", directoryInputs)
- .toString();
- }
-}
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 e9d01a91..fb64dae 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
@@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.devtools.build.lib.actions.MiddlemanType.RUNFILES_MIDDLEMAN;
+
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -32,7 +34,6 @@
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversalRoot;
import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode;
-import com.google.devtools.build.lib.actions.MiddlemanType;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
@@ -143,7 +144,7 @@
artifactDependencies);
FileArtifactValue individualMetadata = actionValue.getExistingFileArtifactValue(artifact);
if (isAggregatingValue(action)) {
- return createAggregatingValue(artifact, action, individualMetadata, env);
+ return createRunfilesArtifactValue(artifact, action, individualMetadata, env);
}
return individualMetadata;
}
@@ -340,7 +341,7 @@
}
@Nullable
- private static AggregatingArtifactValue createAggregatingValue(
+ private static RunfilesArtifactValue createRunfilesArtifactValue(
Artifact artifact,
ActionAnalysisMetadata action,
FileArtifactValue value,
@@ -367,9 +368,9 @@
} else if (inputValue instanceof TreeArtifactValue) {
directoryInputsBuilder.add(Pair.of(input, (TreeArtifactValue) inputValue));
} else {
- // We do not recurse in aggregating middleman artifacts.
+ // We do not recurse in middleman artifacts.
Preconditions.checkState(
- !(inputValue instanceof AggregatingArtifactValue),
+ !(inputValue instanceof RunfilesArtifactValue),
"%s %s %s",
artifact,
action,
@@ -386,24 +387,16 @@
Comparator.comparing(pair -> pair.getFirst().getExecPathString()),
directoryInputsBuilder.build());
- return (action.getActionType() == MiddlemanType.AGGREGATING_MIDDLEMAN)
- ? new AggregatingArtifactValue(fileInputs, directoryInputs, value)
- : new RunfilesArtifactValue(fileInputs, directoryInputs, value);
+ return new RunfilesArtifactValue(fileInputs, directoryInputs, value);
}
/**
* Returns whether this value needs to contain the data of all its inputs. Currently only tests to
- * see if the action is an aggregating or runfiles middleman action. However, may include Fileset
- * artifacts in the future.
+ * see if the action is a runfiles middleman action. However, may include Fileset artifacts in the
+ * future.
*/
private static boolean isAggregatingValue(ActionAnalysisMetadata action) {
- switch (action.getActionType()) {
- case AGGREGATING_MIDDLEMAN:
- case RUNFILES_MIDDLEMAN:
- return true;
- default:
- return false;
- }
+ return action.getActionType() == RUNFILES_MIDDLEMAN;
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 52c40f2..b6a9f23 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -421,7 +421,6 @@
srcs = ["ActionInputMapHelper.java"],
deps = [
":action_execution_value",
- ":aggregating_artifact_value",
":metadata_consumer_for_metrics",
":runfiles_artifact_value",
":tree_artifact_value",
@@ -756,8 +755,8 @@
)
java_library(
- name = "aggregating_artifact_value",
- srcs = ["AggregatingArtifactValue.java"],
+ name = "runfiles_artifact_value",
+ srcs = ["RunfilesArtifactValue.java"],
deps = [
":tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
@@ -789,7 +788,6 @@
deps = [
":action_execution_value",
":action_template_expansion_value",
- ":aggregating_artifact_value",
":coverage_report_value",
":detailed_exceptions",
":metadata_consumer_for_metrics",
@@ -2264,19 +2262,6 @@
)
java_library(
- name = "runfiles_artifact_value",
- srcs = ["RunfilesArtifactValue.java"],
- deps = [
- ":aggregating_artifact_value",
- ":tree_artifact_value",
- "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
- "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
- "//src/main/java/com/google/devtools/build/lib/util",
- "//third_party:guava",
- ],
-)
-
-java_library(
name = "sane_analysis_exception",
srcs = ["SaneAnalysisException.java"],
deps = [
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
index 5d78d76..afe3c78 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
@@ -1,4 +1,4 @@
-// Copyright 2018 The Bazel Authors. All rights reserved.
+// Copyright 2014 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -13,17 +13,73 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.skyframe.SkyValue;
+import java.util.Collection;
/** The artifacts behind a runfiles middleman. */
-final class RunfilesArtifactValue extends AggregatingArtifactValue {
+class RunfilesArtifactValue implements SkyValue {
+ private final ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs;
+ private final ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs;
+ private final FileArtifactValue metadata;
+
RunfilesArtifactValue(
ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs,
ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs,
FileArtifactValue metadata) {
- super(fileInputs, directoryInputs, metadata);
+ this.fileInputs = Preconditions.checkNotNull(fileInputs);
+ this.directoryInputs = Preconditions.checkNotNull(directoryInputs);
+ this.metadata = Preconditions.checkNotNull(metadata);
+ }
+
+ /** Returns the none tree artifacts that this artifact expands to, together with their data. */
+ Collection<Pair<Artifact, FileArtifactValue>> getFileArtifacts() {
+ return fileInputs;
+ }
+
+ /**
+ * Returns the tree artifacts that this artifact expands to, together with the information to
+ * which artifacts the tree artifacts expand to.
+ */
+ Collection<Pair<Artifact, TreeArtifactValue>> getTreeArtifacts() {
+ return directoryInputs;
+ }
+
+ /** Returns the data of the artifact for this value, as computed by the action cache checker. */
+ FileArtifactValue getMetadata() {
+ return metadata;
+ }
+
+ @SuppressWarnings("EqualsGetClass") // RunfilesArtifactValue not equal to Aggregating.
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ RunfilesArtifactValue that = (RunfilesArtifactValue) o;
+ return metadata.equals(that.metadata)
+ && fileInputs.equals(that.fileInputs)
+ && directoryInputs.equals(that.directoryInputs);
+ }
+
+ @Override
+ public int hashCode() {
+ return 31 * 31 * directoryInputs.hashCode() + 31 * fileInputs.hashCode() + metadata.hashCode();
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("fileInputs", fileInputs)
+ .add("directoryInputs", directoryInputs)
+ .toString();
}
}
