diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/actions')
20 files changed, 241 insertions, 204 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java index 968c7213da..00f918a17a 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java @@ -395,7 +395,7 @@ public abstract class AbstractAction implements Action, SkylarkValue { } @Override - public boolean shouldReportPathPrefixConflict(Action action) { + public boolean shouldReportPathPrefixConflict(ActionAnalysisMetadata action) { return this != action; } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java index 51514af534..85d22ef304 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Action.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java @@ -77,7 +77,7 @@ import javax.annotation.Nullable; * known set of fields is covered, not that all fields are covered), so carefully check all changes * to action subclasses. */ -public interface Action extends ActionMetadata, Describable { +public interface Action extends ActionExecutionMetadata, Describable { /** * Prepares for executing this action; called by the Builder prior to @@ -192,14 +192,6 @@ public interface Action extends ActionMetadata, Describable { @Nullable ResourceSet estimateResourceConsumption(Executor executor); /** - * @return true iff path prefix conflict (conflict where two actions generate - * two output artifacts with one of the artifact's path being the - * prefix for another) between this action and another action should - * be reported. - */ - boolean shouldReportPathPrefixConflict(Action action); - - /** * Returns true if the output should bypass output filtering. This is used for test actions. */ boolean showsOutputUnconditionally(); @@ -218,38 +210,4 @@ public interface Action extends ActionMetadata, Describable { * a different thread than the one this action is executed on. */ ExtraActionInfo.Builder getExtraActionInfo(); - - /** - * Returns the action type. Must not be {@code null}. - */ - MiddlemanType getActionType(); - - /** - * The action type. - */ - public enum MiddlemanType { - - /** A normal action. */ - NORMAL, - - /** A normal middleman, which just encapsulates a list of artifacts. */ - AGGREGATING_MIDDLEMAN, - - /** - * A middleman that enforces action ordering, is not validated by the dependency checker, but - * allows errors to be propagated. - */ - ERROR_PROPAGATING_MIDDLEMAN, - - /** - * A runfiles middleman, which is validated by the dependency checker, but is not expanded - * in blaze. Instead, the runfiles manifest is sent to remote execution client, which - * performs the expansion. - */ - RUNFILES_MIDDLEMAN; - - public boolean isMiddleman() { - return this != NORMAL; - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java index 08b501c4c6..a3e73b38cf 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java @@ -16,26 +16,11 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import javax.annotation.Nullable; - /** - * Side-effect free query methods for information about an {@link Action}. - * - * <p>This method is intended for use in situations when the intention is to pass around information - * about an action without allowing actual execution of the action. - * - * <p>The split between {@link Action} and {@link ActionMetadata} is somewhat arbitrary, other than - * that all methods with side effects must belong to the former. + * An Analysis phase interface for an {@link Action} or Action-like object, containing only + * side-effect-free query methods for information needed during action analysis. */ -public interface ActionMetadata { - /** - * If this executable can supply verbose information, returns a string that can be used as a - * progress message while this executable is running. A return value of {@code null} indicates no - * message should be reported. - */ - @Nullable - String getProgressMessage(); - +public interface ActionAnalysisMetadata { /** * Returns the owner of this executable if this executable can supply verbose information. This is * typically the rule that constructed it; see ActionOwner class comment for details. Returns @@ -158,63 +143,40 @@ public interface ActionMetadata { Iterable<Artifact> getMandatoryInputs(); /** - * <p>Returns a string encoding all of the significant behaviour of this - * Action that might affect the output. The general contract of - * <code>getKey</code> is this: if the work to be performed by the - * execution of this action changes, the key must change. </p> - * - * <p>As a corollary, the build system is free to omit the execution of an - * Action <code>a1</code> if (a) at some time in the past, it has already - * executed an Action <code>a0</code> with the same key as - * <code>a1</code>, and (b) the names and contents of the input files listed - * by <code>a1.getInputs()</code> are identical to the names and contents of - * the files listed by <code>a0.getInputs()</code>. </p> - * - * <p>Examples of changes that should affect the key are: - * <ul> - * <li>Changes to the BUILD file that materially affect the rule which gave - * rise to this Action.</li> - * - * <li>Changes to the command-line options, environment, or other global - * configuration resources which affect the behaviour of this kind of Action - * (other than changes to the names of the input/output files, which are - * handled externally).</li> - * - * <li>An upgrade to the build tools which changes the program logic of this - * kind of Action (typically this is achieved by incorporating a UUID into - * the key, which is changed each time the program logic of this action - * changes).</li> - * - * </ul></p> + * @return true iff path prefix conflict (conflict where two actions generate + * two output artifacts with one of the artifact's path being the + * prefix for another) between this action and another action should + * be reported. */ - String getKey(); + boolean shouldReportPathPrefixConflict(ActionAnalysisMetadata action); - /** - * Returns a human-readable description of the inputs to {@link #getKey()}. - * Used in the output from '--explain', and in error messages for - * '--check_up_to_date' and '--check_tests_up_to_date'. - * May return null, meaning no extra information is available. - * - * <p>If the return value is non-null, for consistency it should be a multiline message of the - * form: - * <pre> - * <var>Summary</var> - * <var>Fieldname</var>: <var>value</var> - * <var>Fieldname</var>: <var>value</var> - * ... - * </pre> - * where each line after the first one is intended two spaces, and where any fields that might - * contain newlines or other funny characters are escaped using {@link - * com.google.devtools.build.lib.shell.ShellUtils#shellEscape}. - * For example: - * <pre> - * Compiling foo.cc - * Command: /usr/bin/gcc - * Argument: '-c' - * Argument: foo.cc - * Argument: '-o' - * Argument: foo.o - * </pre> - */ - @Nullable String describeKey(); + /** Returns the action type. Must not be {@code null}. */ + MiddlemanType getActionType(); + + /** The action type. */ + public enum MiddlemanType { + + /** A normal action. */ + NORMAL, + + /** A normal middleman, which just encapsulates a list of artifacts. */ + AGGREGATING_MIDDLEMAN, + + /** + * A middleman that enforces action ordering, is not validated by the dependency checker, but + * allows errors to be propagated. + */ + ERROR_PROPAGATING_MIDDLEMAN, + + /** + * A runfiles middleman, which is validated by the dependency checker, but is not expanded + * in blaze. Instead, the runfiles manifest is sent to remote execution client, which + * performs the expansion. + */ + RUNFILES_MIDDLEMAN; + + public boolean isMiddleman() { + return this != NORMAL; + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java index 4fd06a949f..4a977890a7 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java @@ -16,7 +16,7 @@ package com.google.devtools.build.lib.actions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.actions.Action.MiddlemanType; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType; import com.google.devtools.build.lib.actions.cache.ActionCache; import com.google.devtools.build.lib.actions.cache.ActionCache.Entry; import com.google.devtools.build.lib.actions.cache.Digest; diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java new file mode 100644 index 0000000000..004a37a3f1 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java @@ -0,0 +1,94 @@ +// 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.actions; + +import javax.annotation.Nullable; + +/** + * An interface for an {@link Action}, containing only side-effect-free query methods for + * information needed during both action analysis and execution. + * + * <p>The split between {@link Action} and {@link ActionExecutionMetadata} is somewhat arbitrary, + * other than that all methods with side effects must belong to the former. + */ +public interface ActionExecutionMetadata extends ActionAnalysisMetadata { + /** + * If this executable can supply verbose information, returns a string that can be used as a + * progress message while this executable is running. A return value of {@code null} indicates no + * message should be reported. + */ + @Nullable + String getProgressMessage(); + + /** + * <p>Returns a string encoding all of the significant behaviour of this + * Action that might affect the output. The general contract of + * <code>getKey</code> is this: if the work to be performed by the + * execution of this action changes, the key must change. </p> + * + * <p>As a corollary, the build system is free to omit the execution of an + * Action <code>a1</code> if (a) at some time in the past, it has already + * executed an Action <code>a0</code> with the same key as + * <code>a1</code>, and (b) the names and contents of the input files listed + * by <code>a1.getInputs()</code> are identical to the names and contents of + * the files listed by <code>a0.getInputs()</code>. </p> + * + * <p>Examples of changes that should affect the key are: + * <ul> + * <li>Changes to the BUILD file that materially affect the rule which gave + * rise to this Action.</li> + * + * <li>Changes to the command-line options, environment, or other global + * configuration resources which affect the behaviour of this kind of Action + * (other than changes to the names of the input/output files, which are + * handled externally).</li> + * + * <li>An upgrade to the build tools which changes the program logic of this + * kind of Action (typically this is achieved by incorporating a UUID into + * the key, which is changed each time the program logic of this action + * changes).</li> + * + * </ul></p> + */ + String getKey(); + + /** + * Returns a human-readable description of the inputs to {@link #getKey()}. + * Used in the output from '--explain', and in error messages for + * '--check_up_to_date' and '--check_tests_up_to_date'. + * May return null, meaning no extra information is available. + * + * <p>If the return value is non-null, for consistency it should be a multiline message of the + * form: + * <pre> + * <var>Summary</var> + * <var>Fieldname</var>: <var>value</var> + * <var>Fieldname</var>: <var>value</var> + * ... + * </pre> + * where each line after the first one is intended two spaces, and where any fields that might + * contain newlines or other funny characters are escaped using {@link + * com.google.devtools.build.lib.shell.ShellUtils#shellEscape}. + * For example: + * <pre> + * Compiling foo.cc + * Command: /usr/bin/gcc + * Argument: '-c' + * Argument: foo.cc + * Argument: '-o' + * Argument: foo.o + * </pre> + */ + @Nullable String describeKey(); +} diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java index 7a2a410d85..fd56653208 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java @@ -57,7 +57,7 @@ public final class ActionExecutionStatusReporter { * The status of each action "in flight", i.e. whose ExecuteBuildAction.call() method is active. * Used for implementing the "still waiting" message. */ - private final Map<ActionMetadata, Pair<String, Long>> actionStatus = + private final Map<ActionExecutionMetadata, Pair<String, Long>> actionStatus = new ConcurrentHashMap<>(100); public static ActionExecutionStatusReporter create(EventHandler eventHandler) { @@ -98,7 +98,7 @@ public final class ActionExecutionStatusReporter { } } - private void setStatus(ActionMetadata action, String message) { + private void setStatus(ActionExecutionMetadata action, String message) { actionStatus.put(action, Pair.of(message, clock.nanoTime())); } @@ -116,14 +116,14 @@ public final class ActionExecutionStatusReporter { updateStatus(ActionStatusMessage.preparingStrategy(action)); } - public void setRunningFromBuildData(ActionMetadata action) { + public void setRunningFromBuildData(ActionExecutionMetadata action) { updateStatus(ActionStatusMessage.runningStrategy(action, "unknown")); } @Subscribe public void updateStatus(ActionStatusMessage statusMsg) { String message = statusMsg.getMessage(); - ActionMetadata action = statusMsg.getActionMetadata(); + ActionExecutionMetadata action = statusMsg.getActionMetadata(); setStatus(action, message); } @@ -132,9 +132,10 @@ public final class ActionExecutionStatusReporter { } private static void appendGroupStatus(StringBuilder buffer, - Map<ActionMetadata, Pair<String, Long>> statusMap, String status, long currentTime) { - List<Pair<Long, ActionMetadata>> actions = new ArrayList<>(); - for (Map.Entry<ActionMetadata, Pair<String, Long>> entry : statusMap.entrySet()) { + Map<ActionExecutionMetadata, Pair<String, Long>> statusMap, String status, + long currentTime) { + List<Pair<Long, ActionExecutionMetadata>> actions = new ArrayList<>(); + for (Map.Entry<ActionExecutionMetadata, Pair<String, Long>> entry : statusMap.entrySet()) { if (entry.getValue().first.equals(status)) { actions.add(Pair.of(entry.getValue().second, entry.getKey())); } @@ -142,12 +143,12 @@ public final class ActionExecutionStatusReporter { if (actions.isEmpty()) { return; } - Collections.sort(actions, Pair.<Long, ActionMetadata>compareByFirst()); + Collections.sort(actions, Pair.<Long, ActionExecutionMetadata>compareByFirst()); buffer.append("\n " + status + ":"); boolean truncateList = actions.size() > MAX_LINES; - for (Pair<Long, ActionMetadata> entry : actions.subList(0, + for (Pair<Long, ActionExecutionMetadata> entry : actions.subList(0, truncateList ? MAX_LINES - 1 : actions.size())) { String message = entry.second.getProgressMessage(); if (message == null) { @@ -167,7 +168,8 @@ public final class ActionExecutionStatusReporter { /** * Get message showing currently executing actions. */ - private String getExecutionStatusMessage(Map<ActionMetadata, Pair<String, Long>> statusMap) { + private String getExecutionStatusMessage( + Map<ActionExecutionMetadata, Pair<String, Long>> statusMap) { int count = statusMap.size(); StringBuilder s = count != 1 ? new StringBuilder("Still waiting for ").append(count).append(" jobs to complete:") @@ -177,7 +179,7 @@ public final class ActionExecutionStatusReporter { // A tree is just as fast as HashSet for small data sets. Set<String> statuses = new TreeSet<>(); - for (Map.Entry<ActionMetadata, Pair<String, Long>> entry : statusMap.entrySet()) { + for (Map.Entry<ActionExecutionMetadata, Pair<String, Long>> entry : statusMap.entrySet()) { statuses.add(entry.getValue().first); } @@ -192,7 +194,7 @@ public final class ActionExecutionStatusReporter { */ public void showCurrentlyExecutingActions(String progressPercentageMessage) { // Defensive copy to ensure thread safety. - Map<ActionMetadata, Pair<String, Long>> statusMap = new HashMap<>(actionStatus); + Map<ActionExecutionMetadata, Pair<String, Long>> statusMap = new HashMap<>(actionStatus); if (!statusMap.isEmpty()) { eventHandler.handle( Event.progress(progressPercentageMessage + getExecutionStatusMessage(statusMap))); @@ -205,13 +207,13 @@ public final class ActionExecutionStatusReporter { */ void warnAboutCurrentlyExecutingActions() { // Defensive copy to ensure thread safety. - Map<ActionMetadata, Pair<String, Long>> statusMap = new HashMap<>(actionStatus); + Map<ActionExecutionMetadata, Pair<String, Long>> statusMap = new HashMap<>(actionStatus); if (statusMap.isEmpty()) { // There are no tasks in the queue so there is nothing to report. eventHandler.handle(Event.warn("There are no active jobs - stopping the build")); return; } - Iterator<ActionMetadata> iterator = statusMap.keySet().iterator(); + Iterator<ActionExecutionMetadata> iterator = statusMap.keySet().iterator(); while (iterator.hasNext()) { // Filter out actions that are not executed yet. if (statusMap.get(iterator.next()).first.equals(ActionStatusMessage.PREPARING)) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionGraph.java b/src/main/java/com/google/devtools/build/lib/actions/ActionGraph.java index bd105c4fef..5b850d9022 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionGraph.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionGraph.java @@ -34,5 +34,5 @@ public interface ActionGraph { * are unknown. */ @Nullable - Action getGeneratingAction(Artifact artifact); + ActionAnalysisMetadata getGeneratingAction(Artifact artifact); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionGraphVisitor.java b/src/main/java/com/google/devtools/build/lib/actions/ActionGraphVisitor.java index 7e13e5d2a8..6d335a8f7e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionGraphVisitor.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionGraphVisitor.java @@ -17,7 +17,8 @@ package com.google.devtools.build.lib.actions; * An abstract visitor for the action graph. Specializes {@link BipartiteVisitor} for artifacts and * actions, and takes care of visiting the complete transitive closure. */ -public abstract class ActionGraphVisitor extends BipartiteVisitor<Action, Artifact> { +public abstract class ActionGraphVisitor extends + BipartiteVisitor<ActionAnalysisMetadata, Artifact> { private final ActionGraph actionGraph; @@ -37,7 +38,7 @@ public abstract class ActionGraphVisitor extends BipartiteVisitor<Action, Artifa * * @param action */ - protected void visitAction(Action action) {} + protected void visitAction(ActionAnalysisMetadata action) {} /** * Whether the given action should be visited. If this returns false, the visitation stops here, @@ -45,7 +46,7 @@ public abstract class ActionGraphVisitor extends BipartiteVisitor<Action, Artifa * * @param action */ - protected boolean shouldVisit(Action action) { + protected boolean shouldVisit(ActionAnalysisMetadata action) { return true; } @@ -67,14 +68,14 @@ public abstract class ActionGraphVisitor extends BipartiteVisitor<Action, Artifa } @Override protected void white(Artifact artifact) { - Action action = actionGraph.getGeneratingAction(artifact); + ActionAnalysisMetadata action = actionGraph.getGeneratingAction(artifact); visitArtifact(artifact); if (action != null && shouldVisit(action)) { visitBlackNode(action); } } - @Override protected void black(Action action) { + @Override protected void black(ActionAnalysisMetadata action) { visitAction(action); for (Artifact input : action.getInputs()) { if (shouldVisit(input)) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java index a4a85d2677..26bbb6d1af 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java @@ -49,7 +49,7 @@ public final class ActionInputHelper { // going away anyway. Preconditions.checkArgument(mm.isMiddlemanArtifact(), "%s is not a middleman artifact", mm); - Action middlemanAction = actionGraph.getGeneratingAction(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 diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java b/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java index 8755879758..de85ce9f80 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java @@ -23,7 +23,7 @@ public interface ActionRegistry { /** * This method notifies the registry new actions. */ - void registerAction(Action... actions); + void registerAction(ActionAnalysisMetadata... actions); /** * Get the (Label and BuildConfiguration) of the ConfiguredTarget ultimately responsible for all @@ -37,7 +37,7 @@ public interface ActionRegistry { @VisibleForTesting public static final ActionRegistry NOP = new ActionRegistry() { @Override - public void registerAction(Action... actions) {} + public void registerAction(ActionAnalysisMetadata... actions) {} @Override public ArtifactOwner getOwner() { diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionStatusMessage.java b/src/main/java/com/google/devtools/build/lib/actions/ActionStatusMessage.java index 144e862297..f0ef8d859f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionStatusMessage.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionStatusMessage.java @@ -20,16 +20,16 @@ package com.google.devtools.build.lib.actions; * used to notify any interested parties. */ public class ActionStatusMessage { - private final ActionMetadata action; + private final ActionExecutionMetadata action; private final String message; public static final String PREPARING = "Preparing"; - private ActionStatusMessage(ActionMetadata action, String message) { + private ActionStatusMessage(ActionExecutionMetadata action, String message) { this.action = action; this.message = message; } - public ActionMetadata getActionMetadata() { + public ActionExecutionMetadata getActionMetadata() { return action; } @@ -38,22 +38,23 @@ public class ActionStatusMessage { } /** Creates "Analyzing" status message. */ - public static ActionStatusMessage analysisStrategy(ActionMetadata action) { + public static ActionStatusMessage analysisStrategy(ActionExecutionMetadata action) { return new ActionStatusMessage(action, "Analyzing"); } /** Creates "Preparing" status message. */ - public static ActionStatusMessage preparingStrategy(ActionMetadata action) { + public static ActionStatusMessage preparingStrategy(ActionExecutionMetadata action) { return new ActionStatusMessage(action, PREPARING); } /** Creates "Scheduling" status message. */ - public static ActionStatusMessage schedulingStrategy(ActionMetadata action) { + public static ActionStatusMessage schedulingStrategy(ActionExecutionMetadata action) { return new ActionStatusMessage(action, "Scheduling"); } /** Creates "Running (strategy)" status message. */ - public static ActionStatusMessage runningStrategy(ActionMetadata action, String strategy) { + public static ActionStatusMessage runningStrategy( + ActionExecutionMetadata action, String strategy) { return new ActionStatusMessage(action, String.format("Running (%s)", strategy)); } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Actions.java b/src/main/java/com/google/devtools/build/lib/actions/Actions.java index 3878fcd148..9d74e3b30d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Actions.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Actions.java @@ -51,19 +51,27 @@ public final class Actions { * <p>This method implements an equivalence relationship across actions, based on the action * class, the key, and the list of inputs and outputs. */ - public static boolean canBeShared(Action a, Action b) { + public static boolean canBeShared(ActionAnalysisMetadata a, ActionAnalysisMetadata b) { if (!a.getMnemonic().equals(b.getMnemonic())) { return false; } - if (!a.getKey().equals(b.getKey())) { + + // Non-Actions cannot be shared. + if (!(a instanceof Action) || !(b instanceof Action)) { + return false; + } + + Action actionA = (Action) a; + Action actionB = (Action) b; + if (!actionA.getKey().equals(actionB.getKey())) { return false; } // Don't bother to check input and output counts first; the expected result for these tests is // to always be true (i.e., that this method returns true). - if (!Iterables.elementsEqual(a.getMandatoryInputs(), b.getMandatoryInputs())) { + if (!Iterables.elementsEqual(actionA.getMandatoryInputs(), actionB.getMandatoryInputs())) { return false; } - if (!Iterables.elementsEqual(a.getOutputs(), b.getOutputs())) { + if (!Iterables.elementsEqual(actionA.getOutputs(), actionB.getOutputs())) { return false; } return true; @@ -80,18 +88,19 @@ public final class Actions { * @throws ActionConflictException iff there are two unshareable actions generating the same * output */ - public static Map<Artifact, Action> filterSharedActionsAndThrowActionConflict( - Iterable<Action> actions) throws ActionConflictException { + public static Map<Artifact, ActionAnalysisMetadata> filterSharedActionsAndThrowActionConflict( + Iterable<? extends ActionAnalysisMetadata> actions) throws ActionConflictException { return Actions.maybeFilterSharedActionsAndThrowIfConflict( actions, /*allowSharedAction=*/ true); } - private static Map<Artifact, Action> maybeFilterSharedActionsAndThrowIfConflict( - Iterable<Action> actions, boolean allowSharedAction) throws ActionConflictException { - Map<Artifact, Action> generatingActions = new HashMap<>(); - for (Action action : actions) { + private static Map<Artifact, ActionAnalysisMetadata> maybeFilterSharedActionsAndThrowIfConflict( + Iterable<? extends ActionAnalysisMetadata> actions, boolean allowSharedAction) + throws ActionConflictException { + Map<Artifact, ActionAnalysisMetadata> generatingActions = new HashMap<>(); + for (ActionAnalysisMetadata action : actions) { for (Artifact artifact : action.getOutputs()) { - Action previousAction = generatingActions.put(artifact, action); + ActionAnalysisMetadata previousAction = generatingActions.put(artifact, action); if (previousAction != null && previousAction != action) { if (!allowSharedAction || !Actions.canBeShared(previousAction, action)) { throw new ActionConflictException(artifact, previousAction, action); @@ -112,16 +121,17 @@ public final class Actions { * @return A map between actions that generated the conflicting artifacts and their associated * {@link ArtifactPrefixConflictException}. */ - public static Map<Action, ArtifactPrefixConflictException> findArtifactPrefixConflicts( - ActionGraph actionGraph, SortedMap<PathFragment, Artifact> artifactPathMap) { + public static Map<ActionAnalysisMetadata, ArtifactPrefixConflictException> + findArtifactPrefixConflicts(ActionGraph actionGraph, + SortedMap<PathFragment, Artifact> artifactPathMap) { // No actions in graph -- currently happens only in tests. Special-cased because .next() call // below is unconditional. if (artifactPathMap.isEmpty()) { - return ImmutableMap.<Action, ArtifactPrefixConflictException>of(); + return ImmutableMap.<ActionAnalysisMetadata, ArtifactPrefixConflictException>of(); } // Keep deterministic ordering of bad actions. - Map<Action, ArtifactPrefixConflictException> badActions = new LinkedHashMap(); + Map<ActionAnalysisMetadata, ArtifactPrefixConflictException> badActions = new LinkedHashMap(); Iterator<PathFragment> iter = artifactPathMap.keySet().iterator(); // Report an error for every derived artifact which is a prefix of another. @@ -140,9 +150,9 @@ public final class Actions { if (pathJ.startsWith(pathI)) { // prefix conflict. Artifact artifactI = Preconditions.checkNotNull(artifactPathMap.get(pathI), pathI); Artifact artifactJ = Preconditions.checkNotNull(artifactPathMap.get(pathJ), pathJ); - Action actionI = + ActionAnalysisMetadata actionI = Preconditions.checkNotNull(actionGraph.getGeneratingAction(artifactI), artifactI); - Action actionJ = + ActionAnalysisMetadata actionJ = Preconditions.checkNotNull(actionGraph.getGeneratingAction(artifactJ), artifactJ); if (actionI.shouldReportPathPrefixConflict(actionJ)) { ArtifactPrefixConflictException exception = new ArtifactPrefixConflictException(pathI, diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java index f28683b451..95faf512bd 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java @@ -22,7 +22,7 @@ import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; -import com.google.devtools.build.lib.actions.Action.MiddlemanType; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.shell.ShellUtils; diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java index 8e34389185..2e3a69de29 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java @@ -44,7 +44,7 @@ public class BaseSpawn implements Spawn { private final ImmutableMap<PathFragment, Artifact> runfilesManifests; private final ImmutableSet<PathFragment> optionalOutputFiles; private final RunfilesSupplier runfilesSupplier; - private final ActionMetadata action; + private final ActionExecutionMetadata action; private final ResourceSet localResources; // TODO(bazel-team): When we migrate ActionSpawn to use this constructor decide on and enforce @@ -57,7 +57,7 @@ public class BaseSpawn implements Spawn { Map<String, String> executionInfo, Map<PathFragment, Artifact> runfilesManifests, RunfilesSupplier runfilesSupplier, - ActionMetadata action, + ActionExecutionMetadata action, ResourceSet localResources, Collection<PathFragment> optionalOutputFiles) { this.arguments = ImmutableList.copyOf(arguments); @@ -78,7 +78,7 @@ public class BaseSpawn implements Spawn { Map<String, String> environment, Map<String, String> executionInfo, RunfilesSupplier runfilesSupplier, - ActionMetadata action, + ActionExecutionMetadata action, ResourceSet localResources) { this( arguments, @@ -99,7 +99,7 @@ public class BaseSpawn implements Spawn { Map<String, String> environment, Map<String, String> executionInfo, Map<PathFragment, Artifact> runfilesManifests, - ActionMetadata action, + ActionExecutionMetadata action, ResourceSet localResources) { this( arguments, @@ -118,7 +118,7 @@ public class BaseSpawn implements Spawn { public BaseSpawn(List<String> arguments, Map<String, String> environment, Map<String, String> executionInfo, - ActionMetadata action, + ActionExecutionMetadata action, ResourceSet localResources) { this( arguments, @@ -134,7 +134,7 @@ public class BaseSpawn implements Spawn { Map<String, String> environment, Map<String, String> executionInfo, RunfilesSupplier runfilesSupplier, - ActionMetadata action, + ActionExecutionMetadata action, ResourceSet localResources, Collection<PathFragment> optionalOutputFiles) { this( @@ -258,7 +258,7 @@ public class BaseSpawn implements Spawn { } @Override - public ActionMetadata getResourceOwner() { + public ActionExecutionMetadata getResourceOwner() { return action; } @@ -291,7 +291,8 @@ public class BaseSpawn implements Spawn { * A local spawn requiring zero resources. */ public static class Local extends BaseSpawn { - public Local(List<String> arguments, Map<String, String> environment, ActionMetadata action) { + public Local(List<String> arguments, Map<String, String> environment, + ActionExecutionMetadata action) { super(arguments, environment, ImmutableMap.<String, String>of("local", ""), action, ResourceSet.ZERO); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java index acb9cd64d4..d72a686679 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java @@ -100,7 +100,7 @@ public class DelegateSpawn implements Spawn { } @Override - public ActionMetadata getResourceOwner() { + public ActionExecutionMetadata getResourceOwner() { return spawn.getResourceOwner(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/MapBasedActionGraph.java b/src/main/java/com/google/devtools/build/lib/actions/MapBasedActionGraph.java index 2535a2e7ef..54d3411419 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MapBasedActionGraph.java +++ b/src/main/java/com/google/devtools/build/lib/actions/MapBasedActionGraph.java @@ -24,19 +24,19 @@ import javax.annotation.concurrent.ThreadSafe; */ @ThreadSafe public final class MapBasedActionGraph implements MutableActionGraph { - private final ConcurrentMultimapWithHeadElement<Artifact, Action> generatingActionMap = - new ConcurrentMultimapWithHeadElement<>(); + private final ConcurrentMultimapWithHeadElement<Artifact, ActionAnalysisMetadata> + generatingActionMap = new ConcurrentMultimapWithHeadElement<>(); @Override @Nullable - public Action getGeneratingAction(Artifact artifact) { + public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) { return generatingActionMap.get(artifact); } @Override - public void registerAction(Action action) throws ActionConflictException { + public void registerAction(ActionAnalysisMetadata action) throws ActionConflictException { for (Artifact artifact : action.getOutputs()) { - Action previousAction = generatingActionMap.putAndGet(artifact, action); + ActionAnalysisMetadata previousAction = generatingActionMap.putAndGet(artifact, action); if (previousAction != null && previousAction != action && !Actions.canBeShared(action, previousAction)) { generatingActionMap.remove(artifact, action); @@ -46,10 +46,10 @@ public final class MapBasedActionGraph implements MutableActionGraph { } @Override - public void unregisterAction(Action action) { + public void unregisterAction(ActionAnalysisMetadata action) { for (Artifact artifact : action.getOutputs()) { generatingActionMap.remove(artifact, action); - Action otherAction = generatingActionMap.get(artifact); + ActionAnalysisMetadata otherAction = generatingActionMap.get(artifact); Preconditions.checkState(otherAction == null || (otherAction != action && Actions.canBeShared(action, otherAction)), "%s %s", action, otherAction); diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java index 24c436c190..404b0de10d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java +++ b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.actions.Action.MiddlemanType; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.util.Pair; diff --git a/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java b/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java index d5da897c43..50b3fa7886 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java +++ b/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java @@ -42,7 +42,7 @@ public interface MutableActionGraph extends ActionGraph { * then both conflicts are detected; if either A or C is registered first, then only one conflict * is detected. */ - void registerAction(Action action) throws ActionConflictException; + void registerAction(ActionAnalysisMetadata action) throws ActionConflictException; /** * Removes an action from this action graph if it is present. @@ -50,7 +50,7 @@ public interface MutableActionGraph extends ActionGraph { * <p>Throws {@link IllegalStateException} if one of the outputs of the action is in fact * generated by a different {@link Action} instance (even if they are sharable). */ - void unregisterAction(Action action); + void unregisterAction(ActionAnalysisMetadata action); /** * Clear the action graph. @@ -65,13 +65,13 @@ public interface MutableActionGraph extends ActionGraph { public static final class ActionConflictException extends Exception { private final Artifact artifact; - private final Action previousAction; - private final Action attemptedAction; + private final ActionAnalysisMetadata previousAction; + private final ActionAnalysisMetadata attemptedAction; private static final int MAX_DIFF_ARTIFACTS_TO_REPORT = 5; - public ActionConflictException(Artifact artifact, Action previousAction, - Action attemptedAction) { + public ActionConflictException(Artifact artifact, ActionAnalysisMetadata previousAction, + ActionAnalysisMetadata attemptedAction) { super( String.format( "for %s, previous action: %s, attempted action: %s", @@ -145,7 +145,7 @@ public interface MutableActionGraph extends ActionGraph { } // See also Actions.canBeShared() - private String suffix(Action a, Action b) { + private String suffix(ActionAnalysisMetadata a, ActionAnalysisMetadata b) { // Note: the error message reveals to users the names of intermediate files that are not // documented in the BUILD language. This error-reporting logic is rather elaborate but it // does help to diagnose some tricky situations. @@ -162,7 +162,15 @@ public interface MutableActionGraph extends ActionGraph { addStringDetail(sb, "Configuration", aNull ? null : aOwner.getConfigurationChecksum(), bNull ? null : bOwner.getConfigurationChecksum()); addStringDetail(sb, "Mnemonic", a.getMnemonic(), b.getMnemonic()); - addStringDetail(sb, "Progress message", a.getProgressMessage(), b.getProgressMessage()); + + if ((a instanceof ActionExecutionMetadata) && (b instanceof ActionExecutionMetadata)) { + addStringDetail( + sb, + "Progress message", + ((ActionExecutionMetadata) a).getProgressMessage(), + ((ActionExecutionMetadata) b).getProgressMessage()); + } + addStringDetail( sb, "PrimaryInput", diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java index c263b2d05f..3ebaee0036 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java @@ -61,16 +61,16 @@ import java.util.concurrent.CountDownLatch; public class ResourceManager { /** - * A handle returned by {@link #acquireResources(ActionMetadata, ResourceSet)} that must be closed - * in order to free the resources again. + * A handle returned by {@link #acquireResources(ActionExecutionMetadata, ResourceSet)} that must + * be closed in order to free the resources again. */ public static class ResourceHandle implements AutoCloseable { final ResourceManager rm; - final ActionMetadata actionMetadata; + final ActionExecutionMetadata actionMetadata; final ResourceSet resourceSet; - public ResourceHandle( - ResourceManager rm, ActionMetadata actionMetadata, ResourceSet resources) { + public ResourceHandle(ResourceManager rm, ActionExecutionMetadata actionMetadata, + ResourceSet resources) { this.rm = rm; this.actionMetadata = actionMetadata; this.resourceSet = resources; @@ -200,7 +200,7 @@ public class ResourceManager { * Acquires requested resource set. Will block if resource is not available. * NB! This method must be thread-safe! */ - public ResourceHandle acquireResources(ActionMetadata owner, ResourceSet resources) + public ResourceHandle acquireResources(ActionExecutionMetadata owner, ResourceSet resources) throws InterruptedException { Preconditions.checkNotNull(resources); AutoProfiler p = profiled(owner, ProfilerTask.ACTION_LOCK); @@ -228,7 +228,7 @@ public class ResourceManager { * Acquires the given resources if available immediately. Does not block. * @return true iff the given resources were locked (all or nothing). */ - public boolean tryAcquire(ActionMetadata owner, ResourceSet resources) { + public boolean tryAcquire(ActionExecutionMetadata owner, ResourceSet resources) { boolean acquired = false; synchronized (this) { if (areResourcesAvailable(resources)) { @@ -279,14 +279,14 @@ public class ResourceManager { this.eventBus = null; } - private void waiting(ActionMetadata owner) { + private void waiting(ActionExecutionMetadata owner) { if (eventBus != null) { // Null only in tests. eventBus.post(ActionStatusMessage.schedulingStrategy(owner)); } } - private void acquired(ActionMetadata owner) { + private void acquired(ActionExecutionMetadata owner) { if (eventBus != null) { // Null only in tests. eventBus.post(ActionStatusMessage.runningStrategy(owner, "unknown")); @@ -298,7 +298,7 @@ public class ResourceManager { * * <p>NB! This method must be thread-safe! */ - public void releaseResources(ActionMetadata owner, ResourceSet resources) { + public void releaseResources(ActionExecutionMetadata owner, ResourceSet resources) { boolean isConflict = false; AutoProfiler p = profiled(owner, ProfilerTask.ACTION_RELEASE); try { diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index b4ed5c79fa..3e24fdca19 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -129,7 +129,7 @@ public interface Spawn { /** * Returns the resource owner for local fallback. */ - ActionMetadata getResourceOwner(); + ActionExecutionMetadata getResourceOwner(); /** * Returns the amount of resources needed for local fallback. |