Skip to content

Commit

Permalink
C++: Remove remaining usages of context in compile
Browse files Browse the repository at this point in the history
A new method was added to ActionConstructionContext in order to get a Tree artifact and the API must now take the grep-includes artifact.

RELNOTES:none
PiperOrigin-RevId: 232634667
  • Loading branch information
oquenchil authored and Copybara-Service committed Feb 6, 2019
1 parent c52be2b commit 441fd75
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -710,13 +710,7 @@ public Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot r
return getAnalysisEnvironment().getDerivedArtifact(rootRelativePath, root);
}

/**
* Creates a TreeArtifact under a given root with the given root-relative path.
*
* <p>Verifies that it is in the root-relative directory corresponding to the package of the rule,
* thus ensuring that it doesn't clash with other artifacts generated by other rules using this
* method.
*/
@Override
public SpecialArtifact getTreeArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
Preconditions.checkState(rootRelativePath.startsWith(getPackageDirectory()),
"Output artifact '%s' not under package directory '%s' for target '%s'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
Expand Down Expand Up @@ -59,6 +60,15 @@ public interface ActionConstructionContext {
*/
Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot root);

/**
* Creates a TreeArtifact under a given root with the given root-relative path.
*
* <p>Verifies that it is in the root-relative directory corresponding to the package of the rule,
* thus ensuring that it doesn't clash with other artifacts generated by other rules using this
* method.
*/
SpecialArtifact getTreeArtifact(PathFragment rootRelativePath, ArtifactRoot root);

/**
* Returns an artifact that can be an output of shared actions. Only use when there is no other
* option.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ public CcCompilationContext getCcCompilationContext() {
}
}

private final RuleContext ruleContext;
private final CppSemantics semantics;
private final BuildConfiguration configuration;
private final CppConfiguration cppConfiguration;
Expand Down Expand Up @@ -270,6 +269,7 @@ public CcCompilationContext getCcCompilationContext() {
private final ActionRegistry actionRegistry;
private final ActionConstructionContext actionConstructionContext;
private final Label label;
private final Artifact grepIncludes;

/**
* Creates a CcCompilationHelper.
Expand Down Expand Up @@ -317,7 +317,6 @@ public CcCompilationHelper(
CcToolchainProvider ccToolchain,
FdoContext fdoContext,
BuildConfiguration configuration) {
this.ruleContext = Preconditions.checkNotNull(ruleContext);
this.semantics = Preconditions.checkNotNull(semantics);
this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
this.sourceCategory = Preconditions.checkNotNull(sourceCatagory);
Expand All @@ -336,6 +335,10 @@ public CcCompilationHelper(
actionRegistry = ruleContext;
actionConstructionContext = ruleContext;
label = ruleContext.getLabel();
grepIncludes =
ruleContext.attributes().has("$grep_includes")
? ruleContext.getPrerequisiteArtifact("$grep_includes", Mode.HOST)
: null;
}

/**
Expand Down Expand Up @@ -1370,7 +1373,8 @@ private Artifact createCompileActionTemplate(
boolean usePic) {
SpecialArtifact sourceArtifact = (SpecialArtifact) source.getSource();
SpecialArtifact outputFiles =
CppHelper.getCompileOutputTreeArtifact(ruleContext, sourceArtifact, outputName, usePic);
CppHelper.getCompileOutputTreeArtifact(
actionConstructionContext, label, sourceArtifact, outputName, usePic);
// TODO(rduan): Dotd file output is not supported yet.
builder.setOutputs(outputFiles, /* dotdFile= */ null);
builder.setVariables(
Expand Down Expand Up @@ -1493,7 +1497,8 @@ private static String toPathString(Artifact a) {
*/
private CppCompileActionBuilder initializeCompileAction(Artifact sourceArtifact) {
CppCompileActionBuilder builder =
new CppCompileActionBuilder(ruleContext, ccToolchain, configuration);
new CppCompileActionBuilder(
actionConstructionContext, grepIncludes, ccToolchain, configuration);
builder.setSourceFile(sourceArtifact);
builder.setCcCompilationContext(ccCompilationContext);
builder.setCoptsFilter(coptsFilter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,22 @@ public class CppCompileActionBuilder {
* rule.
*/
public CppCompileActionBuilder(RuleContext ruleContext, CcToolchainProvider ccToolchain) {
this(ruleContext, ccToolchain, ruleContext.getConfiguration());
this(
ruleContext,
ruleContext.attributes().has("$grep_includes")
? ruleContext.getPrerequisiteArtifact("$grep_includes", Mode.HOST)
: null,
ccToolchain,
ruleContext.getConfiguration());
}

/** Creates a builder from a rule and configuration. */
public CppCompileActionBuilder(
RuleContext ruleContext,
ActionConstructionContext actionConstructionContext,
Artifact grepIncludes,
CcToolchainProvider ccToolchain,
BuildConfiguration configuration) {
this(
ruleContext.getActionOwner(),
configuration,
ccToolchain,
ruleContext.attributes().has("$grep_includes")
? ruleContext.getPrerequisiteArtifact("$grep_includes", Mode.HOST)
: null);
this(actionConstructionContext.getActionOwner(), configuration, ccToolchain, grepIncludes);
}

/** Creates a builder from a rule and configuration. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -717,11 +717,13 @@ static Artifact getCompileOutputArtifact(

/** Returns the corresponding compiled TreeArtifact given the source TreeArtifact. */
public static SpecialArtifact getCompileOutputTreeArtifact(
RuleContext ruleContext, Artifact sourceTreeArtifact, String outputName, boolean usePic) {
PathFragment objectDir = getObjDirectory(ruleContext.getLabel(), usePic);

return ruleContext.getTreeArtifact(
objectDir.getRelative(outputName), sourceTreeArtifact.getRoot());
ActionConstructionContext actionConstructionContext,
Label label,
Artifact sourceTreeArtifact,
String outputName,
boolean usePic) {
return actionConstructionContext.getTreeArtifact(
getObjDirectory(label, usePic).getRelative(outputName), sourceTreeArtifact.getRoot());
}

public static String getArtifactNameForCategory(
Expand Down

0 comments on commit 441fd75

Please sign in to comment.