diff options
author | Michael Thvedt <mthvedt@google.com> | 2016-02-09 00:57:46 +0000 |
---|---|---|
committer | Dmitry Lomov <dslomov@google.com> | 2016-02-09 12:20:47 +0000 |
commit | 434e68ebae77b4fd89c05ac676f20406e1c5b368 (patch) | |
tree | 3510106cecabdbc77a30adbbb973b13ca742231f /src | |
parent | e3b1cb765a04c858a87ca7c7b0ecfa63d55be269 (diff) |
Rename MiddlemanExpander to ArtifactExpander, and refactor it to yield ArtifactFiles.
--
MOS_MIGRATED_REVID=114166208
Diffstat (limited to 'src')
12 files changed, 114 insertions, 87 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java index 66b6c4861a..dce5819ed3 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.actions; -import com.google.devtools.build.lib.actions.Artifact.MiddlemanExpander; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.io.FileOutErr; @@ -32,32 +32,32 @@ public class ActionExecutionContext { private final ActionInputFileCache actionInputFileCache; private final MetadataHandler metadataHandler; private final FileOutErr fileOutErr; - private final MiddlemanExpander middlemanExpander; + private final ArtifactExpander artifactExpander; @Nullable private final Environment env; private ActionExecutionContext(Executor executor, ActionInputFileCache actionInputFileCache, MetadataHandler metadataHandler, FileOutErr fileOutErr, - @Nullable MiddlemanExpander middlemanExpander, + @Nullable ArtifactExpander artifactExpander, @Nullable SkyFunction.Environment env) { this.actionInputFileCache = actionInputFileCache; this.metadataHandler = metadataHandler; this.fileOutErr = fileOutErr; this.executor = executor; - this.middlemanExpander = middlemanExpander; + this.artifactExpander = artifactExpander; this.env = env; } public ActionExecutionContext(Executor executor, ActionInputFileCache actionInputFileCache, - MetadataHandler metadataHandler, FileOutErr fileOutErr, MiddlemanExpander middlemanExpander) { - this(executor, actionInputFileCache, metadataHandler, fileOutErr, middlemanExpander, null); + MetadataHandler metadataHandler, FileOutErr fileOutErr, ArtifactExpander artifactExpander) { + this(executor, actionInputFileCache, metadataHandler, fileOutErr, artifactExpander, null); } public static ActionExecutionContext normal(Executor executor, ActionInputFileCache actionInputFileCache, MetadataHandler metadataHandler, - FileOutErr fileOutErr, MiddlemanExpander middlemanExpander) { + FileOutErr fileOutErr, ArtifactExpander artifactExpander) { return new ActionExecutionContext(executor, actionInputFileCache, metadataHandler, fileOutErr, - middlemanExpander, null); + artifactExpander, null); } public static ActionExecutionContext forInputDiscovery(Executor executor, @@ -79,8 +79,8 @@ public class ActionExecutionContext { return executor; } - public MiddlemanExpander getMiddlemanExpander() { - return middlemanExpander; + public ArtifactExpander getArtifactExpander() { + return artifactExpander; } /** @@ -104,6 +104,6 @@ public class ActionExecutionContext { */ public ActionExecutionContext withFileOutErr(FileOutErr fileOutErr) { return new ActionExecutionContext(executor, actionInputFileCache, metadataHandler, fileOutErr, - middlemanExpander, env); + artifactExpander, env); } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java index ed45880125..e87d72e2ee 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java @@ -25,5 +25,5 @@ import java.util.Map; */ public interface ActionExecutionContextFactory { ActionExecutionContext getContext(ActionInputFileCache graphFileCache, - MetadataHandler metadataHandler, Map<Artifact, Collection<Artifact>> expandedInputMiddlemen); + MetadataHandler metadataHandler, Map<Artifact, Collection<ArtifactFile>> expandedInputs); } 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 723c9fbd37..8bceb9acd3 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 @@ -19,6 +19,7 @@ import com.google.common.base.Function; import com.google.common.base.Functions; import com.google.common.collect.Collections2; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; @@ -34,11 +35,11 @@ public final class ActionInputHelper { } @VisibleForTesting - public static Artifact.MiddlemanExpander actionGraphMiddlemanExpander( + public static ArtifactExpander actionGraphArtifactExpander( final ActionGraph actionGraph) { - return new Artifact.MiddlemanExpander() { + return new ArtifactExpander() { @Override - public void expand(Artifact mm, Collection<? super Artifact> output) { + public void expand(Artifact mm, Collection<? super ArtifactFile> 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 @@ -157,13 +158,27 @@ public final class ActionInputHelper { }); } + /** Returns an Collection of ArtifactFiles with the given parent and parent relative paths. */ + public static Collection<ArtifactFile> asArtifactFiles( + final Artifact parent, Collection<? extends PathFragment> parentRelativePaths) { + Preconditions.checkState(parent.isTreeArtifact(), + "Given parent %s must be a TreeArtifact", parent); + return Collections2.transform(parentRelativePaths, + new Function<PathFragment, ArtifactFile>() { + @Override + public ArtifactFile apply(PathFragment pathFragment) { + return artifactFile(parent, pathFragment); + } + }); + } + /** * Expands middleman artifacts in a sequence of {@link ActionInput}s. * * <p>Non-middleman artifacts are returned untouched. */ - public static List<ActionInput> expandMiddlemen(Iterable<? extends ActionInput> inputs, - Artifact.MiddlemanExpander middlemanExpander) { + public static List<ActionInput> expandArtifacts(Iterable<? extends ActionInput> inputs, + ArtifactExpander artifactExpander) { List<ActionInput> result = new ArrayList<>(); List<Artifact> containedArtifacts = new ArrayList<>(); @@ -174,7 +189,7 @@ public final class ActionInputHelper { } containedArtifacts.add((Artifact) input); } - Artifact.addExpandedArtifacts(containedArtifacts, result, middlemanExpander); + Artifact.addExpandedArtifacts(containedArtifacts, result, artifactExpander); return result; } 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 23e2a4cb46..2a700023ce 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 @@ -76,7 +76,7 @@ import javax.annotation.Nullable; * This is a legacy facility and should not be used by any new rule implementations. * In particular, the file system cache integrity checks fail for directories. * <li>An 'aggregating middleman' special Artifact, which may be expanded using a - * {@link MiddlemanExpander} at Action execution time. This is used by a handful of rules to save + * {@link ArtifactExpander} at Action execution time. This is used by a handful of rules to save * memory. * <li>A 'constant metadata' special Artifact. These represent real files, changes to which are * ignored by the build system. They are useful for files which change frequently but do not affect @@ -116,14 +116,15 @@ public class Artifact implements FileType.HasFilename, ArtifactFile, SkylarkValu }; /** An object that can expand middleman artifacts. */ - public interface MiddlemanExpander { + public interface ArtifactExpander { /** - * Expands the middleman artifact "mm", and populates "output" with the result. + * Expands the given artifact, and populates "output" with the result. * - * <p>{@code mm.isMiddlemanArtifact()} must be true. Only aggregating middlemen are expanded. + * <p>{@code artifact.isMiddlemanArtifact() || artifact.isTreeArtifact()} must be true. + * Only aggregating middlemen and tree artifacts are expanded. */ - void expand(Artifact mm, Collection<? super Artifact> output); + void expand(Artifact artifact, Collection<? super ArtifactFile> output); } public static final ImmutableList<Artifact> NO_ARTIFACTS = ImmutableList.of(); @@ -639,8 +640,8 @@ public class Artifact implements FileType.HasFilename, ArtifactFile, SkylarkValu * {@link MiddlemanType#AGGREGATING_MIDDLEMAN} middleman actions expanded once. */ public static void addExpandedArtifacts(Iterable<Artifact> artifacts, - Collection<? super Artifact> output, MiddlemanExpander middlemanExpander) { - addExpandedArtifacts(artifacts, output, Functions.<Artifact>identity(), middlemanExpander); + Collection<? super ArtifactFile> output, ArtifactExpander artifactExpander) { + addExpandedArtifacts(artifacts, output, Functions.<ArtifactFile>identity(), artifactExpander); } /** @@ -652,9 +653,9 @@ public class Artifact implements FileType.HasFilename, ArtifactFile, SkylarkValu @VisibleForTesting public static void addExpandedExecPathStrings(Iterable<Artifact> artifacts, Collection<String> output, - MiddlemanExpander middlemanExpander) { + ArtifactExpander artifactExpander) { addExpandedArtifacts(artifacts, output, ActionInputHelper.EXEC_PATH_STRING_FORMATTER, - middlemanExpander); + artifactExpander); } /** @@ -664,8 +665,8 @@ public class Artifact implements FileType.HasFilename, ArtifactFile, SkylarkValu * once. */ public static void addExpandedExecPaths(Iterable<Artifact> artifacts, - Collection<PathFragment> output, MiddlemanExpander middlemanExpander) { - addExpandedArtifacts(artifacts, output, EXEC_PATH_FORMATTER, middlemanExpander); + Collection<PathFragment> output, ArtifactExpander artifactExpander) { + addExpandedArtifacts(artifacts, output, EXEC_PATH_FORMATTER, artifactExpander); } /** @@ -673,27 +674,29 @@ public class Artifact implements FileType.HasFilename, ArtifactFile, SkylarkValu * outputFormatter and adds them to a given collection. Middleman artifacts * are expanded once. */ - private static <E> void addExpandedArtifacts(Iterable<Artifact> artifacts, + private static <E> void addExpandedArtifacts(Iterable<? extends Artifact> artifacts, Collection<? super E> output, - Function<? super Artifact, E> outputFormatter, - MiddlemanExpander middlemanExpander) { + Function<? super ArtifactFile, E> outputFormatter, + ArtifactExpander artifactExpander) { for (Artifact artifact : artifacts) { - if (artifact.isMiddlemanArtifact()) { - expandMiddlemanArtifact(artifact, output, outputFormatter, middlemanExpander); + if (artifact.isMiddlemanArtifact() || artifact.isTreeArtifact()) { + expandArtifact(artifact, output, outputFormatter, artifactExpander); } else { output.add(outputFormatter.apply(artifact)); } } } - private static <E> void expandMiddlemanArtifact(Artifact middleman, - Collection<? super E> output, - Function<? super Artifact, E> outputFormatter, - MiddlemanExpander middlemanExpander) { - Preconditions.checkArgument(middleman.isMiddlemanArtifact()); - List<Artifact> artifacts = new ArrayList<>(); - middlemanExpander.expand(middleman, artifacts); - addExpandedArtifacts(artifacts, output, outputFormatter, middlemanExpander); + private static <E> void expandArtifact(Artifact middleman, + Collection<? super E> output, + Function<? super ArtifactFile, E> outputFormatter, + ArtifactExpander artifactExpander) { + Preconditions.checkArgument(middleman.isMiddlemanArtifact() || middleman.isTreeArtifact()); + List<ArtifactFile> artifacts = new ArrayList<>(); + artifactExpander.expand(middleman, artifacts); + for (ArtifactFile artifact : artifacts) { + output.add(outputFormatter.apply(artifact)); + } } /** @@ -702,9 +705,9 @@ public class Artifact implements FileType.HasFilename, ArtifactFile, SkylarkValu * returned list is mutable. */ public static List<String> asExpandedExecPathStrings(Iterable<Artifact> artifacts, - MiddlemanExpander middlemanExpander) { + ArtifactExpander artifactExpander) { List<String> result = new ArrayList<>(); - addExpandedExecPathStrings(artifacts, result, middlemanExpander); + addExpandedExecPathStrings(artifacts, result, artifactExpander); return result; } @@ -714,9 +717,9 @@ public class Artifact implements FileType.HasFilename, ArtifactFile, SkylarkValu * returned list is mutable. */ public static List<PathFragment> asExpandedExecPaths(Iterable<Artifact> artifacts, - MiddlemanExpander middlemanExpander) { + ArtifactExpander artifactExpander) { List<PathFragment> result = new ArrayList<>(); - addExpandedExecPaths(artifacts, result, middlemanExpander); + addExpandedExecPaths(artifacts, result, artifactExpander); return result; } 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 32cfeb4aee..7f3120c66c 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 @@ -27,7 +27,8 @@ import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.Artifact.MiddlemanExpander; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.ArtifactFile; import com.google.devtools.build.lib.actions.ArtifactResolver; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Executor; @@ -686,15 +687,15 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable @VisibleForTesting public void validateInclusions( Iterable<Artifact> inputsForValidation, - MiddlemanExpander middlemanExpander, + ArtifactExpander artifactExpander, EventHandler eventHandler) throws ActionExecutionException { IncludeProblems errors = new IncludeProblems(); IncludeProblems warnings = new IncludeProblems(); - Set<Artifact> allowedIncludes = new HashSet<>(); + Set<ArtifactFile> allowedIncludes = new HashSet<>(); for (Artifact input : mandatoryInputs) { - if (input.isMiddlemanArtifact()) { - middlemanExpander.expand(input, allowedIncludes); + if (input.isMiddlemanArtifact() || input.isTreeArtifact()) { + artifactExpander.expand(input, allowedIncludes); } allowedIncludes.add(input); } @@ -1138,7 +1139,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable // hdrs_check: This cannot be switched off, because doing so would allow for incorrect builds. validateInclusions( discoveredInputs, - actionExecutionContext.getMiddlemanExpander(), + actionExecutionContext.getArtifactExpander(), executor.getEventHandler()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java index b92e61496d..d445dc24ce 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java @@ -151,7 +151,7 @@ public class FakeCppCompileAction extends CppCompileAction { try { validateInclusions( discoveredInputs, - actionExecutionContext.getMiddlemanExpander(), + actionExecutionContext.getArtifactExpander(), executor.getEventHandler()); } catch (ActionExecutionException e) { // TODO(bazel-team): (2009) make this into an error, once most of the current warnings diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java index 9148014a5e..a6f73854a8 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java @@ -442,8 +442,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { MountMap mounts = new MountMap(); List<ActionInput> inputs = - ActionInputHelper.expandMiddlemen( - spawn.getInputFiles(), actionExecutionContext.getMiddlemanExpander()); + ActionInputHelper.expandArtifacts( + spawn.getInputFiles(), actionExecutionContext.getArtifactExpander()); if (spawn.getResourceOwner() instanceof CppCompileAction) { CppCompileAction action = (CppCompileAction) spawn.getResourceOwner(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 228d8f1b09..c79fd134d1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactFile; import com.google.devtools.build.lib.actions.MissingInputFileException; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.actions.PackageRootResolutionException; @@ -124,7 +125,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver env.getValues(state.allInputs.keysRequested); Preconditions.checkState(!env.valuesMissing(), "%s %s", action, state); } - Pair<Map<Artifact, FileArtifactValue>, Map<Artifact, Collection<Artifact>>> checkedInputs = + Pair<Map<Artifact, FileArtifactValue>, Map<Artifact, Collection<ArtifactFile>>> checkedInputs = null; try { // Declare deps on known inputs to action. We do this unconditionally to maintain our @@ -163,7 +164,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver if (checkedInputs != null) { Preconditions.checkState(!state.hasArtifactData(), "%s %s", state, action); state.inputArtifactData = checkedInputs.first; - state.expandedMiddlemen = checkedInputs.second; + state.expandedArtifacts = checkedInputs.second; } ActionExecutionValue result; @@ -369,7 +370,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver } actionExecutionContext = skyframeActionExecutor.getContext(perActionFileCache, - metadataHandler, state.expandedMiddlemen); + metadataHandler, state.expandedArtifacts); if (!state.hasExecutedAction()) { state.value = skyframeActionExecutor.executeAction(action, metadataHandler, actionStartTime, actionExecutionContext); @@ -515,8 +516,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver * Declare dependency on all known inputs of action. Throws exception if any are known to be * missing. Some inputs may not yet be in the graph, in which case the builder should abort. */ - private Pair<Map<Artifact, FileArtifactValue>, Map<Artifact, Collection<Artifact>>> checkInputs( - Environment env, Action action, + private Pair<Map<Artifact, FileArtifactValue>, Map<Artifact, Collection<ArtifactFile>>> + checkInputs(Environment env, Action action, Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> inputDeps) throws ActionExecutionException { int missingCount = 0; @@ -530,7 +531,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver NestedSetBuilder<Label> rootCauses = NestedSetBuilder.stableOrder(); Map<Artifact, FileArtifactValue> inputArtifactData = new HashMap<>(populateInputData ? inputDeps.size() : 0); - Map<Artifact, Collection<Artifact>> expandedMiddlemen = + Map<Artifact, Collection<ArtifactFile>> expandedArtifacts = new HashMap<>(populateInputData ? 128 : 0); ActionExecutionException firstActionExecutionException = null; @@ -547,9 +548,14 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver // We have to cache the "digest" of the aggregating value itself, because the action cache // checker may want it. inputArtifactData.put(input, aggregatingValue.getSelfData()); - expandedMiddlemen.put(input, + expandedArtifacts.put(input, Collections2.transform(aggregatingValue.getInputs(), - Pair.<Artifact, FileArtifactValue>firstFunction())); + new Function<Pair<Artifact, FileArtifactValue>, ArtifactFile>() { + @Override + public ArtifactFile apply(Pair<Artifact, FileArtifactValue> pair) { + return pair.first; + } + })); } else if (populateInputData && value instanceof FileArtifactValue) { // TODO(bazel-team): Make sure middleman "virtual" artifact data is properly processed. inputArtifactData.put(input, (FileArtifactValue) value); @@ -590,7 +596,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver } return Pair.of( Collections.unmodifiableMap(inputArtifactData), - Collections.unmodifiableMap(expandedMiddlemen)); + Collections.unmodifiableMap(expandedArtifacts)); } /** @@ -668,7 +674,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver private static class ContinuationState { AllInputs allInputs; Map<Artifact, FileArtifactValue> inputArtifactData = null; - Map<Artifact, Collection<Artifact>> expandedMiddlemen = null; + Map<Artifact, Collection<ArtifactFile>> expandedArtifacts = null; Token token = null; Collection<Artifact> discoveredInputs = null; ActionExecutionValue value = null; @@ -679,7 +685,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver boolean hasArtifactData() { boolean result = inputArtifactData != null; - Preconditions.checkState(result == (expandedMiddlemen != null), this); + Preconditions.checkState(result == (expandedArtifacts != null), this); return result; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index cf8556d2b6..5e2ba5d73b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -38,7 +38,8 @@ import com.google.devtools.build.lib.actions.ActionStartedEvent; import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.Artifact.MiddlemanExpander; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.ArtifactFile; import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException; import com.google.devtools.build.lib.actions.CachedActionEvent; import com.google.devtools.build.lib.actions.EnvironmentalExecException; @@ -428,17 +429,17 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto } } - private static class MiddlemanExpanderImpl implements MiddlemanExpander { - private final Map<Artifact, Collection<Artifact>> expandedInputMiddlemen; + private static class ArtifactExpanderImpl implements ArtifactExpander { + private final Map<Artifact, Collection<ArtifactFile>> expandedInputs; - private MiddlemanExpanderImpl(Map<Artifact, Collection<Artifact>> expandedInputMiddlemen) { - this.expandedInputMiddlemen = expandedInputMiddlemen; + private ArtifactExpanderImpl(Map<Artifact, Collection<ArtifactFile>> expandedInputMiddlemen) { + this.expandedInputs = expandedInputMiddlemen; } @Override - public void expand(Artifact middlemanArtifact, Collection<? super Artifact> output) { - Preconditions.checkState(middlemanArtifact.isMiddlemanArtifact(), middlemanArtifact); - Collection<Artifact> result = expandedInputMiddlemen.get(middlemanArtifact); + public void expand(Artifact artifact, Collection<? super ArtifactFile> output) { + Preconditions.checkState(artifact.isMiddlemanArtifact(), artifact); + Collection<ArtifactFile> result = expandedInputs.get(artifact); // Note that result may be null for non-aggregating middlemen. if (result != null) { output.addAll(result); @@ -454,14 +455,14 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto @Override public ActionExecutionContext getContext( ActionInputFileCache graphFileCache, MetadataHandler metadataHandler, - Map<Artifact, Collection<Artifact>> expandedInputMiddlemen) { + Map<Artifact, Collection<ArtifactFile>> expandedInputs) { FileOutErr fileOutErr = actionLogBufferPathGenerator.generate(); return new ActionExecutionContext( executorEngine, new DelegatingPairFileCache(graphFileCache, perBuildFileCache), metadataHandler, fileOutErr, - new MiddlemanExpanderImpl(expandedInputMiddlemen)); + new ArtifactExpanderImpl(expandedInputs)); } /** diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java index 20e0755be0..edee914438 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java @@ -161,8 +161,8 @@ final class WorkerSpawnStrategy implements SpawnActionContext { expandArgument(requestBuilder, Iterables.getLast(spawn.getArguments())); List<ActionInput> inputs = - ActionInputHelper.expandMiddlemen( - spawn.getInputFiles(), actionExecutionContext.getMiddlemanExpander()); + ActionInputHelper.expandArtifacts( + spawn.getInputFiles(), actionExecutionContext.getArtifactExpander()); for (ActionInput input : inputs) { ByteString digest = inputFileCache.getDigest(input); diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java index cd978217d0..c0c992f2b5 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java @@ -176,7 +176,7 @@ public class ArtifactTest { List<String> paths = new ArrayList<>(); MutableActionGraph actionGraph = new MapBasedActionGraph(); Artifact.addExpandedExecPathStrings(getFooBarArtifacts(actionGraph, true), paths, - ActionInputHelper.actionGraphMiddlemanExpander(actionGraph)); + ActionInputHelper.actionGraphArtifactExpander(actionGraph)); assertThat(paths).containsExactly("bar1.h", "bar1.h", "bar2.h", "bar3.h"); } @@ -185,7 +185,7 @@ public class ArtifactTest { List<PathFragment> paths = new ArrayList<>(); MutableActionGraph actionGraph = new MapBasedActionGraph(); Artifact.addExpandedExecPaths(getFooBarArtifacts(actionGraph, true), paths, - ActionInputHelper.actionGraphMiddlemanExpander(actionGraph)); + ActionInputHelper.actionGraphArtifactExpander(actionGraph)); assertThat(paths).containsExactly( new PathFragment("bar1.h"), new PathFragment("bar1.h"), @@ -195,11 +195,11 @@ public class ArtifactTest { @Test public void testAddExpandedArtifacts() throws Exception { - List<Artifact> expanded = new ArrayList<>(); + List<ArtifactFile> expanded = new ArrayList<>(); MutableActionGraph actionGraph = new MapBasedActionGraph(); List<Artifact> original = getFooBarArtifacts(actionGraph, true); Artifact.addExpandedArtifacts(original, expanded, - ActionInputHelper.actionGraphMiddlemanExpander(actionGraph)); + ActionInputHelper.actionGraphArtifactExpander(actionGraph)); List<Artifact> manuallyExpanded = new ArrayList<>(); for (Artifact artifact : original) { @@ -226,7 +226,7 @@ public class ArtifactTest { List<String> paths = new ArrayList<>(); MutableActionGraph actionGraph = new MapBasedActionGraph(); Artifact.addExpandedExecPathStrings(getFooBarArtifacts(actionGraph, true), paths, - ActionInputHelper.actionGraphMiddlemanExpander(actionGraph)); + ActionInputHelper.actionGraphArtifactExpander(actionGraph)); assertThat(paths).containsExactly("bar1.h", "bar1.h", "bar2.h", "bar3.h"); } @@ -235,7 +235,7 @@ public class ArtifactTest { List<PathFragment> paths = new ArrayList<>(); MutableActionGraph actionGraph = new MapBasedActionGraph(); Artifact.addExpandedExecPaths(getFooBarArtifacts(actionGraph, true), paths, - ActionInputHelper.actionGraphMiddlemanExpander(actionGraph)); + ActionInputHelper.actionGraphArtifactExpander(actionGraph)); assertThat(paths).containsExactly( new PathFragment("bar1.h"), new PathFragment("bar1.h"), @@ -243,13 +243,14 @@ public class ArtifactTest { new PathFragment("bar3.h")); } + // TODO consider tests for the future @Test public void testAddExpandedArtifactsNewActionGraph() throws Exception { - List<Artifact> expanded = new ArrayList<>(); + List<ArtifactFile> expanded = new ArrayList<>(); MutableActionGraph actionGraph = new MapBasedActionGraph(); List<Artifact> original = getFooBarArtifacts(actionGraph, true); Artifact.addExpandedArtifacts(original, expanded, - ActionInputHelper.actionGraphMiddlemanExpander(actionGraph)); + ActionInputHelper.actionGraphArtifactExpander(actionGraph)); List<Artifact> manuallyExpanded = new ArrayList<>(); for (Artifact artifact : original) { diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 305188e693..38aad5ce28 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -90,7 +90,7 @@ public final class ActionsTestUtil { metadataHandler, fileOutErr, actionGraph == null ? null - : ActionInputHelper.actionGraphMiddlemanExpander(actionGraph)); + : ActionInputHelper.actionGraphArtifactExpander(actionGraph)); } public static ActionExecutionContext createContextForInputDiscovery(Executor executor, |