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(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
index cb6fe32..5d45da6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
@@ -24,21 +24,17 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
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.actions.Root;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate.OutputPathMapper;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.PathFragment;
-
+import java.util.List;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import java.util.List;
-
/**
* Tests {@link SpawnActionTemplate}.
*/
@@ -292,82 +288,6 @@
}
}
- @Test
- public void testExpandedAction_actionConflicts() throws Exception {
- Artifact inputTreeArtifact = createInputTreeArtifact();
- Artifact outputTreeArtifact = createOutputTreeArtifact();
-
- OutputPathMapper mapper = new OutputPathMapper() {
- @Override
- public PathFragment parentRelativeOutputPath(TreeFileArtifact inputTreeFileArtifact) {
- return new PathFragment("conflict_path");
- }
- };
-
- SpawnActionTemplate actionTemplate = builder(inputTreeArtifact, outputTreeArtifact)
- .setExecutable(new PathFragment("/bin/cp"))
- .setCommandLineTemplate(
- createSimpleCommandLineTemplate(inputTreeArtifact, outputTreeArtifact))
- .setOutputPathMapper(mapper)
- .build(ActionsTestUtil.NULL_ACTION_OWNER);
-
- Iterable<TreeFileArtifact> inputTreeFileArtifacts =
- createInputTreeFileArtifacts(inputTreeArtifact);
-
- try {
- actionTemplate.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ArtifactOwner.NULL_OWNER);
- fail("Expected ActionConflictException");
- } catch (ActionConflictException e) {
- // expected
- }
- }
-
- @Test
- public void testExpandedAction_artifactPrefixConflicts() throws Exception {
- Artifact inputTreeArtifact = createInputTreeArtifact();
- Artifact outputTreeArtifact = createOutputTreeArtifact();
-
- OutputPathMapper mapper = new OutputPathMapper() {
- private int i = 0;
- @Override
- public PathFragment parentRelativeOutputPath(TreeFileArtifact inputTreeFileArtifact) {
- PathFragment path;
- switch (i) {
- case 0:
- path = new PathFragment("path_prefix");
- break;
- case 1:
- path = new PathFragment("path_prefix/conflict");
- break;
- default:
- path = inputTreeFileArtifact.getParentRelativePath();
- }
-
- ++i;
- return path;
- }
- };
-
- SpawnActionTemplate actionTemplate = builder(inputTreeArtifact, outputTreeArtifact)
- .setExecutable(new PathFragment("/bin/cp"))
- .setCommandLineTemplate(
- createSimpleCommandLineTemplate(inputTreeArtifact, outputTreeArtifact))
- .setOutputPathMapper(mapper)
- .build(ActionsTestUtil.NULL_ACTION_OWNER);
-
- Iterable<TreeFileArtifact> inputTreeFileArtifacts =
- createInputTreeFileArtifacts(inputTreeArtifact);
-
- try {
- actionTemplate.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ArtifactOwner.NULL_OWNER);
- fail("Expected ArtifactPrefixConflictException");
- } catch (ArtifactPrefixConflictException e) {
- // expected
- }
- }
-
private SpawnActionTemplate.Builder builder(Artifact inputTreeArtifact,
Artifact outputTreeArtifact) {
return new SpawnActionTemplate.Builder(inputTreeArtifact, outputTreeArtifact);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index cbb3976..307e4a6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.actions.util.TestAction;
import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
+import com.google.devtools.build.lib.analysis.actions.ActionTemplate;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventKind;
@@ -706,7 +707,7 @@
ImmutableList.<Artifact>of(treeFileArtifactB), expectedOutputTreeFileArtifact2);
actionTemplateExpansionFunction = new DummyActionTemplateExpansionFunction(
- ImmutableMultimap.of(
+ ImmutableMultimap.<ActionTemplate<?>, Action>of(
actionTemplate, generateOutputAction,
actionTemplate, noGenerateOutputAction));
@@ -746,7 +747,7 @@
ImmutableList.<Artifact>of(expectedOutputTreeFileArtifact2));
actionTemplateExpansionFunction = new DummyActionTemplateExpansionFunction(
- ImmutableMultimap.of(
+ ImmutableMultimap.<ActionTemplate<?>, Action>of(
actionTemplate, generateOutputAction,
actionTemplate, noGenerateOutputAction));
@@ -791,7 +792,7 @@
ImmutableList.<Artifact>of(expectedOutputTreeFileArtifact2));
actionTemplateExpansionFunction = new DummyActionTemplateExpansionFunction(
- ImmutableMultimap.of(
+ ImmutableMultimap.<ActionTemplate<?>, Action>of(
actionTemplate, generateOutputAction,
actionTemplate, throwingAction));
@@ -835,7 +836,7 @@
ImmutableList.<Artifact>of(expectedOutputTreeFileArtifact2));
actionTemplateExpansionFunction = new DummyActionTemplateExpansionFunction(
- ImmutableMultimap.of(
+ ImmutableMultimap.<ActionTemplate<?>, Action>of(
actionTemplate, throwingAction,
actionTemplate, anotherThrowingAction));
@@ -1179,17 +1180,17 @@
/** A dummy action template expansion function that just returns the injected actions */
private static class DummyActionTemplateExpansionFunction implements SkyFunction {
- private final Multimap<SpawnActionTemplate, Action> actionTemplateToActionMap;
+ private final Multimap<ActionTemplate<?>, Action> actionTemplateToActionMap;
DummyActionTemplateExpansionFunction(
- Multimap<SpawnActionTemplate, Action> actionTemplateToActionMap) {
+ Multimap<ActionTemplate<?>, Action> actionTemplateToActionMap) {
this.actionTemplateToActionMap = actionTemplateToActionMap;
}
@Override
public SkyValue compute(SkyKey skyKey, Environment env) {
ActionTemplateExpansionKey key = (ActionTemplateExpansionKey) skyKey.argument();
- SpawnActionTemplate actionTemplate = key.getActionTemplate();
+ ActionTemplate<?> actionTemplate = key.getActionTemplate();
return new ActionTemplateExpansionValue(
Preconditions.checkNotNull(actionTemplateToActionMap.get(actionTemplate)));
}
