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 2536826c2c6006..3d837acf0e8fd5 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 @@ -125,8 +125,8 @@ public final boolean inputsKnown() { /** * {@inheritDoc} * - *

Should be overridden along with {@link #discoverInputs}, {@link #inputsDiscovered}, and - * {@link #setInputsDiscovered} by actions that do input discovery. + *

Should be overridden along with {@link #discoverInputs}, {@link #inputsDiscovered}, {@link + * #setInputsDiscovered} and {@link #getOriginalInputs} by actions that do input discovery. */ @Override public boolean discoversInputs() { @@ -137,8 +137,7 @@ public boolean discoversInputs() { @Nullable public NestedSet discoverInputs(ActionExecutionContext actionExecutionContext) throws ActionExecutionException, InterruptedException { - throw new IllegalStateException("discoverInputs cannot be called for " + this.prettyPrint() - + " since it does not discover inputs"); + throw new IllegalStateException("Not an input-discovering action: " + this); } @Override @@ -147,12 +146,9 @@ public final void resetDiscoveredInputs() { if (!inputsKnown()) { return; } - NestedSet originalInputs = getOriginalInputs(); - if (originalInputs != null) { - synchronized (this) { - inputs = originalInputs; - setInputsDiscovered(false); - } + synchronized (this) { + inputs = getOriginalInputs(); + setInputsDiscovered(false); } } @@ -168,7 +164,7 @@ public final void resetDiscoveredInputs() { @ForOverride @GuardedBy("this") protected boolean inputsDiscovered() { - throw new IllegalStateException("Must be overridden by input-discovering actions: " + this); + throw new IllegalStateException("Must be overridden by input-discovering action: " + this); } /** @@ -178,26 +174,19 @@ protected boolean inputsDiscovered() { @ForOverride @GuardedBy("this") protected void setInputsDiscovered(boolean inputsDiscovered) { - throw new IllegalStateException("Must be overridden by input-discovering actions: " + this); + throw new IllegalStateException("Must be overridden by input-discovering action: " + this); } - /** - * Returns this action's original inputs, prior to {@linkplain #discoverInputs input - * discovery}. - * - *

Input-discovering actions which are able to reconstitute their original inputs may override - * this, allowing for memory savings. - */ - @Nullable - @ForOverride - protected NestedSet getOriginalInputs() { - return null; + @Override + public NestedSet getOriginalInputs() { + checkState(!discoversInputs(), "Must be overridden by input-discovering action"); + return getInputs(); } @Override public NestedSet getAllowedDerivedInputs() { throw new IllegalStateException( - "Method must be overridden for actions that may have unknown inputs."); + "Must be overridden for action that may have unknown inputs: " + this); } @Override @@ -217,7 +206,7 @@ public NestedSet getSchedulingDependencies() { */ @Override public synchronized void updateInputs(NestedSet inputs) { - checkState(discoversInputs(), "Can't update inputs unless discovering: %s %s", this, inputs); + checkState(discoversInputs(), "Not an input-discovering action: %s", this); this.inputs = inputs; setInputsDiscovered(true); } @@ -295,9 +284,7 @@ public Artifact getOriginalPrimaryInput() { // The default behavior is to return the first input artifact of the original input list (before // input discovery). // Call through the method, not the field, because it may be overridden. - NestedSet originalInputs = getOriginalInputs(); - NestedSet inputs = originalInputs == null ? getInputs() : originalInputs; - return Iterables.getFirst(inputs.toList(), null); + return Iterables.getFirst(getOriginalInputs().toList(), null); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java index 0fd11465525322..de39c328468178 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java @@ -128,12 +128,19 @@ String getKey(ActionKeyContext actionKeyContext, @Nullable ArtifactExpander arti /** * Returns the input Artifacts that this Action depends upon. May be empty. * - *

For actions that do input discovery or input pruning, a different result may be returned - * before and after action execution, because input discovery may add additional artifacts from - * {@link #getSchedulingDependencies}, and input pruning may remove them. + *

For actions that do input discovery, a different result may be returned before and after + * action execution, because input discovery may add or remove inputs. The original input set may + * be retrieved from {@link ActionExecutionMetadata#getOriginalInputs}. */ NestedSet getInputs(); + /** + * Returns this action's original inputs prior to input discovery. + * + *

Unlike {@link #getInputs}, the same result is returned before and after action execution. + */ + NestedSet getOriginalInputs(); + /** * Returns the input Artifacts that must be built before the action can be executed, but are not * dependencies of the action in the action cache. @@ -167,7 +174,7 @@ String getKey(ActionKeyContext actionKeyContext, @Nullable ArtifactExpander arti * other files as well. For example C(++) compilation may perform include file header scanning. * This needs to be mirrored by the extra_action rule. Called by {@link * com.google.devtools.build.lib.analysis.extra.ExtraAction} at execution time for actions that - * return true for {link #discoversInputs()}. + * return true for {link ActionExecutionMetadata#discoversInputs}. * * @param actionExecutionContext Services in the scope of the action, like the Out/Err streams. * @throws ActionExecutionException only when code called from this method throws that exception. 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 index eb9cc3d0e68897..606491accd4451 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java @@ -79,7 +79,8 @@ default String getProgressMessage(RepositoryMapping mainRepositoryMapping) { String describeKey(); /** - * Returns true iff the {@link #getInputs} set is known to be complete. + * Returns true iff the {@link #getInputs} set has been updated taking input discovery into + * account. * *

For most actions, this always returns true. For actions which {@linkplain #discoversInputs * discover inputs} (e.g. C++ compilation), inputs are dynamically discovered from the previous diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java index 90c91a3518de7a..dac19db7813b39 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java @@ -206,6 +206,11 @@ public NestedSet getInputs() { return allInputs; } + @Override + public NestedSet getOriginalInputs() { + return getInputs(); + } + @Override public NestedSet getSchedulingDependencies() { return NestedSetBuilder.emptySet(Order.STABLE_ORDER); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java index 7288670b9ff7c7..c6df48534c8e04 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java @@ -256,7 +256,7 @@ public boolean discoversInputs() { } @Override - protected NestedSet getOriginalInputs() { + public NestedSet getOriginalInputs() { return allStarlarkActionInputs; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java index b1c4c6391a0736..d2a3f128926fbb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java @@ -136,6 +136,11 @@ public NestedSet discoverInputs(ActionExecutionContext actionExecution Order.STABLE_ORDER, Sets.difference(getInputs().toSet(), oldInputs.toSet())); } + @Override + public NestedSet getOriginalInputs() { + return shadowedAction.getOriginalInputs(); + } + @Override public NestedSet getSchedulingDependencies() { return shadowedAction.getSchedulingDependencies(); diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AqueryProcessor.java b/src/main/java/com/google/devtools/build/lib/buildtool/AqueryProcessor.java index d938f7ae112285..be2787e860ca38 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/AqueryProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/AqueryProcessor.java @@ -87,7 +87,7 @@ public BlazeCommandResult dumpActionGraphFromSkyframe(CommandEnvironment env) { new ActionGraphDump( aqueryOptions.includeCommandline, aqueryOptions.includeArtifacts, - aqueryOptions.includeSchedulingDependencies, + aqueryOptions.includePrunedInputs, actionFilters, aqueryOptions.includeParamFiles, aqueryOptions.includeFileWriteContents, diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 79c78797b9ec0e..e78715b40620d2 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -589,7 +589,7 @@ private void dumpSkyframeStateAfterBuild( new ActionGraphDump( /* includeActionCmdLine= */ false, /* includeArtifacts= */ true, - /* includeSchedulingDependencies= */ true, + /* includePrunedInputs= */ true, /* actionFilters= */ null, /* includeParamFiles= */ false, /* includeFileWriteContents= */ false, diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java index 190ee1c2e4f754..c963770e5827a6 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java @@ -202,6 +202,11 @@ public NestedSet getInputs() { return actionExecutionMetadata.getInputs(); } + @Override + public NestedSet getOriginalInputs() { + return actionExecutionMetadata.getInputs(); + } + @Override public NestedSet getSchedulingDependencies() { throw new UnsupportedOperationException(); diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java index b328d234b429b1..7f608e1f0b1b26 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java @@ -75,7 +75,7 @@ public class ActionGraphProtoOutputFormatterCallback extends AqueryThreadsafeCal new ActionGraphDump( options.includeCommandline, options.includeArtifacts, - options.includeSchedulingDependencies, + options.includePrunedInputs, this.actionFilters, options.includeParamFiles, options.includeFileWriteContents, diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java index 69a74f0c2eb160..bf7c6919f529fe 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java @@ -82,7 +82,7 @@ public void processOutput(Iterable partialResult) } private void processAction(ActionAnalysisMetadata action) throws InterruptedException { - if (!AqueryUtils.matchesAqueryFilters(action, actionFilters)) { + if (!AqueryUtils.matchesAqueryFilters(action, actionFilters, options.includePrunedInputs)) { return; } diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java index 836e5849e76634..1e557686996817 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.query2.aquery; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.devtools.build.lib.query2.aquery.AqueryUtils.getActionInputs; import static com.google.devtools.build.lib.util.StringUtil.reencodeInternalToExternal; import static java.nio.charset.StandardCharsets.UTF_8; @@ -37,6 +38,7 @@ import com.google.devtools.build.lib.analysis.starlark.UnresolvedSymlinkAction; import com.google.devtools.build.lib.buildeventstream.BuildEvent; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.LabelPrinter; @@ -122,7 +124,7 @@ private void writeAction(ActionAnalysisMetadata action, PrintStream printStream) getParamFileNameToContentMap().put(paramFileName, fileContent); } - if (!AqueryUtils.matchesAqueryFilters(action, actionFilters)) { + if (!AqueryUtils.matchesAqueryFilters(action, actionFilters, options.includePrunedInputs)) { return; } @@ -199,26 +201,17 @@ private void writeAction(ActionAnalysisMetadata action, PrintStream printStream) } if (options.includeArtifacts) { + NestedSet inputs = getActionInputs(action, options.includePrunedInputs); + stringBuilder .append(" Inputs: [") .append( - action.getInputs().toList().stream() + inputs.toList().stream() .map(input -> escapeBytestringUtf8(input.getExecPathString())) .sorted() .collect(Collectors.joining(", "))) .append("]\n"); - if (options.includeSchedulingDependencies && !action.getSchedulingDependencies().isEmpty()) { - stringBuilder - .append(" SchedulingDependencies: [") - .append( - action.getSchedulingDependencies().toList().stream() - .map(input -> escapeBytestringUtf8(input.getExecPathString())) - .sorted() - .collect(Collectors.joining(", "))) - .append("]\n"); - } - stringBuilder .append(" Outputs: [") .append( @@ -279,7 +272,7 @@ private void writeAction(ActionAnalysisMetadata action, PrintStream printStream) if (options.includeParamFiles) { // Assumption: if an Action takes a param file as an input, it will be used // to provide params to the command. - for (Artifact input : action.getInputs().toList()) { + for (Artifact input : getActionInputs(action, options.includePrunedInputs).toList()) { String inputFileName = input.getExecPathString(); if (getParamFileNameToContentMap().containsKey(inputFileName)) { stringBuilder diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java index 845477c0a639b9..699ee950d2635f 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java @@ -43,20 +43,19 @@ public class AqueryOptions extends CommonQueryOptions { defaultValue = "true", documentationCategory = OptionDocumentationCategory.QUERY, effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, - help = - "Includes names of the action inputs and outputs in the output " + "(potentially large).") + help = "Includes names of the action inputs and outputs in the output (potentially large).") public boolean includeArtifacts; @Option( - name = "include_scheduling_dependencies", - defaultValue = "false", + name = "include_pruned_inputs", + defaultValue = "true", documentationCategory = OptionDocumentationCategory.QUERY, effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, help = - "Includes names of the scheduling dependencies of actions " - + "(potentially large). " - + "Only takes effect if --include_artifacts is also set.") - public boolean includeSchedulingDependencies; + "Includes action inputs that were pruned during action execution. Only affects actions" + + " that discover inputs and have been executed in a previous invocation. Only takes" + + " effect if --include_artifacts is also set.") + public boolean includePrunedInputs; @Option( name = "include_param_files", diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryUtils.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryUtils.java index 0eba0fd1da05db..abfc2f04d0942b 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryUtils.java +++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryUtils.java @@ -19,26 +19,59 @@ import com.google.common.collect.Streams; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; 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.analysis.actions.TemplateExpansionAction; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import java.io.IOException; /** Utility class for Aquery */ public class AqueryUtils { + private AqueryUtils() {} + + /** + * Returns the set of action inputs according to the --include_pruned_inputs flag. + * + *

This may differ from {@link ActionAnalysisMetadata#getInputs} for actions that discover + * inputs. + * + * @param action the analysis metadata of an action + * @param includePrunedInputs the value of the --include_pruned_inputs flag + */ + public static NestedSet getActionInputs( + ActionAnalysisMetadata action, boolean includePrunedInputs) { + if (includePrunedInputs + || (action instanceof ActionExecutionMetadata actionExecutionMetadata + && !actionExecutionMetadata.inputsKnown())) { + // getInputs() is potentially missing inputs that will be added by discovery (if the action + // hasn't yet executed) and inputs that have been removed by discovery (if the action has + // already executed). Instead, assemble the inputs from getOriginalInputs() and + // getSchedulingDependencies(), which also include those added or removed by discovery. + return NestedSetBuilder.stableOrder() + .addTransitive(action.getOriginalInputs()) + .addTransitive(action.getSchedulingDependencies()) + .build(); + } + return action.getInputs(); + } + /** * Return true if the given {@code action} matches the filters specified in {@code actionFilters}. * * @param action the analysis metadata of an action * @param actionFilters the filters parsed from the query expression + * @param includePrunedInputs the value of the --include_pruned_inputs flag * @return whether the action matches the filtering patterns */ public static boolean matchesAqueryFilters( - ActionAnalysisMetadata action, AqueryActionFilter actionFilters) { - NestedSet inputs = action.getInputs(); + ActionAnalysisMetadata action, + AqueryActionFilter actionFilters, + boolean includePrunedInputs) { + NestedSet inputs = getActionInputs(action, includePrunedInputs); Iterable outputs = action.getOutputs(); String mnemonic = action.getMnemonic(); @@ -49,7 +82,7 @@ public static boolean matchesAqueryFilters( } if (actionFilters.hasFilterForFunction(INPUTS)) { - Boolean containsFile = + boolean containsFile = inputs.toList().stream() .anyMatch( artifact -> @@ -62,7 +95,7 @@ public static boolean matchesAqueryFilters( } if (actionFilters.hasFilterForFunction(OUTPUTS)) { - Boolean containsFile = + boolean containsFile = Streams.stream(outputs) .anyMatch( artifact -> diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 533b2d96c225b7..8ba94f289cbaeb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -616,7 +616,7 @@ public NestedSet discoverInputs(ActionExecutionContext actionExecution } @Override - protected final NestedSet getOriginalInputs() { + public final NestedSet getOriginalInputs() { return mandatoryInputs; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java index 518d9d17e94b47..6968a2651ce303 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java @@ -346,6 +346,11 @@ public NestedSet getInputs() { .build(); } + @Override + public NestedSet getOriginalInputs() { + return getInputs(); + } + @Override public NestedSet getSchedulingDependencies() { return NestedSetBuilder.emptySet(Order.STABLE_ORDER); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java index 5546f87bb8591b..2f50b6a6e62f98 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java @@ -223,7 +223,7 @@ public NestedSet discoverInputs(ActionExecutionContext actionExecution } @Override - protected NestedSet getOriginalInputs() { + public NestedSet getOriginalInputs() { return mandatoryInputs; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTemplate.java index 01021d282ce32b..6c40c222a35981 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTemplate.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTemplate.java @@ -408,6 +408,11 @@ public NestedSet getInputs() { return allInputs; } + @Override + public NestedSet getOriginalInputs() { + return allInputs; + } + @Override public ImmutableSet getOutputs() { ImmutableSet.Builder builder = ImmutableSet.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java index 50fbeb1304871e..9449bbc0d7cf30 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe.actiongraph.v2; +import static com.google.devtools.build.lib.query2.aquery.AqueryUtils.getActionInputs; + import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -68,7 +70,7 @@ public class ActionGraphDump { @Nullable private final AqueryActionFilter actionFilters; private final boolean includeActionCmdLine; private final boolean includeArtifacts; - private final boolean includeSchedulingDependencies; + private final boolean includePrunedInputs; private final boolean includeParamFiles; private final boolean includeFileWriteContents; private final AqueryOutputHandler aqueryOutputHandler; @@ -79,7 +81,7 @@ public class ActionGraphDump { public ActionGraphDump( boolean includeActionCmdLine, boolean includeArtifacts, - boolean includeSchedulingDependencies, + boolean includePrunedInputs, AqueryActionFilter actionFilters, boolean includeParamFiles, boolean includeFileWriteContents, @@ -89,7 +91,7 @@ public ActionGraphDump( /* actionGraphTargets= */ ImmutableList.of("..."), includeActionCmdLine, includeArtifacts, - includeSchedulingDependencies, + includePrunedInputs, actionFilters, includeParamFiles, includeFileWriteContents, @@ -101,7 +103,7 @@ public ActionGraphDump( List actionGraphTargets, boolean includeActionCmdLine, boolean includeArtifacts, - boolean includeSchedulingDependencies, + boolean includePrunedInputs, AqueryActionFilter actionFilters, boolean includeParamFiles, boolean includeFileWriteContents, @@ -110,7 +112,7 @@ public ActionGraphDump( this.actionGraphTargets = ImmutableSet.copyOf(actionGraphTargets); this.includeActionCmdLine = includeActionCmdLine; this.includeArtifacts = includeArtifacts; - this.includeSchedulingDependencies = includeSchedulingDependencies; + this.includePrunedInputs = includePrunedInputs; this.actionFilters = actionFilters; this.includeParamFiles = includeParamFiles; this.includeFileWriteContents = includeFileWriteContents; @@ -150,7 +152,8 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM getParamFileNameToContentMap().put(paramFileExecPath, fileContent); } - if (actionFilters != null && !AqueryUtils.matchesAqueryFilters(action, actionFilters)) { + if (actionFilters != null + && !AqueryUtils.matchesAqueryFilters(action, actionFilters, includePrunedInputs)) { return; } @@ -217,7 +220,7 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM if (includeParamFiles) { // Assumption: if an Action takes a params file as an input, it will be used // to provide params to the command. - for (Artifact input : action.getInputs().toList()) { + for (Artifact input : getActionInputs(action, includePrunedInputs).toList()) { String inputFileExecPath = input.getExecPathString(); if (getParamFileNameToContentMap().containsKey(inputFileExecPath)) { AnalysisProtosV2.ParamFile paramFile = @@ -257,19 +260,13 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM } if (includeArtifacts) { - // Store inputs - NestedSet inputs = action.getInputs(); + // Store inputs. + NestedSet inputs = getActionInputs(action, includePrunedInputs); if (!inputs.isEmpty()) { actionBuilder.addInputDepSetIds(knownNestedSets.dataToIdAndStreamOutputProto(inputs)); } - NestedSet schedulingDependencies = action.getSchedulingDependencies(); - if (includeSchedulingDependencies && !schedulingDependencies.isEmpty()) { - actionBuilder.addInputDepSetIds( - knownNestedSets.dataToIdAndStreamOutputProto(schedulingDependencies)); - } - - // store outputs + // Store outputs. for (Artifact artifact : action.getOutputs()) { actionBuilder.addOutputIds(knownArtifacts.dataToIdAndStreamOutputProto(artifact)); } diff --git a/src/main/protobuf/analysis_v2.proto b/src/main/protobuf/analysis_v2.proto index 83f420359a5a8e..89ad91c4101ff5 100644 --- a/src/main/protobuf/analysis_v2.proto +++ b/src/main/protobuf/analysis_v2.proto @@ -80,14 +80,10 @@ message Action { repeated KeyValuePair environment_variables = 7; // The set of input dep sets that the action depends upon. If the action does - // input discovery, the contents of this set might change during execution. + // input discovery and `--include_pruned_outputs` is disabled, the contents of + // this set might differ before and after execution. repeated uint32 input_dep_set_ids = 8; - // The set of scheduling dependency dep sets the action depends on. - // Only set of both --include_artifacts and --include_scheduling_dependencies - // is set to true. - repeated uint32 scheduling_dep_dep_set_ids = 20; - // The list of Artifact IDs that represent the output files that this action // will generate. repeated uint32 output_ids = 9; @@ -129,6 +125,8 @@ message Action { // If FileWrite actions should make their output executable. // (ctx.actions.write(is_executable=True)) bool is_executable = 19; + + reserved 20; } // Represents a single target (without configuration information) that is diff --git a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java index dcb5c4ec8b184f..0d2db28283f27e 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java @@ -860,6 +860,11 @@ public NestedSet getInputs() { throw new IllegalStateException(); } + @Override + public NestedSet getOriginalInputs() { + throw new IllegalStateException(); + } + @Override public NestedSet getSchedulingDependencies() { throw new IllegalStateException(); diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java b/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java index 38144fec8b88d3..dd4a5f9c3fa6c0 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java @@ -105,6 +105,11 @@ protected void setInputsDiscovered(boolean inputsDiscovered) { this.inputsDiscovered = inputsDiscovered; } + @Override + public NestedSet getOriginalInputs() { + return mandatoryInputs; + } + @Override public NestedSet getAllowedDerivedInputs() { return NestedSetBuilder.wrap(Order.STABLE_ORDER, optionalInputs); diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java b/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java index d11baababdc3d3..2f52090552d434 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java @@ -136,6 +136,11 @@ public NestedSet getInputs() { throw new UnsupportedOperationException(); } + @Override + public NestedSet getOriginalInputs() { + throw new UnsupportedOperationException(); + } + @Override public NestedSet getSchedulingDependencies() { throw new UnsupportedOperationException(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java index 990bb9738e2f5f..db3df56ec78cdc 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java @@ -506,6 +506,11 @@ public NestedSet getInputs() { return NestedSetBuilder.create(Order.STABLE_ORDER, inputTreeArtifact); } + @Override + public NestedSet getOriginalInputs() { + return getInputs(); + } + @Override public NestedSet getSchedulingDependencies() { return NestedSetBuilder.emptySet(Order.STABLE_ORDER); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java index 0ce35b40b60189..ca07e206da9dbe 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java @@ -1458,6 +1458,11 @@ public NestedSet getInputs() { return NestedSetBuilder.create(Order.STABLE_ORDER, inputArtifact); } + @Override + public NestedSet getOriginalInputs() { + return getInputs(); + } + @Override public NestedSet getSchedulingDependencies() { return NestedSetBuilder.emptySet(Order.STABLE_ORDER); diff --git a/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java b/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java index 1a19428793587d..0aef5076f60ca6 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java @@ -117,6 +117,11 @@ public NestedSet getInputs() { return NestedSetBuilder.emptySet(Order.STABLE_ORDER); } + @Override + public NestedSet getOriginalInputs() { + return NestedSetBuilder.emptySet(Order.STABLE_ORDER); + } + @Override public NestedSet getSchedulingDependencies() { return NestedSetBuilder.emptySet(Order.STABLE_ORDER); diff --git a/src/test/shell/integration/aquery_test.sh b/src/test/shell/integration/aquery_test.sh index 5a0d7901463c7d..38a31f51a684c2 100755 --- a/src/test/shell/integration/aquery_test.sh +++ b/src/test/shell/integration/aquery_test.sh @@ -243,33 +243,261 @@ EOF assert_not_contains "Outputs: \[" output } -function test_aquery_include_scheduling_dependencies() { +function test_prunable_headers() { local pkg="${FUNCNAME[0]}" mkdir -p "$pkg" || fail "mkdir -p $pkg" cat > "$pkg/BUILD" <<'EOF' -cc_binary(name="b", srcs=["b.cc"], deps=[":l"]) -cc_library(name="l", hdrs=["library_header.h"]) +cc_binary( + name = "foo", + srcs = ["foo.cc"], + deps = [":bar"], +) +cc_library( + name = "bar", + hdrs = ["used.h", "useless.h"], +) +EOF + cat > "$pkg/foo.cc" <<'EOF' +#include "used.h" +int main() { used(); return 0; } +EOF + cat > "$pkg/used.h" <<'EOF' +inline void used() {} +EOF + cat > "$pkg/useless.h" <<'EOF' +inline void useless() {} EOF - bazel aquery \ - --include_artifacts \ - --include_scheduling_dependencies \ - "mnemonic(CppCompile,//$pkg:b)" > output 2> "$TEST_log" \ + # Test --include_pruned_inputs before build. + # Expect used.h and useless.h to be considered inputs. + + bazel aquery "mnemonic(CppCompile,//$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_contains "Inputs:.*used.h" output + assert_contains "Inputs:.*useless.h" output + + bazel aquery "inputs(.*used.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery "inputs(.*useless.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + # Test --include_pruned_inputs after build. + # Expect used.h and useless.h to be considered inputs. + + bazel build "//$pkg:foo" || fail "Expected success" + + bazel aquery "mnemonic(CppCompile,//$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_contains "Inputs:.*used.h" output + assert_contains "Inputs:.*useless.h" output + + bazel aquery "inputs(.*used.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery "inputs(.*useless.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + if [[ "${PRODUCT_NAME}" == "bazel" ]]; then + # Include scanning not supported. + return 0 + fi + + bazel clean + + # Test --noinclude_pruned_inputs before build. + # Expect used.h and useless.h to be considered inputs. + + bazel aquery --noinclude_pruned_inputs \ + "mnemonic(CppCompile,//$pkg:foo)" > output 2> "$TEST_log" \ || fail "Expected success" cat output >> "$TEST_log" - assert_contains "SchedulingDependencies:.*library_header.h" output + assert_contains "Inputs:.*used.h" output + assert_contains "Inputs:.*useless.h" output - bazel aquery \ - --include_artifacts \ - --include_scheduling_dependencies \ - --output=jsonproto \ - "mnemonic(CppCompile,//$pkg:b)" > output 2> "$TEST_log" \ + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*used.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*useless.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + # Test --noinclude_pruned_inputs after build. + # Expect used.h to be considered an input, but not useless.h. + + bazel build "//$pkg:foo" || fail "Expected success" + + bazel aquery --noinclude_pruned_inputs \ + "mnemonic(CppCompile,//$pkg:foo)" > output 2> "$TEST_log" \ || fail "Expected success" cat output >> "$TEST_log" - assert_contains "library_header.h" output + + assert_contains "Inputs:.*used.h" output + assert_not_contains "Inputs:.*useless.h" output + + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*used.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*useless.h, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_empty_file output } +function test_unused_inputs() { + local pkg="${FUNCNAME[0]}" + mkdir -p "$pkg" || fail "mkdir -p $pkg" + cat > "$pkg/defs.bzl" < "$pkg/BUILD" <<'EOF' +load(":defs.bzl", "foo") +sh_binary( + name = "tool", + srcs = ["tool.sh"], +) +foo( + name = "foo", + used = "used.txt", + unused = "useless.txt", + out = "out.txt", +) +EOF + cat > "$pkg/tool.sh" <<'EOF' +#!/bin/bash +echo "$1" > "$2" +EOF + chmod +x "$pkg/tool.sh" + touch "$pkg/used.txt" "$pkg/useless.txt" + + # Test --include_pruned_inputs before build. + # Expect used.txt and useless.txt to be considered inputs. + + bazel aquery "mnemonic(Action,//$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_contains "Inputs:.*used.txt" output + assert_contains "Inputs:.*useless.txt" output + + bazel aquery "inputs(.*used.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery "inputs(.*useless.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + # Test --include_pruned_inputs after build. + # Expect used.txt and useless.txt to be considered inputs. + + bazel build "//$pkg:foo" || fail "Expected success" + + bazel aquery "mnemonic(Action,//$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_contains "Inputs:.*used.txt" output + assert_contains "Inputs:.*useless.txt" output + + bazel aquery "inputs(.*used.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery "inputs(.*useless.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel clean + + # Test --noinclude_pruned_inputs before build. + # Expect used.txt and useless.txt to be considered inputs. + + bazel aquery --noinclude_pruned_inputs \ + "mnemonic(Action,//$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + cat output >> "$TEST_log" + + assert_contains "Inputs:.*used.txt" output + assert_contains "Inputs:.*useless.txt" output + + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*used.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*useless.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + # Test --noinclude_pruned_inputs after build. + # Expect used.h to be considered an input, but not useless.h. + + bazel build "//$pkg:foo" || fail "Expected success" + + bazel aquery --noinclude_pruned_inputs \ + "mnemonic(Action,//$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + cat output >> "$TEST_log" + + assert_contains "Inputs:.*used.txt" output + assert_not_contains "Inputs:.*useless.txt" output + + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*used.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_nonempty_file output + + bazel aquery --noinclude_pruned_inputs \ + "inputs(.*useless.txt, //$pkg:foo)" > output 2> "$TEST_log" \ + || fail "Expected success" + + assert_empty_file output +} function test_aquery_starlark_env() { local pkg="${FUNCNAME[0]}" diff --git a/src/test/shell/unittest.bash b/src/test/shell/unittest.bash index 9cdc011a4bae2b..2b92e634e474d0 100644 --- a/src/test/shell/unittest.bash +++ b/src/test/shell/unittest.bash @@ -514,6 +514,36 @@ function assert_contains_n() { return 1 } +# Usage: assert_empty_file [error-message] +# Asserts that the file exists and is empty. On failure copies the file to +# undeclared outputs, prints the specified (optional) error message, and returns +# non-zero. +function assert_empty_file() { + local file=$1 + local message=${2:-"Expected '$file' to exist and be empty"} + if [[ -f "$file" && ! -s "$file" ]]; then + return 0 + fi + + fail "$message" $(__copy_to_undeclared_outputs "$1") + return 1 +} + +# Usage: assert_nonempty_file [error-message] +# Asserts that the file exists and is nonempty. On failure copies the file to +# undeclared outputs, prints the specified (optional) error message, and returns +# non-zero. +function assert_nonempty_file() { + local file=$1 + local message=${2:-"Expected '$file' to exist and be nonempty"} + if [[ -f "$file" && -s "$file" ]]; then + return 0 + fi + + fail "$message" $(__copy_to_undeclared_outputs "$1") + return 1 +} + # Copies $1 to undeclared outputs with a unique path and logs the mapping # between the original file and the new file. function __copy_to_undeclared_outputs() {