Introduce CppCompileActionTemplate, which expands into a list of CppCompileActions that to be executed at execution time. -- PiperOrigin-RevId: 147163077 MOS_MIGRATED_REVID=147163077
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 8072b3a..29b9902 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
@@ -508,6 +508,10 @@ "The parent of TreeFileArtifact (parent-relative path: %s) is not a TreeArtifact: %s", parentRelativePath, parent); + Preconditions.checkState( + parentRelativePath.isNormalized() && !parentRelativePath.isAbsolute(), + "%s is not a proper normalized relative path", + parentRelativePath); this.parentTreeArtifact = parent; this.parentRelativePath = parentRelativePath; }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ActionTemplate.java new file mode 100644 index 0000000..afad7b7 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ActionTemplate.java
@@ -0,0 +1,75 @@ +// Copyright 2017 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.analysis.actions; + +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; +import com.google.devtools.build.lib.actions.ArtifactOwner; + +/** + * A placeholder action that, at execution time, expands into a list of {@link Action}s to be + * executed. + * + * <p>ActionTemplate is for users who want to dynamically register Actions operating on + * individual {@link TreeFileArtifact} inside input and output TreeArtifacts at execution time. + * + * <p>It takes in one TreeArtifact and generates one TreeArtifact. The following happens at + * execution time for ActionTemplate: + * <ol> + * <li>Input TreeArtifact is resolved. + * <li>For each individual {@link TreeFileArtifact} inside input TreeArtifact, generate an output + * {@link TreeFileArtifact} inside output TreeArtifact. + * <li>For each pair of input and output {@link TreeFileArtifact}s, generate an associated + * {@link Action}. + * <li>All expanded {@link Action}s are executed and their output {@link TreeFileArtifact}s + * collected. + * <li>Output TreeArtifact is resolved. + * </ol> + * + * <p>Implementations of ActionTemplate must follow the contract of this interface and also make + * sure: + * <ol> + * <li>ActionTemplate instances should be immutable and side-effect free. + * <li>ActionTemplate inputs and outputs are supersets of the inputs and outputs of expanded + * actions, excluding inputs discovered at execution time. This ensures the ActionTemplate + * can properly represent the expanded actions at analysis time, and the action graph + * at analysis time is correct. This is important because the action graph is walked in a lot + * of places for correctness checks and build analysis. + * <li>The outputs of expanded actions must be under the output TreeArtifact and must not have + * artifact or artifact path prefix conflicts. + * </ol> + */ +public interface ActionTemplate<T extends Action> extends ActionAnalysisMetadata { + /** + * Given a list of input TreeFileArtifacts resolved at execution time, returns a list of expanded + * SpawnActions to be executed. + * + * @param inputTreeFileArtifacts the list of {@link TreeFileArtifact}s inside input TreeArtifact + * resolved at execution time + * @param artifactOwner the {@link ArtifactOwner} of the generated output + * {@link TreeFileArtifact}s + * @return a list of expanded {@link Action}s to execute, one for each input + * {@link TreeFileArtifact} + */ + Iterable<T> generateActionForInputArtifacts( + Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner); + + /** Returns the input TreeArtifact. */ + Artifact getInputTreeArtifact(); + + /** Returns the output TreeArtifact. */ + Artifact getOutputTreeArtifact(); +}
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 4587e4a..6394c0c 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
@@ -18,12 +18,9 @@ import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.ActionOwner; -import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; -import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException; -import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -32,27 +29,9 @@ import java.util.Map; /** - * A placeholder action that, at execution time, expands into a list of {@link SpawnAction}s that - * will be executed. - * - * <p>SpawnActionTemplate is for users who want to dynamically register SpawnActions operating on - * individual {@link TreeFileArtifact} inside input and output TreeArtifacts at execution time. - * - * <p>It takes in one TreeArtifact and generates one TreeArtifact. The following happens at - * execution time for SpawnActionTemplate: - * <ol> - * <li>Input TreeArtifact is resolved. - * <li>For each individual {@link TreeFileArtifact} inside input TreeArtifact, generate an output - * {@link TreeFileArtifact} inside output TreeArtifact at the parent-relative path provided by - * {@link OutputPathMapper}. - * <li>For each pair of input and output {@link TreeFileArtifact}s, generate an associated - * {@link SpawnAction}. - * <li>All expanded {@link SpawnAction}s are executed and their output {@link TreeFileArtifact}s - * collected. - * <li>Output TreeArtifact is resolved. - * </ol> + * An {@link ActionTemplate} that expands into {@link SpawnAction}s at execution time. */ -public final class SpawnActionTemplate implements ActionAnalysisMetadata { +public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> { private final Artifact inputTreeArtifact; private final Artifact outputTreeArtifact; private final NestedSet<Artifact> commonInputs; @@ -110,39 +89,23 @@ this.commandLineTemplate = commandLineTemplate; } - /** - * Given a list of input TreeFileArtifacts resolved at execution time, returns a list of expanded - * SpawnActions to be executed. - * - * @param inputTreeFileArtifacts the list of {@link TreeFileArtifact}s inside input TreeArtifact - * resolved at execution time - * @param artifactOwner the {@link ArtifactOwner} of the generated output - * {@link TreeFileArtifact}s - * @return a list of expanded {@link SpawnAction}s to execute, one for each input - * {@link TreeFileArtifact} - * @throws ActionConflictException if the expanded actions have duplicated outputs - * @throws ArtifactPrefixConflictException if there is prefix conflict among the outputs of - * expanded actions - */ + @Override public Iterable<SpawnAction> generateActionForInputArtifacts( - Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner) - throws ActionConflictException, ArtifactPrefixConflictException { + Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner) { ImmutableList.Builder<SpawnAction> expandedActions = new ImmutableList.Builder<>(); for (TreeFileArtifact inputTreeFileArtifact : inputTreeFileArtifacts) { PathFragment parentRelativeOutputPath = outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact); - TreeFileArtifact outputTreeFileArtifact = createTreeFileArtifact( + TreeFileArtifact outputTreeFileArtifact = ActionInputHelper.treeFileArtifact( outputTreeArtifact, - checkOutputParentRelativePath(parentRelativeOutputPath), + parentRelativeOutputPath, artifactOwner); expandedActions.add(createAction(inputTreeFileArtifact, outputTreeFileArtifact)); } - Iterable<SpawnAction> actions = expandedActions.build(); - checkActionAndArtifactConflicts(ImmutableList.<ActionAnalysisMetadata>copyOf(actions)); - return actions; + return expandedActions.build(); } /** @@ -171,37 +134,6 @@ /*paramFileWriteAction=*/ null); } - private static void checkActionAndArtifactConflicts(Iterable<ActionAnalysisMetadata> actions) - throws ActionConflictException, ArtifactPrefixConflictException { - Map<Artifact, ActionAnalysisMetadata> generatingActions = - Actions.findAndThrowActionConflict(actions); - Map<ActionAnalysisMetadata, ArtifactPrefixConflictException> artifactPrefixConflictMap = - Actions.findArtifactPrefixConflicts(generatingActions); - - if (!artifactPrefixConflictMap.isEmpty()) { - throw artifactPrefixConflictMap.values().iterator().next(); - } - - return; - } - - private static PathFragment checkOutputParentRelativePath(PathFragment parentRelativeOutputPath) { - Preconditions.checkArgument( - parentRelativeOutputPath.isNormalized() && !parentRelativeOutputPath.isAbsolute(), - "%s is not a proper relative path", - parentRelativeOutputPath); - return parentRelativeOutputPath; - } - - private static TreeFileArtifact createTreeFileArtifact(Artifact parentTreeArtifact, - PathFragment parentRelativeOutputPath, ArtifactOwner artifactOwner) { - return ActionInputHelper.treeFileArtifact( - parentTreeArtifact, - parentRelativeOutputPath, - artifactOwner); - } - - /** * Returns the input TreeArtifact. * @@ -209,11 +141,13 @@ * TreeFileArtifacts. Skyframe then expands this SpawnActionTemplate with the TreeFileArtifacts * through {@link #generateActionForInputArtifacts}. */ + @Override public Artifact getInputTreeArtifact() { return inputTreeArtifact; } /** Returns the output TreeArtifact. */ + @Override public Artifact getOutputTreeArtifact() { return outputTreeArtifact; } @@ -223,7 +157,6 @@ return actionOwner; } - @Override public final String getMnemonic() { return mnemonic;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java index f601913d..f70a4ed 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java
@@ -200,7 +200,10 @@ * Adds an .o file. */ public Builder addObjectFile(Artifact artifact) { - Preconditions.checkArgument(Link.OBJECT_FILETYPES.matches(artifact.getFilename())); + // We skip file extension checks for TreeArtifacts because they represent directory artifacts + // without a file extension. + Preconditions.checkArgument( + artifact.isTreeArtifact() || Link.OBJECT_FILETYPES.matches(artifact.getFilename())); objectFiles.add(artifact); return this; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index 355d01b..9d52957 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -451,13 +451,16 @@ * file type also to compilationUnitSources. */ private void addHeader(Artifact header, Label label) { - boolean isHeader = CppFileTypes.CPP_HEADER.matches(header.getExecPath()); + // We assume TreeArtifacts passed in are directories containing proper headers. + boolean isHeader = + CppFileTypes.CPP_HEADER.matches(header.getExecPath()) || header.isTreeArtifact(); boolean isTextualInclude = CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(header.getExecPath()); publicHeaders.add(header); if (isTextualInclude || !isHeader || !shouldProcessHeaders()) { return; } - compilationUnitSources.add(CppSource.create(header, label, ImmutableMap.<String, String>of())); + compilationUnitSources.add( + CppSource.create(header, label, ImmutableMap.<String, String>of(), CppSource.Type.HEADER)); } /** Adds a header to {@code publicHeaders}, but not to this target's module map. */ @@ -476,14 +479,26 @@ Preconditions.checkNotNull(featureConfiguration); boolean isHeader = CppFileTypes.CPP_HEADER.matches(source.getExecPath()); boolean isTextualInclude = CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(source.getExecPath()); - boolean isCompiledSource = sourceCategory.getSourceTypes().matches(source.getExecPathString()); + // We assume TreeArtifacts passed in are directories containing proper sources for compilation. + boolean isCompiledSource = + sourceCategory.getSourceTypes().matches(source.getExecPathString()) + || source.isTreeArtifact(); if (isHeader || isTextualInclude) { privateHeaders.add(source); } if (isTextualInclude || !isCompiledSource || (isHeader && !shouldProcessHeaders())) { return; } - compilationUnitSources.add(CppSource.create(source, label, buildVariables)); + boolean isClifInputProto = CppFileTypes.CLIF_INPUT_PROTO.matches(source.getExecPathString()); + CppSource.Type type; + if (isHeader) { + type = CppSource.Type.HEADER; + } else if (isClifInputProto) { + type = CppSource.Type.CLIF_INPUT_PROTO; + } else { + type = CppSource.Type.SOURCE; + } + compilationUnitSources.add(CppSource.create(source, label, buildVariables, type)); } private boolean shouldProcessHeaders() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java index f9295c6..3d03ece 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -1407,6 +1407,13 @@ private final Map<String, VariableValue> variablesMap = new LinkedHashMap<>(); private final Map<String, String> stringVariablesMap = new LinkedHashMap<>(); + public Builder() {}; + + public Builder(Variables variables) { + variablesMap.putAll(variables.variablesMap); + stringVariablesMap.putAll(variables.stringVariablesMap); + } + /** Add a variable that expands {@code name} to {@code value}. */ public Builder addStringVariable(String name, String value) { Preconditions.checkArgument( @@ -1419,6 +1426,14 @@ return this; } + /** Overrides a variable to expands {@code name} to {@code value} instead. */ + public Builder overrideStringVariable(String name, String value) { + Preconditions.checkNotNull( + value, "Cannot set null as a value for variable '%s'", name); + stringVariablesMap.put(name, value); + return this; + } + /** * Add a sequence variable that expands {@code name} to {@code values}. *
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 6d73265..c51edff 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
@@ -36,7 +36,6 @@ import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.extra.CppCompileInfo; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; -import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.PerLabelOptions; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.CollectionUtils; @@ -236,6 +235,7 @@ * Creates a new action to compile C/C++ source files. * * @param owner the owner of the action, usually the configured target that emitted it + * @param allInputs the list of all action inputs. * @param features TODO(bazel-team): Add parameter description. * @param featureConfiguration TODO(bazel-team): Add parameter description. * @param variables TODO(bazel-team): Add parameter description. @@ -266,14 +266,15 @@ * @param executionRequirements out-of-band hints to be passed to the execution backend to signal * platform requirements * @param environment TODO(bazel-team): Add parameter description + * @param builtinIncludeFiles List of include files that may be included even if they are not + * mentioned in the source file or any of the headers included by it * @param actionName a string giving the name of this action for the purpose of toolchain * evaluation - * @param ruleContext The rule-context that produced this action * @param cppSemantics C++ compilation semantics - * @param ccToolchain C++ toolchain provider */ protected CppCompileAction( ActionOwner owner, + NestedSet<Artifact> allInputs, // TODO(bazel-team): Eventually we will remove 'features'; all functionality in 'features' // will be provided by 'featureConfiguration'. ImmutableList<String> features, @@ -306,18 +307,11 @@ ImmutableMap<String, String> executionInfo, ImmutableMap<String, String> environment, String actionName, - RuleContext ruleContext, - CppSemantics cppSemantics, - CcToolchainProvider ccToolchain) { + Iterable<Artifact> builtinIncludeFiles, + CppSemantics cppSemantics) { super( owner, - createInputs( - ruleContext, - ccToolchain, - mandatoryInputs, - context.getTransitiveCompilationPrerequisites(), - optionalSourceFile, - lipoScannables), + allInputs, CollectionUtils.asListWithoutNulls( outputFile, (dotdFile == null ? null : dotdFile.artifact()), gcnoFile, dwoFile)); this.localShellEnvironment = localShellEnvironment; @@ -351,81 +345,12 @@ // artifact and will definitely exist prior to this action execution. this.mandatoryInputs = mandatoryInputs; this.prunableInputs = prunableInputs; - this.builtinIncludeFiles = ccToolchain.getBuiltinIncludeFiles(); + this.builtinIncludeFiles = ImmutableList.copyOf(builtinIncludeFiles); this.cppSemantics = cppSemantics; - if (cppSemantics.needsIncludeValidation()) { - verifyIncludePaths(ruleContext); - } this.additionalIncludeScannables = ImmutableList.copyOf(additionalIncludeScannables); } /** - * Verifies that the include paths of this action are within the limits of the execution root. - */ - private void verifyIncludePaths(RuleContext ruleContext) { - if (ruleContext == null) { - return; - } - - Iterable<PathFragment> ignoredDirs = getValidationIgnoredDirs(); - - // We currently do not check the output of: - // - getQuoteIncludeDirs(): those only come from includes attributes, and are checked in - // CcCommon.getIncludeDirsFromIncludesAttribute(). - // - getBuiltinIncludeDirs(): while in practice this doesn't happen, bazel can be configured - // to use an absolute system root, in which case the builtin include dirs might be absolute. - for (PathFragment include : Iterables.concat(getIncludeDirs(), getSystemIncludeDirs())) { - - // Ignore headers from built-in include directories. - if (FileSystemUtils.startsWithAny(include, ignoredDirs)) { - continue; - } - - // One starting ../ is okay for getting to a sibling repository. - if (include.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX))) { - include = include.relativeTo(Label.EXTERNAL_PATH_PREFIX); - } - - if (include.isAbsolute() - || !PathFragment.EMPTY_FRAGMENT.getRelative(include).normalize().isNormalized()) { - ruleContext.ruleError( - "The include path '" + include + "' references a path outside of the execution root."); - } - } - } - - private static NestedSet<Artifact> createInputs( - RuleContext ruleContext, - CcToolchainProvider ccToolchain, - NestedSet<Artifact> mandatoryInputs, - Set<Artifact> prerequisites, - Artifact optionalSourceFile, - Iterable<IncludeScannable> lipoScannables) { - NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder(); - if (optionalSourceFile != null) { - builder.add(optionalSourceFile); - } - builder.addAll(prerequisites); - builder.addAll(ccToolchain.getBuiltinIncludeFiles()); - builder.addTransitive(mandatoryInputs); - if (lipoScannables != null && lipoScannables.iterator().hasNext()) { - // We need to add "legal generated scanner files" coming through LIPO scannables here. These - // usually contain pre-grepped source files, i.e. files just containing the #include lines - // extracted from generated files. With LIPO, some of these files can be accessed, even though - // there is no direct dependency on them. Adding the artifacts as inputs to this compile - // action ensures that the action generating them is actually executed. - for (IncludeScannable lipoScannable : lipoScannables) { - for (Artifact value : lipoScannable.getLegalGeneratedScannerFileMap().values()) { - if (value != null) { - builder.add(value); - } - } - } - } - return builder.build(); - } - - /** * Whether we should do "include scanning". Note that this does *not* mean whether we should parse * the .d files to determine which include files were used during compilation. Instead, this means * whether we should a) run the pre-execution include scanner (see {@code IncludeScanningContext}) @@ -959,7 +884,7 @@ errors.assertProblemFree(this, getSourceFile()); } - private Iterable<PathFragment> getValidationIgnoredDirs() { + Iterable<PathFragment> getValidationIgnoredDirs() { List<PathFragment> cxxSystemIncludeDirs = cppConfiguration.getBuiltInIncludeDirectories(); return Iterables.concat( cxxSystemIncludeDirs, context.getSystemIncludeDirs()); @@ -1383,7 +1308,7 @@ CcToolchainFeatures.Variables variables, String actionName) { this.sourceFile = Preconditions.checkNotNull(sourceFile); - this.dotdFile = CppFileTypes.mustProduceDotdFile(sourceFile.getPath().toString()) + this.dotdFile = CppFileTypes.mustProduceDotdFile(sourceFile) ? Preconditions.checkNotNull(dotdFile) : null; this.copts = Preconditions.checkNotNull(copts); this.coptsFilter = coptsFilter;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index ee606bc..39e7ff9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -22,7 +22,6 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -34,12 +33,14 @@ import com.google.devtools.build.lib.rules.cpp.CppCompileAction.SpecialInputsHandler; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.regex.Pattern; @@ -53,7 +54,7 @@ private final List<String> features = new ArrayList<>(); private CcToolchainFeatures.FeatureConfiguration featureConfiguration; private CcToolchainFeatures.Variables variables; - private final Artifact sourceFile; + private Artifact sourceFile; private final Label sourceLabel; private final NestedSetBuilder<Artifact> mandatoryInputsBuilder; private Artifact optionalSourceFile; @@ -62,12 +63,10 @@ private PathFragment tempOutputFile; private DotdFile dotdFile; private Artifact gcnoFile; - private final BuildConfiguration configuration; private CppCompilationContext context = CppCompilationContext.EMPTY; private final List<String> copts = new ArrayList<>(); private final List<String> pluginOpts = new ArrayList<>(); private final List<Pattern> nocopts = new ArrayList<>(); - private AnalysisEnvironment analysisEnvironment; private ImmutableList<PathFragment> extraSystemIncludePrefixes = ImmutableList.of(); private boolean usePic; private boolean allowUsingHeaderModules; @@ -78,34 +77,50 @@ private ImmutableMap<Artifact, IncludeScannable> lipoScannableMap; private final ImmutableList.Builder<Artifact> additionalIncludeFiles = new ImmutableList.Builder<>(); - private RuleContext ruleContext = null; private Boolean shouldScanIncludes; private Map<String, String> executionInfo = new LinkedHashMap<>(); private Map<String, String> environment = new LinkedHashMap<>(); private CppSemantics cppSemantics; private CcToolchainProvider ccToolchain; + private final ImmutableMap<String, String> localShellEnvironment; + private final boolean codeCoverageEnabled; // New fields need to be added to the copy constructor. /** * Creates a builder from a rule. This also uses the configuration and * artifact factory from the rule. */ - public CppCompileActionBuilder(RuleContext ruleContext, Artifact sourceFile, Label sourceLabel, + public CppCompileActionBuilder(RuleContext ruleContext, Label sourceLabel, CcToolchainProvider ccToolchain) { - this.owner = ruleContext.getActionOwner(); - this.actionContext = CppCompileActionContext.class; - this.cppConfiguration = ruleContext.getFragment(CppConfiguration.class); - this.analysisEnvironment = ruleContext.getAnalysisEnvironment(); - this.sourceFile = sourceFile; - this.sourceLabel = sourceLabel; - this.configuration = ruleContext.getConfiguration(); - this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder(); - this.lipoScannableMap = getLipoScannableMap(ruleContext); - this.ruleContext = ruleContext; - this.allowUsingHeaderModules = true; - this.ccToolchain = ccToolchain; + this( + ruleContext.getActionOwner(), + sourceLabel, + ruleContext.getFragment(CppConfiguration.class), + ruleContext.getConfiguration(), + getLipoScannableMap(ruleContext), + ruleContext.getFeatures(), + ccToolchain); + } - features.addAll(ruleContext.getFeatures()); + private CppCompileActionBuilder( + ActionOwner actionOwner, + Label sourceLabel, + CppConfiguration cppConfiguration, + BuildConfiguration configuration, + Map<Artifact, IncludeScannable> lipoScannableMap, + Set<String> features, + CcToolchainProvider ccToolchain) { + this.owner = actionOwner; + this.sourceLabel = sourceLabel; + this.cppConfiguration = cppConfiguration; + this.lipoScannableMap = ImmutableMap.copyOf(lipoScannableMap); + this.features.addAll(features); + this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder(); + this.allowUsingHeaderModules = true; + this.localShellEnvironment = configuration.getLocalShellEnvironment(); + this.codeCoverageEnabled = configuration.isCodeCoverageEnabled(); + this.actionContext = CppCompileActionContext.class; + this.ccToolchain = ccToolchain; } private static ImmutableMap<Artifact, IncludeScannable> getLipoScannableMap( @@ -115,7 +130,7 @@ // contain headers, will still create CppCompileActions without providing a // lipo_context_collector. || ruleContext.attributes().getAttributeDefinition(":lipo_context_collector") == null) { - return null; + return ImmutableMap.<Artifact, IncludeScannable>of(); } LipoContextProvider provider = ruleContext.getPrerequisite( ":lipo_context_collector", Mode.DONT_CHECK, LipoContextProvider.class); @@ -129,6 +144,7 @@ this.owner = other.owner; this.features.addAll(other.features); this.featureConfiguration = other.featureConfiguration; + this.variables = other.variables; this.sourceFile = other.sourceFile; this.sourceLabel = other.sourceLabel; this.mandatoryInputsBuilder = NestedSetBuilder.<Artifact>stableOrder() @@ -139,12 +155,10 @@ this.tempOutputFile = other.tempOutputFile; this.dotdFile = other.dotdFile; this.gcnoFile = other.gcnoFile; - this.configuration = other.configuration; this.context = other.context; this.copts.addAll(other.copts); this.pluginOpts.addAll(other.pluginOpts); this.nocopts.addAll(other.nocopts); - this.analysisEnvironment = other.analysisEnvironment; this.extraSystemIncludePrefixes = ImmutableList.copyOf(other.extraSystemIncludePrefixes); this.specialInputsHandler = other.specialInputsHandler; this.actionClassId = other.actionClassId; @@ -153,10 +167,11 @@ this.usePic = other.usePic; this.allowUsingHeaderModules = other.allowUsingHeaderModules; this.lipoScannableMap = other.lipoScannableMap; - this.ruleContext = other.ruleContext; this.shouldScanIncludes = other.shouldScanIncludes; this.executionInfo = new LinkedHashMap<>(other.executionInfo); this.environment = new LinkedHashMap<>(other.environment); + this.localShellEnvironment = other.localShellEnvironment; + this.codeCoverageEnabled = other.codeCoverageEnabled; this.cppSemantics = other.cppSemantics; this.ccToolchain = other.ccToolchain; } @@ -165,6 +180,11 @@ return tempOutputFile; } + public CppCompileActionBuilder setSourceFile(Artifact sourceFile) { + this.sourceFile = sourceFile; + return this; + } + public Artifact getSourceFile() { return sourceFile; } @@ -198,16 +218,21 @@ } private Iterable<IncludeScannable> getLipoScannables(NestedSet<Artifact> realMandatoryInputs) { - return lipoScannableMap == null ? ImmutableList.<IncludeScannable>of() : Iterables.filter( - Iterables.transform( - Iterables.filter( - FileType.filter( - realMandatoryInputs, - CppFileTypes.C_SOURCE, CppFileTypes.CPP_SOURCE, - CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR), - Predicates.not(Predicates.equalTo(getSourceFile()))), - Functions.forMap(lipoScannableMap, null)), - Predicates.notNull()); + boolean fake = tempOutputFile != null; + + return lipoScannableMap == null || fake + ? ImmutableList.<IncludeScannable>of() + : Iterables.filter( + Iterables.transform( + Iterables.filter( + FileType.filter( + realMandatoryInputs, + CppFileTypes.C_SOURCE, + CppFileTypes.CPP_SOURCE, + CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR), + Predicates.not(Predicates.equalTo(getSourceFile()))), + Functions.forMap(lipoScannableMap, null)), + Predicates.notNull()); } private String getActionName() { @@ -248,7 +273,23 @@ } /** - * Builds the Action as configured and returns the to be generated Artifact. + * Builds the action and performs some validations on the action. + * + * <p>This method may be called multiple times to create multiple compile + * actions (usually after calling some setters to modify the generated + * action). + */ + public CppCompileAction buildAndValidate(RuleContext ruleContext) { + CppCompileAction action = build(); + if (cppSemantics.needsIncludeValidation()) { + verifyActionIncludePaths(action, ruleContext); + } + return action; + } + + /** + * Builds the Action as configured without validations. Users may want to call + * {@link #buildAndValidate} instead. * * <p>This method may be called multiple times to create multiple compile * actions (usually after calling some setters to modify the generated @@ -262,19 +303,6 @@ allowUsingHeaderModules && featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES); - boolean fake = tempOutputFile != null; - // Configuration can be null in tests. - NestedSetBuilder<Artifact> realMandatoryInputsBuilder = NestedSetBuilder.compileOrder(); - realMandatoryInputsBuilder.addTransitive(mandatoryInputsBuilder.build()); - boolean shouldPruneModules = - cppConfiguration.getPruneCppModules() && shouldScanIncludes && useHeaderModules; - if (useHeaderModules && !shouldPruneModules) { - realMandatoryInputsBuilder.addTransitive(context.getTransitiveModules(usePic)); - } - realMandatoryInputsBuilder.addTransitive(context.getAdditionalInputs()); - - realMandatoryInputsBuilder.add(sourceFile); - // If the crosstool uses action_configs to configure cc compilation, collect execution info // from there, otherwise, use no execution info. // TODO(b/27903698): Assert that the crosstool has an action_config for this action. @@ -285,7 +313,8 @@ } } - NestedSet<Artifact> realMandatoryInputs = realMandatoryInputsBuilder.build(); + NestedSet<Artifact> realMandatoryInputs = buildMandatoryInputs(); + NestedSet<Artifact> allInputs = buildAllInputs(); NestedSetBuilder<Artifact> prunableInputBuilder = NestedSetBuilder.stableOrder(); prunableInputBuilder.addTransitive(context.getDeclaredIncludeSrcs()); @@ -294,15 +323,17 @@ NestedSet<Artifact> prunableInputs = prunableInputBuilder.build(); // Copying the collections is needed to make the builder reusable. + boolean fake = tempOutputFile != null; if (fake) { return new FakeCppCompileAction( owner, + allInputs, ImmutableList.copyOf(features), featureConfiguration, variables, sourceFile, shouldScanIncludes, - shouldPruneModules, + shouldPruneModules(), usePic, useHeaderModules, sourceLabel, @@ -311,25 +342,27 @@ outputFile, tempOutputFile, dotdFile, - configuration, + localShellEnvironment, + codeCoverageEnabled, cppConfiguration, context, actionContext, ImmutableList.copyOf(copts), getNocoptPredicate(nocopts), - ruleContext, + getLipoScannables(realMandatoryInputs), + ccToolchain.getBuiltinIncludeFiles(), cppSemantics, - ccToolchain, ImmutableMap.copyOf(executionInfo)); } else { return new CppCompileAction( owner, + allInputs, ImmutableList.copyOf(features), featureConfiguration, variables, sourceFile, shouldScanIncludes, - shouldPruneModules, + shouldPruneModules(), usePic, useHeaderModules, sourceLabel, @@ -340,8 +373,8 @@ gcnoFile, dwoFile, optionalSourceFile, - configuration.getLocalShellEnvironment(), - configuration.isCodeCoverageEnabled(), + localShellEnvironment, + codeCoverageEnabled, cppConfiguration, context, actionContext, @@ -354,9 +387,84 @@ ImmutableMap.copyOf(executionInfo), ImmutableMap.copyOf(environment), getActionName(), - ruleContext, - cppSemantics, - ccToolchain); + ccToolchain.getBuiltinIncludeFiles(), + cppSemantics); + } + } + + /** + * Returns the list of mandatory inputs for the {@link CppCompileAction} as configured. + */ + NestedSet<Artifact> buildMandatoryInputs() { + NestedSetBuilder<Artifact> realMandatoryInputsBuilder = NestedSetBuilder.compileOrder(); + realMandatoryInputsBuilder.addTransitive(mandatoryInputsBuilder.build()); + if (useHeaderModules() && !shouldPruneModules()) { + realMandatoryInputsBuilder.addTransitive(context.getTransitiveModules(usePic)); + } + realMandatoryInputsBuilder.addTransitive(context.getAdditionalInputs()); + realMandatoryInputsBuilder.add(Preconditions.checkNotNull(sourceFile)); + return realMandatoryInputsBuilder.build(); + } + + /** + * Returns the list of all inputs for the {@link CppCompileAction} as configured. + */ + NestedSet<Artifact> buildAllInputs() { + NestedSet<Artifact> mandatoryInputs = buildMandatoryInputs(); + NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder(); + if (optionalSourceFile != null) { + builder.add(optionalSourceFile); + } + builder.addAll(context.getTransitiveCompilationPrerequisites()); + builder.addAll(ccToolchain.getBuiltinIncludeFiles()); + builder.addTransitive(mandatoryInputs); + Iterable<IncludeScannable> lipoScannables = getLipoScannables(mandatoryInputs); + // We need to add "legal generated scanner files" coming through LIPO scannables here. These + // usually contain pre-grepped source files, i.e. files just containing the #include lines + // extracted from generated files. With LIPO, some of these files can be accessed, even though + // there is no direct dependency on them. Adding the artifacts as inputs to this compile + // action ensures that the action generating them is actually executed. + for (IncludeScannable lipoScannable : lipoScannables) { + for (Artifact value : lipoScannable.getLegalGeneratedScannerFileMap().values()) { + if (value != null) { + builder.add(value); + } + } + } + return builder.build(); + } + + private boolean useHeaderModules() { + return allowUsingHeaderModules + && featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES); + } + + private boolean shouldPruneModules() { + return cppConfiguration.getPruneCppModules() && shouldScanIncludes && useHeaderModules(); + } + + private static void verifyActionIncludePaths(CppCompileAction action, RuleContext ruleContext) { + Iterable<PathFragment> ignoredDirs = action.getValidationIgnoredDirs(); + // We currently do not check the output of: + // - getQuoteIncludeDirs(): those only come from includes attributes, and are checked in + // CcCommon.getIncludeDirsFromIncludesAttribute(). + // - getBuiltinIncludeDirs(): while in practice this doesn't happen, bazel can be configured + // to use an absolute system root, in which case the builtin include dirs might be absolute. + for (PathFragment include : + Iterables.concat(action.getIncludeDirs(), action.getSystemIncludeDirs())) { + // Ignore headers from built-in include directories. + if (FileSystemUtils.startsWithAny(include, ignoredDirs)) { + continue; + } + // One starting ../ is okay for getting to a sibling repository. + if (include.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX))) { + include = include.relativeTo(Label.EXTERNAL_PATH_PREFIX); + } + if (include.isAbsolute() + || !PathFragment.EMPTY_FRAGMENT.getRelative(include).normalize().isNormalized()) { + ruleContext.ruleError( + "The include path '" + include + "' references a path outside of the execution root."); + } } } @@ -377,6 +485,13 @@ return this; } + /** + * Returns the build variables to be used for the action. + */ + CcToolchainFeatures.Variables getVariables() { + return variables; + } + public CppCompileActionBuilder addEnvironment(Map<String, String> environment) { this.environment.putAll(environment); return this; @@ -435,22 +550,24 @@ return this; } - public CppCompileActionBuilder setOutputsForTesting(Artifact outputFile, Artifact dotdFile) { + public CppCompileActionBuilder setOutputs(Artifact outputFile, Artifact dotdFile) { this.outputFile = outputFile; this.dotdFile = dotdFile == null ? null : new DotdFile(dotdFile); return this; } public CppCompileActionBuilder setOutputs( - ArtifactCategory outputCategory, String outputName, boolean generateDotd) { + RuleContext ruleContext, ArtifactCategory outputCategory, String outputName, + boolean generateDotd) { this.outputFile = CppHelper.getCompileOutputArtifact( ruleContext, CppHelper.getArtifactNameForCategory(ruleContext, ccToolchain, outputCategory, outputName)); if (generateDotd) { String dotdFileName = CppHelper.getDotdFileName(ruleContext, ccToolchain, outputCategory, outputName); - if (configuration.getFragment(CppConfiguration.class).getInmemoryDotdFiles()) { + if (cppConfiguration.getInmemoryDotdFiles()) { // Just set the path, no artifact is constructed + BuildConfiguration configuration = ruleContext.getConfiguration(); dotdFile = new DotdFile( configuration.getBinDirectory(ruleContext.getRule().getRepository()).getExecPath() .getRelative(CppHelper.getObjDirectory(ruleContext.getLabel()))
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 new file mode 100644 index 0000000..21abea8 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
@@ -0,0 +1,200 @@ +// Copyright 2017 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.rules.cpp; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType; +import com.google.devtools.build.lib.actions.ActionInputHelper; +import com.google.devtools.build.lib.actions.ActionOwner; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; +import com.google.devtools.build.lib.actions.ArtifactOwner; +import com.google.devtools.build.lib.analysis.actions.ActionTemplate; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.PathFragment; + +/** + * An {@link ActionTemplate} that expands into {@link CppCompileAction}s at execution time. + */ +public final class CppCompileActionTemplate implements ActionTemplate<CppCompileAction> { + private final CppCompileActionBuilder cppCompileActionBuilder; + private final Artifact sourceTreeArtifact; + private final Artifact outputTreeArtifact; + private final CppConfiguration cppConfiguration; + private final Iterable<ArtifactCategory> categories; + private final ActionOwner actionOwner; + private final NestedSet<Artifact> mandatoryInputs; + private final NestedSet<Artifact> allInputs; + + /** + * Creates an CppCompileActionTemplate. + * @param sourceTreeArtifact the TreeArtifact that contains source files to compile. + * @param outputTreeArtifact the TreeArtifact that contains compilation outputs. + * @param cppCompileActionBuilder An almost completely configured {@link CppCompileActionBuilder} + * without the input and output files set. It is used as a template to instantiate expanded + * {CppCompileAction}s. + * @param cppConfiguration configuration for cpp. + * @param categories A list of {@link ArtifactCategory} used to calculate output file name from + * a source file name. + * @param actionOwner the owner of this {@link ActionTemplate}. + */ + CppCompileActionTemplate( + Artifact sourceTreeArtifact, + Artifact outputTreeArtifact, + CppCompileActionBuilder cppCompileActionBuilder, + CppConfiguration cppConfiguration, + Iterable<ArtifactCategory> categories, + ActionOwner actionOwner) { + this.cppCompileActionBuilder = cppCompileActionBuilder; + this.sourceTreeArtifact = sourceTreeArtifact; + this.outputTreeArtifact = outputTreeArtifact; + this.cppConfiguration = cppConfiguration; + this.categories = categories; + this.actionOwner = actionOwner; + this.mandatoryInputs = cppCompileActionBuilder.buildMandatoryInputs(); + this.allInputs = cppCompileActionBuilder.buildAllInputs(); + } + + @Override + public Iterable<CppCompileAction> generateActionForInputArtifacts( + Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner) { + ImmutableList.Builder<CppCompileAction> expandedActions = new ImmutableList.Builder<>(); + for (TreeFileArtifact inputTreeFileArtifact : inputTreeFileArtifacts) { + String outputName = outputTreeFileArtifactName(inputTreeFileArtifact); + TreeFileArtifact outputTreeFileArtifact = ActionInputHelper.treeFileArtifact( + outputTreeArtifact, + new PathFragment(outputName), + artifactOwner); + + expandedActions.add(createAction(inputTreeFileArtifact, outputTreeFileArtifact)); + } + + return expandedActions.build(); + } + + private CppCompileAction createAction( + Artifact sourceTreeFileArtifact, Artifact outputTreeFileArtifact) { + CppCompileActionBuilder builder = new CppCompileActionBuilder(cppCompileActionBuilder); + builder.setSourceFile(sourceTreeFileArtifact); + builder.setOutputs(outputTreeFileArtifact, null); + + CcToolchainFeatures.Variables.Builder buildVariables = + new CcToolchainFeatures.Variables.Builder(builder.getVariables()); + buildVariables.overrideStringVariable( + "source_file", sourceTreeFileArtifact.getExecPathString()); + buildVariables.overrideStringVariable( + "output_file", outputTreeFileArtifact.getExecPathString()); + buildVariables.overrideStringVariable( + "output_object_file", outputTreeFileArtifact.getExecPathString()); + + builder.setVariables(buildVariables.build()); + + return builder.build(); + } + + private String outputTreeFileArtifactName(TreeFileArtifact inputTreeFileArtifact) { + String outputName = FileSystemUtils.removeExtension( + inputTreeFileArtifact.getParentRelativePath().getPathString()); + for (ArtifactCategory category : categories) { + outputName = cppConfiguration.getFeatures().getArtifactNameForCategory(category, outputName); + } + return outputName; + } + + @Override + public Artifact getInputTreeArtifact() { + return sourceTreeArtifact; + } + + @Override + public Artifact getOutputTreeArtifact() { + return outputTreeArtifact; + } + + @Override + public ActionOwner getOwner() { + return actionOwner; + } + + @Override + public final String getMnemonic() { + return "CppCompileActionTemplate"; + } + + @Override + public Iterable<Artifact> getMandatoryInputs() { + return NestedSetBuilder.<Artifact>compileOrder() + .add(sourceTreeArtifact) + .addTransitive(mandatoryInputs) + .build(); + } + + @Override + public ImmutableSet<Artifact> getMandatoryOutputs() { + return ImmutableSet.<Artifact>of(); + } + + @Override + public Iterable<Artifact> getTools() { + return ImmutableList.<Artifact>of(); + } + + @Override + public Iterable<Artifact> getInputs() { + return NestedSetBuilder.<Artifact>stableOrder() + .add(sourceTreeArtifact) + .addTransitive(allInputs) + .build(); + } + + @Override + public ImmutableSet<Artifact> getOutputs() { + return ImmutableSet.of(outputTreeArtifact); + } + + @Override + public Iterable<String> getClientEnvironmentVariables() { + return ImmutableList.<String>of(); + } + + @Override + public Artifact getPrimaryInput() { + return sourceTreeArtifact; + } + + @Override + public Artifact getPrimaryOutput() { + return outputTreeArtifact; + } + + @Override + public boolean shouldReportPathPrefixConflict(ActionAnalysisMetadata action) { + return this != action; + } + + @Override + public MiddlemanType getActionType() { + return MiddlemanType.NORMAL; + } + + @Override + public String prettyPrint() { + return String.format( + "CppCompileActionTemplate compiling " + sourceTreeArtifact.getExecPathString()); + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java index 2ad342d..dc17485 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
@@ -183,10 +183,16 @@ } }; - public static final boolean mustProduceDotdFile(String source) { - return !ASSEMBLER.matches(source) - && !PIC_ASSEMBLER.matches(source) - && !CLIF_INPUT_PROTO.matches(source); + public static final boolean mustProduceDotdFile(Artifact source) { + // Sources from TreeArtifacts and TreeFileArtifacts will not generate dotd file. + if (source.isTreeArtifact() || source.hasParent()) { + return false; + } + + String fileName = source.getFilename(); + return !ASSEMBLER.matches(fileName) + && !PIC_ASSEMBLER.matches(fileName) + && !CLIF_INPUT_PROTO.matches(fileName); } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index e4b19dc..5228588 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -584,9 +584,19 @@ ruleContext.getConfiguration().getBinDirectory(ruleContext.getRule().getRepository())); } - static String getArtifactNameForCategory( - RuleContext ruleContext, CcToolchainProvider toolchain, ArtifactCategory category, - String outputName) { + /** + * Returns the corresponding compiled TreeArtifact given the source TreeArtifact. + */ + public static Artifact getCompileOutputTreeArtifact( + RuleContext ruleContext, Artifact sourceTreeArtifact) { + PathFragment objectDir = getObjDirectory(ruleContext.getLabel()); + PathFragment rootRelativePath = sourceTreeArtifact.getRootRelativePath(); + return ruleContext.getTreeArtifact( + objectDir.getRelative(rootRelativePath), sourceTreeArtifact.getRoot()); + } + + static String getArtifactNameForCategory(RuleContext ruleContext, CcToolchainProvider toolchain, + ArtifactCategory category, String outputName) { return toolchain.getFeatures().getArtifactNameForCategory(category, outputName); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index b22e989..ffe5790 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -954,8 +954,11 @@ } private void addObjectFile(LinkerInput input) { + // We skip file extension checks for TreeArtifacts because they represent directory artifacts + // without a file extension. String name = input.getArtifact().getFilename(); - Preconditions.checkArgument(Link.OBJECT_FILETYPES.matches(name), name); + Preconditions.checkArgument( + input.getArtifact().isTreeArtifact() || Link.OBJECT_FILETYPES.matches(name), name); this.objectFiles.add(input); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index 1e18629..7960bac 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -144,9 +144,10 @@ * units, including module files or headers to be parsed or preprocessed. */ public CppModel addCompilationUnitSources( - Iterable<Artifact> sourceFiles, Label sourceLabel, Map<String, String> buildVariables) { + Iterable<Artifact> sourceFiles, Label sourceLabel, Map<String, String> buildVariables, + CppSource.Type type) { for (Artifact sourceFile : sourceFiles) { - this.sourceFiles.add(CppSource.create(sourceFile, sourceLabel, buildVariables)); + this.sourceFiles.add(CppSource.create(sourceFile, sourceLabel, buildVariables, type)); } return this; } @@ -380,7 +381,7 @@ buildVariables.addStringVariable("output_object_file", realOutputFilePath); } - DotdFile dotdFile = CppFileTypes.mustProduceDotdFile(sourceFile.getPath().toString()) + DotdFile dotdFile = CppFileTypes.mustProduceDotdFile(sourceFile) ? Preconditions.checkNotNull(builder.getDotdFile()) : null; // Set dependency_file to enable <object>.d file generation. if (dotdFile != null) { @@ -485,33 +486,61 @@ CppCompileActionBuilder builder = initializeCompileAction(sourceArtifact, sourceLabel); builder.setSemantics(semantics); - - if (CppFileTypes.CPP_HEADER.matches(source.getSource().getExecPath())) { - createHeaderAction(outputName, result, env, builder, - CppFileTypes.mustProduceDotdFile(sourceArtifact.getFilename())); - } else if (CppFileTypes.CLIF_INPUT_PROTO.matches(source.getSource().getExecPath())) { - createClifMatchAction(outputName, result, env, builder); + + if (!sourceArtifact.isTreeArtifact()) { + switch (source.getType()) { + case HEADER: + createHeaderAction(outputName, result, env, builder, + CppFileTypes.mustProduceDotdFile(sourceArtifact)); + break; + case CLIF_INPUT_PROTO: + createClifMatchAction(outputName, result, env, builder); + break; + default: + boolean bitcodeOutput = + featureConfiguration.isEnabled(CppRuleClasses.THIN_LTO) + && CppFileTypes.LTO_SOURCE.matches(sourceArtifact.getFilename()); + createSourceAction( + outputName, + result, + env, + sourceArtifact, + builder, + ArtifactCategory.OBJECT_FILE, + context.getCppModuleMap(), + /*addObject=*/ true, + isCodeCoverageEnabled(), + // The source action does not generate dwo when it has bitcode + // output (since it isn't generating a native object with debug + // info). In that case the LTOBackendAction will generate the dwo. + /*generateDwo=*/ cppConfiguration.useFission() && !bitcodeOutput, + CppFileTypes.mustProduceDotdFile(sourceArtifact), + source.getBuildVariables(), /*addHeaderTokenFile=*/ + false); + break; + } } else { - boolean bitcodeOutput = - featureConfiguration.isEnabled(CppRuleClasses.THIN_LTO) - && CppFileTypes.LTO_SOURCE.matches(sourceArtifact.getFilename()); - createSourceAction( - outputName, - result, - env, - sourceArtifact, - builder, - ArtifactCategory.OBJECT_FILE, - context.getCppModuleMap(), - /*addObject=*/ true, - isCodeCoverageEnabled(), - // The source action does not generate dwo when it has bitcode - // output (since it isn't generating a native object with debug - // info). In that case the LTOBackendAction will generate the dwo. - /*generateDwo=*/ cppConfiguration.useFission() && !bitcodeOutput, - CppFileTypes.mustProduceDotdFile(sourceArtifact.getFilename()), - source.getBuildVariables(), /*addHeaderTokenFile=*/ - false); + switch (source.getType()) { + case HEADER: + Artifact headerTokenFile = + createCompileActionTemplate( + env, + source, + builder, + ImmutableList.of( + ArtifactCategory.GENERATED_HEADER, ArtifactCategory.PROCESSED_HEADER)); + result.addHeaderTokenFile(headerTokenFile); + break; + case SOURCE: + Artifact objectFile = + createCompileActionTemplate( + env, source, builder, ImmutableList.of(ArtifactCategory.OBJECT_FILE)); + result.addObjectFile(objectFile); + break; + default: + throw new IllegalStateException( + "Encountered invalid source types when creating CppCompileActionTemplates"); + } } } @@ -525,7 +554,7 @@ ruleContext, ccToolchain, ArtifactCategory.GENERATED_HEADER, outputName); builder - .setOutputs(ArtifactCategory.PROCESSED_HEADER, outputNameBase, generateDotd) + .setOutputs(ruleContext, ArtifactCategory.PROCESSED_HEADER, outputNameBase, generateDotd) // If we generate pic actions, we prefer the header actions to use the pic artifacts. .setPicMode(this.getGeneratePicActions()); setupCompileBuildVariables( @@ -538,7 +567,7 @@ builder.getContext().getCppModuleMap(), ImmutableMap.<String, String>of()); semantics.finalizeCompileActionBuilder(ruleContext, builder); - CppCompileAction compileAction = builder.build(); + CppCompileAction compileAction = builder.buildAndValidate(ruleContext); env.registerAction(compileAction); Artifact tokenFile = compileAction.getOutputFile(); result.addHeaderTokenFile(tokenFile); @@ -568,7 +597,7 @@ /*addObject=*/ false, /*enableCoverage=*/ false, /*generateDwo=*/ false, - CppFileTypes.mustProduceDotdFile(moduleMapArtifact.getFilename()), + CppFileTypes.mustProduceDotdFile(moduleMapArtifact), ImmutableMap.<String, String>of(), /*addHeaderTokenFile=*/ addHeaderTokenFiles); } @@ -576,7 +605,7 @@ private void createClifMatchAction( String outputName, Builder result, AnalysisEnvironment env, CppCompileActionBuilder builder) { builder - .setOutputs(ArtifactCategory.CLIF_OUTPUT_PROTO, outputName, false) + .setOutputs(ruleContext, ArtifactCategory.CLIF_OUTPUT_PROTO, outputName, false) .setPicMode(false) // The additional headers in a clif action are both mandatory inputs and // need to be include-scanned. @@ -592,7 +621,7 @@ builder.getContext().getCppModuleMap(), /*sourceSpecificBuildVariables=*/ ImmutableMap.<String, String>of()); semantics.finalizeCompileActionBuilder(ruleContext, builder); - CppCompileAction compileAction = builder.build(); + CppCompileAction compileAction = builder.buildAndValidate(ruleContext); env.registerAction(compileAction); Artifact tokenFile = compileAction.getOutputFile(); result.addHeaderTokenFile(tokenFile); @@ -663,7 +692,7 @@ picBuilder.setDwoFile(dwoFile); semantics.finalizeCompileActionBuilder(ruleContext, picBuilder); - CppCompileAction picAction = picBuilder.build(); + CppCompileAction picAction = picBuilder.buildAndValidate(ruleContext); env.registerAction(picAction); if (addHeaderTokenFiles) { result.addHeaderTokenFile(picAction.getOutputFile()); @@ -690,7 +719,7 @@ ruleContext, CppHelper.getArtifactNameForCategory( ruleContext, ccToolchain, outputCategory, outputName)); - builder.setOutputs(outputCategory, outputName, generateDotd); + builder.setOutputs(ruleContext, outputCategory, outputName, generateDotd); String gcnoFileName = CppHelper.getArtifactNameForCategory(ruleContext, ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, outputName); @@ -727,7 +756,7 @@ builder.setDwoFile(noPicDwoFile); semantics.finalizeCompileActionBuilder(ruleContext, builder); - CppCompileAction compileAction = builder.build(); + CppCompileAction compileAction = builder.buildAndValidate(ruleContext); env.registerAction(compileAction); Artifact objectFile = compileAction.getOutputFile(); if (addHeaderTokenFiles) { @@ -751,6 +780,35 @@ } } + private Artifact createCompileActionTemplate(AnalysisEnvironment env, + CppSource source, CppCompileActionBuilder builder, + Iterable<ArtifactCategory> outputCategories) { + Artifact sourceArtifact = source.getSource(); + Artifact outputFiles = CppHelper.getCompileOutputTreeArtifact(ruleContext, sourceArtifact); + // TODO(rduan): Dotd file output is not supported yet. + builder.setOutputs(outputFiles, null); + setupCompileBuildVariables( + builder, + /* usePic=*/ false, + /*ccRelativeName=*/ null, + /*autoFdoImportPath=*/ null, + /*gcnoFile=*/ null, + /*dwoFile=*/ null, + builder.getContext().getCppModuleMap(), + source.getBuildVariables()); + semantics.finalizeCompileActionBuilder(ruleContext, builder); + CppCompileActionTemplate actionTemplate = new CppCompileActionTemplate( + sourceArtifact, + outputFiles, + builder, + cppConfiguration, + outputCategories, + ruleContext.getActionOwner()); + env.registerAction(actionTemplate); + + return outputFiles; + } + String getOutputNameBaseWith(String base, boolean usePic) { return usePic ? CppHelper.getArtifactNameForCategory( @@ -771,7 +829,7 @@ .getPathString(); builder .setPicMode(usePic) - .setOutputs(outputCategory, outputNameBase, generateDotd) + .setOutputs(ruleContext, outputCategory, outputNameBase, generateDotd) .setTempOutputFile(new PathFragment(tempOutputName)); setupCompileBuildVariables( @@ -784,7 +842,7 @@ builder.getContext().getCppModuleMap(), ImmutableMap.<String, String>of()); semantics.finalizeCompileActionBuilder(ruleContext, builder); - CppCompileAction action = builder.build(); + CppCompileAction action = builder.buildAndValidate(ruleContext); env.registerAction(action); if (addObject) { if (usePic) { @@ -818,7 +876,6 @@ maybePicName = CppHelper.getArtifactNameForCategory( ruleContext, ccToolchain, ArtifactCategory.PIC_FILE, maybePicName); } - String linkedName = CppHelper.getArtifactNameForCategory( ruleContext, ccToolchain, linkTargetType.getLinkerOutput(), maybePicName); PathFragment artifactFragment = new PathFragment(ruleContext.getLabel().getName()) @@ -923,7 +980,6 @@ // If the crosstool is configured to select an output artifact, we use that selection. // Otherwise, we use linux defaults. Artifact picArtifact = getLinkedArtifact(picLinkType); - CppLinkAction picAction = newLinkActionBuilder(picArtifact) .addObjectFiles(ccOutputs.getObjectFiles(true)) @@ -1062,9 +1118,8 @@ * inputs, output and dotd file names, compilation context and copts. */ private CppCompileActionBuilder createCompileActionBuilder(Artifact source, Label label) { - CppCompileActionBuilder builder = new CppCompileActionBuilder( - ruleContext, source, label, ccToolchain); - + CppCompileActionBuilder builder = new CppCompileActionBuilder(ruleContext, label, ccToolchain); + builder.setSourceFile(source); builder.setContext(context).addCopts(copts); builder.addEnvironment(ccToolchain.getEnvironment()); return builder; @@ -1080,7 +1135,7 @@ CppCompileActionBuilder picBuilder = new CppCompileActionBuilder(builder); picBuilder .setPicMode(true) - .setOutputs(outputCategory, outputName, generateDotd); + .setOutputs(ruleContext, outputCategory, outputName, generateDotd); return picBuilder; } @@ -1109,7 +1164,7 @@ String outputArtifactNameBase = getOutputNameBaseWith(outputName, usePic); CppCompileActionBuilder dBuilder = new CppCompileActionBuilder(builder); - dBuilder.setOutputs(category, outputArtifactNameBase, generateDotd); + dBuilder.setOutputs(ruleContext, category, outputArtifactNameBase, generateDotd); setupCompileBuildVariables( dBuilder, usePic, @@ -1120,11 +1175,12 @@ builder.getContext().getCppModuleMap(), ImmutableMap.<String, String>of()); semantics.finalizeCompileActionBuilder(ruleContext, dBuilder); - CppCompileAction dAction = dBuilder.build(); + CppCompileAction dAction = dBuilder.buildAndValidate(ruleContext); ruleContext.registerAction(dAction); CppCompileActionBuilder sdBuilder = new CppCompileActionBuilder(builder); - sdBuilder.setOutputs(ArtifactCategory.GENERATED_ASSEMBLY, outputArtifactNameBase, generateDotd); + sdBuilder.setOutputs( + ruleContext, ArtifactCategory.GENERATED_ASSEMBLY, outputArtifactNameBase, generateDotd); setupCompileBuildVariables( sdBuilder, usePic, @@ -1135,7 +1191,7 @@ builder.getContext().getCppModuleMap(), ImmutableMap.<String, String>of()); semantics.finalizeCompileActionBuilder(ruleContext, sdBuilder); - CppCompileAction sdAction = sdBuilder.build(); + CppCompileAction sdAction = sdBuilder.buildAndValidate(ruleContext); ruleContext.registerAction(sdAction); return ImmutableList.of(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSource.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSource.java index 17ec8d0..bc6ff4b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSource.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSource.java
@@ -22,7 +22,17 @@ /** A source file that is an input to a c++ compilation. */ @AutoValue -abstract class CppSource { +public abstract class CppSource { + + /** + * Types of sources. + */ + public enum Type { + SOURCE, + HEADER, + CLIF_INPUT_PROTO, + } + /** * Creates a {@code CppSource}. * @@ -30,9 +40,11 @@ * @param label the label from which this source arises in the build graph * @param buildVariables build variables that should be used specifically in the compilation * of this source + * @param type type of the source file. */ - static CppSource create(Artifact source, Label label, Map<String, String> buildVariables) { - return new AutoValue_CppSource(source, label, buildVariables); + static CppSource create(Artifact source, Label label, Map<String, String> buildVariables, + Type type) { + return new AutoValue_CppSource(source, label, buildVariables, type); } /** @@ -49,4 +61,9 @@ * Returns build variables to be used specifically in the compilation of this source. */ abstract Map<String, String> getBuildVariables(); + + /** + * Returns the type of this source. + */ + abstract Type getType(); }
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 dc42ddd..cf57fd6 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
@@ -29,8 +29,6 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ResourceSet; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -61,6 +59,7 @@ FakeCppCompileAction( ActionOwner owner, + NestedSet<Artifact> allInputs, ImmutableList<String> features, FeatureConfiguration featureConfiguration, CcToolchainFeatures.Variables variables, @@ -75,18 +74,20 @@ Artifact outputFile, PathFragment tempOutputFile, DotdFile dotdFile, - BuildConfiguration configuration, + ImmutableMap<String, String> localShellEnvironment, + boolean codeCoverageEnabled, CppConfiguration cppConfiguration, CppCompilationContext context, Class<? extends CppCompileActionContext> actionContext, ImmutableList<String> copts, Predicate<String> nocopts, - RuleContext ruleContext, + Iterable<IncludeScannable> lipoScannables, + Iterable<Artifact> builtinIncludeFiles, CppSemantics cppSemantics, - CcToolchainProvider ccToolchain, ImmutableMap<String, String> executionInfo) { super( owner, + allInputs, features, featureConfiguration, variables, @@ -103,8 +104,8 @@ null, null, null, - configuration.getLocalShellEnvironment(), - configuration.isCodeCoverageEnabled(), + localShellEnvironment, + codeCoverageEnabled, cppConfiguration, // We only allow inclusion of header files explicitly declared in // "srcs", so we only use declaredIncludeSrcs, not declaredIncludeDirs. @@ -118,15 +119,14 @@ copts, nocopts, VOID_SPECIAL_INPUTS_HANDLER, - ImmutableList.<IncludeScannable>of(), + lipoScannables, ImmutableList.<Artifact>of(), GUID, executionInfo, ImmutableMap.<String, String>of(), CppCompileAction.CPP_COMPILE, - ruleContext, - cppSemantics, - ccToolchain); + builtinIncludeFiles, + cppSemantics); this.tempOutputFile = Preconditions.checkNotNull(tempOutputFile); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java index d4346a3..bb65c34 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java
@@ -47,7 +47,10 @@ break; case OBJECT_FILE: - Preconditions.checkState(Link.OBJECT_FILETYPES.matches(basename)); + // We skip file extension checks for TreeArtifacts because they represent directory + // artifacts without a file extension. + Preconditions.checkState( + artifact.isTreeArtifact() || Link.OBJECT_FILETYPES.matches(basename)); break; default:
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java index 6a18db7..3164f16 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
@@ -15,16 +15,20 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; +import com.google.devtools.build.lib.actions.Actions; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; -import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; +import com.google.devtools.build.lib.analysis.actions.ActionTemplate; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.util.Map; import javax.annotation.Nullable; /** @@ -40,7 +44,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws ActionTemplateExpansionFunctionException, InterruptedException { ActionTemplateExpansionKey key = (ActionTemplateExpansionKey) skyKey.argument(); - SpawnActionTemplate actionTemplate = key.getActionTemplate(); + ActionTemplate actionTemplate = key.getActionTemplate(); // Requests the TreeArtifactValue object for the input TreeArtifact. SkyKey artifactValueKey = ArtifactSkyKey.key(actionTemplate.getInputTreeArtifact(), true); @@ -56,6 +60,9 @@ // Expand the action template using the list of expanded input TreeFileArtifacts. expandedActions = ImmutableList.<Action>copyOf( actionTemplate.generateActionForInputArtifacts(inputTreeFileArtifacts, key)); + // TODO(rduan): Add a check to verify the inputs of expanded actions are subsets of inputs + // of the ActionTemplate. + checkActionAndArtifactConflicts(expandedActions); } catch (ActionConflictException e) { e.reportTo(env.getListener()); throw new ActionTemplateExpansionFunctionException(e); @@ -78,6 +85,18 @@ } } + private static void checkActionAndArtifactConflicts(Iterable<Action> actions) + throws ActionConflictException, ArtifactPrefixConflictException { + Map<Artifact, ActionAnalysisMetadata> generatingActions = + Actions.findAndThrowActionConflict(ImmutableList.<ActionAnalysisMetadata>copyOf(actions)); + Map<ActionAnalysisMetadata, ArtifactPrefixConflictException> artifactPrefixConflictMap = + Actions.findArtifactPrefixConflicts(generatingActions); + + if (!artifactPrefixConflictMap.isEmpty()) { + throw artifactPrefixConflictMap.values().iterator().next(); + } + } + @Nullable @Override public String extractTag(SkyKey skyKey) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java index fa42902..0ac6e60 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
@@ -16,7 +16,7 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; -import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; +import com.google.devtools.build.lib.analysis.actions.ActionTemplate; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.util.Preconditions; @@ -38,22 +38,22 @@ return expandedActions; } - static SkyKey key(SpawnActionTemplate actionTemplate) { + static SkyKey key(ActionTemplate<?> actionTemplate) { return SkyKey.create( SkyFunctions.ACTION_TEMPLATE_EXPANSION, createActionTemplateExpansionKey(actionTemplate)); } static ActionTemplateExpansionKey createActionTemplateExpansionKey( - SpawnActionTemplate actionTemplate) { + ActionTemplate<?> actionTemplate) { return new ActionTemplateExpansionKey(actionTemplate); } static final class ActionTemplateExpansionKey extends ActionLookupKey { - private final SpawnActionTemplate actionTemplate; + private final ActionTemplate<?> actionTemplate; - ActionTemplateExpansionKey(SpawnActionTemplate actionTemplate) { + ActionTemplateExpansionKey(ActionTemplate<?> actionTemplate) { Preconditions.checkNotNull( actionTemplate, "Passed in action template cannot be null: %s", @@ -72,8 +72,8 @@ return actionTemplate.getOwner().getLabel(); } - /** Returns the associated {@link SpawnActionTemplate} */ - SpawnActionTemplate getActionTemplate() { + /** Returns the associated {@link ActionTemplate} */ + ActionTemplate<?> getActionTemplate() { return actionTemplate; }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java index cecc089..fad6603 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -25,7 +25,7 @@ import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.MissingInputFileException; -import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; +import com.google.devtools.build.lib.analysis.actions.ActionTemplate; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; @@ -76,7 +76,7 @@ // If the action is an ActionTemplate, we need to expand the ActionTemplate into concrete // actions, execute those actions in parallel and then aggregate the action execution results. - if (artifact.isTreeArtifact() && actionMetadata instanceof SpawnActionTemplate) { + if (artifact.isTreeArtifact() && actionMetadata instanceof ActionTemplate) { // Create the directory structures for the output TreeArtifact first. try { FileSystemUtils.createDirectoryAndParents(artifact.getPath()); @@ -91,7 +91,7 @@ } return createTreeArtifactValueFromActionTemplate( - (SpawnActionTemplate) actionMetadata, artifact, env); + (ActionTemplate) actionMetadata, artifact, env); } else { Preconditions.checkState( actionMetadata instanceof Action, @@ -118,7 +118,7 @@ } private static TreeArtifactValue createTreeArtifactValueFromActionTemplate( - SpawnActionTemplate actionTemplate, Artifact treeArtifact, Environment env) + ActionTemplate actionTemplate, Artifact treeArtifact, Environment env) throws ArtifactFunctionException, InterruptedException { // Request the list of expanded actions from the ActionTemplate. ActionTemplateExpansionValue expansionValue = (ActionTemplateExpansionValue) env.getValue(