Skip to content

Commit

Permalink
Move ActionInputHelper to a separate target.
Browse files Browse the repository at this point in the history
Move `ActionInputHelper` out of `//main/java/com/google/devtools/build/lib/actions` so that it can be used without taking dependency on it.

Move `ActionInputHelper::actionGraphArtifactExpander` to a test-only helper library.

PiperOrigin-RevId: 381029208
  • Loading branch information
alexjski authored and copybara-github committed Jun 23, 2021
1 parent a617b5a commit 2ace97f
Show file tree
Hide file tree
Showing 22 changed files with 115 additions and 36 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ filegroup(
"//src/main/java/com/google/devtools/build/lib/ssd:srcs",
"//src/main/java/com/google/devtools/build/lib/standalone:srcs",
"//src/main/java/com/google/devtools/build/lib/supplier:srcs",
"//src/main/java/com/google/devtools/build/lib/testing/actions:srcs",
"//src/main/java/com/google/devtools/build/lib/testing/common:srcs",
"//src/main/java/com/google/devtools/build/lib/unix:srcs",
"//src/main/java/com/google/devtools/build/lib/unsafe:srcs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,45 +14,19 @@

package com.google.devtools.build.lib.actions;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Functions;
import com.google.common.base.Preconditions;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

/** Helper utility to create ActionInput instances. */
public final class ActionInputHelper {
private ActionInputHelper() {}

@VisibleForTesting
public static ArtifactExpander actionGraphArtifactExpander(ActionGraph actionGraph) {
return new ArtifactExpander() {
@Override
public void expand(Artifact mm, Collection<? super Artifact> output) {
// Skyframe is stricter in that it checks that "mm" is a input of the action, because
// it cannot expand arbitrary middlemen without access to a global action graph.
// We could check this constraint here too, but it seems unnecessary. This code is
// going away anyway.
Preconditions.checkArgument(mm.isMiddlemanArtifact(), "%s is not a middleman artifact", mm);
ActionAnalysisMetadata middlemanAction = actionGraph.getGeneratingAction(mm);
Preconditions.checkState(middlemanAction != null, mm);
// TODO(bazel-team): Consider expanding recursively or throwing an exception here.
// Most likely, this code will cause silent errors if we ever have a middleman that
// contains a middleman.
if (middlemanAction.getActionType() == MiddlemanType.AGGREGATING_MIDDLEMAN) {
Artifact.addNonMiddlemanArtifacts(
middlemanAction.getInputs().toList(), output, Functions.<Artifact>identity());
}
}
};
}

/**
* Most ActionInputs are created and never used again. On the off chance that one is, however, we
* implement equality via path comparison. Since file caches are keyed by ActionInput, equality
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1413,12 +1413,13 @@ public static void addExecPaths(Iterable<Artifact> artifacts, Collection<String>
}

/**
* Converts a collection of artifacts into the outputs computed by
* outputFormatter and adds them to a given collection. Middleman artifacts
* are ignored.
* Converts a collection of artifacts into the outputs computed by outputFormatter and adds them
* to a given collection. Middleman artifacts are ignored.
*/
static <E> void addNonMiddlemanArtifacts(Iterable<Artifact> artifacts,
Collection<? super E> output, Function<? super Artifact, E> outputFormatter) {
public static <E> void addNonMiddlemanArtifacts(
Iterable<Artifact> artifacts,
Collection<? super E> output,
Function<? super Artifact, E> outputFormatter) {
for (Artifact artifact : artifacts) {
if (MIDDLEMAN_FILTER.apply(artifact)) {
output.add(outputFormatter.apply(artifact));
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ java_library(
],
exclude = [
"ActionInput.java",
"ActionInputHelper.java",
"ActionLookupData.java",
"ActionLookupKey.java",
"AnalysisGraphStatsEvent.java",
Expand Down Expand Up @@ -136,6 +137,18 @@ java_library(
],
)

java_library(
name = "action_input_helper",
srcs = ["ActionInputHelper.java"],
deps = [
":artifacts",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:guava",
],
)

java_library(
name = "total_and_configured_target_only_metric",
srcs = ["TotalAndConfiguredTargetOnlyMetric.java"],
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib:build-request-options",
"//src/main/java/com/google/devtools/build/lib:runtime/build_event_streamer_utils",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:commandline_item",
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/google/devtools/build/lib/exec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ java_library(
srcs = ["SingleBuildFileCache.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/vfs",
Expand Down Expand Up @@ -232,6 +233,7 @@ java_library(
srcs = ["SpawnInputExpander.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/actions:fileset_output_symlink",
Expand Down Expand Up @@ -336,6 +338,7 @@ java_library(
":standalone_test_result",
":test_policy",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ java_library(
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ java_library(
":Retrier",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/actions:fileset_output_symlink",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ java_library(
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/concurrent",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ java_library(
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/profiler",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2021 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.testing.actions;

import com.google.common.base.Functions;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.MiddlemanType;

/** Helper utility to create {@link ArtifactExpander} instances. */
public final class ArtifactExpanderHelper {
private ArtifactExpanderHelper() {}

public static ArtifactExpander actionGraphArtifactExpander(ActionGraph actionGraph) {
return (mm, output) -> {
// Skyframe is stricter in that it checks that "mm" is a input of the action, because
// it cannot expand arbitrary middlemen without access to a global action graph.
// We could check this constraint here too, but it seems unnecessary. This code is
// going away anyway.
Preconditions.checkArgument(mm.isMiddlemanArtifact(), "%s is not a middleman artifact", mm);
ActionAnalysisMetadata middlemanAction = actionGraph.getGeneratingAction(mm);
Preconditions.checkState(middlemanAction != null, mm);
// TODO(bazel-team): Consider expanding recursively or throwing an exception here.
// Most likely, this code will cause silent errors if we ever have a middleman that
// contains a middleman.
if (middlemanAction.getActionType() == MiddlemanType.AGGREGATING_MIDDLEMAN) {
Artifact.addNonMiddlemanArtifacts(
middlemanAction.getInputs().toList(), output, Functions.identity());
}
};
}
}
26 changes: 26 additions & 0 deletions src/main/java/com/google/devtools/build/lib/testing/actions/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
load("@rules_java//java:defs.bzl", "java_library")

package(
default_testonly = True,
default_visibility = ["//src:__subpackages__"],
)

licenses(["notice"])

filegroup(
name = "srcs",
testonly = False,
srcs = glob(["**"]),
visibility = ["//src:__subpackages__"],
)

java_library(
name = "artifact_expander_helper",
srcs = ["ArtifactExpanderHelper.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:middleman_type",
"//third_party:guava",
],
)
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/worker/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ java_library(
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy",
"//src/main/java/com/google/devtools/build/lib/exec:bin_tools",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationDepsUtils;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import com.google.devtools.build.lib.testing.actions.ArtifactExpanderHelper;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.FileTypeSet;
Expand Down Expand Up @@ -226,8 +227,8 @@ public void testAddExpandedArtifacts() throws Exception {
List<Artifact> expanded = new ArrayList<>();
MutableActionGraph actionGraph = new MapBasedActionGraph(actionKeyContext);
List<Artifact> original = getFooBarArtifacts(actionGraph, true);
Artifact.addExpandedArtifacts(original, expanded,
ActionInputHelper.actionGraphArtifactExpander(actionGraph));
Artifact.addExpandedArtifacts(
original, expanded, ArtifactExpanderHelper.actionGraphArtifactExpander(actionGraph));

List<Artifact> manuallyExpanded = new ArrayList<>();
for (Artifact artifact : original) {
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ java_library(
),
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
"//src/main/java/com/google/devtools/build/lib/actions:artifact_owner",
Expand All @@ -52,6 +53,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils:depsutils",
"//src/main/java/com/google/devtools/build/lib/testing/actions:artifact_expander_helper",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util:string",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.google.devtools.build.lib.actions.ActionExecutionContext.LostInputsCheck;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupData;
Expand Down Expand Up @@ -79,6 +78,7 @@
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.testing.actions.ArtifactExpanderHelper;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.ResourceUsage;
Expand Down Expand Up @@ -174,7 +174,7 @@ public static ActionExecutionContext createContext(
/*topLevelFilesets=*/ ImmutableMap.of(),
actionGraph == null
? (artifact, output) -> {}
: ActionInputHelper.actionGraphArtifactExpander(actionGraph),
: ArtifactExpanderHelper.actionGraphArtifactExpander(actionGraph),
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
NestedSetExpander.DEFAULT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/testing/actions:artifact_expander_helper",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:crash_failure_details",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/exec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ java_library(
]),
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/actions:fileset_output_symlink",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ java_library(
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/actions:fileset_output_symlink",
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ java_test(
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ java_library(
),
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//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/clock",
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
"//src/main/java/com/google/devtools/build/lib/actions:package_roots",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:artifact_owner",
Expand Down

0 comments on commit 2ace97f

Please sign in to comment.