Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

propagated for cc_library and cc_binary #9267

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@
public final class RuleContext extends TargetContext
implements ActionConstructionContext, ActionRegistry, RuleErrorConsumer {

@Override
public boolean isAllowTagsPropagation() throws InterruptedException {
return this.getAnalysisEnvironment().getSkylarkSemantics().experimentalAllowTagsPropagation();
}

/**
* The configured version of FilesetEntry.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
*/
public interface ActionConstructionContext {

boolean isAllowTagsPropagation() throws InterruptedException;

/** Returns the bin directory for constructed actions. */
ArtifactRoot getBinDirectory();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public Tuple<Object> compile(
boolean disallowNopicOutputs,
Location location,
Environment environment)
throws EvalException {
throws EvalException, InterruptedException {
return compile(
skylarkActionFactoryApi,
skylarkFeatureConfiguration,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,18 @@ public static Map<String, String> getExecutionInfo(Rule rule) {
return ImmutableMap.copyOf(map);
}

/**
* Returns the execution info from the tags declared on the target if tags propagation is allowed.
* These include only some tags {@link #legalExecInfoKeys} as keys with empty values.
*/
public static ImmutableMap<String, String> getExecutionInfo(Rule rule, boolean allowTagsPropagation) {
if (allowTagsPropagation) {
return ImmutableMap.copyOf(getExecutionInfo(rule));
} else {
return ImmutableMap.of();
}
}

/**
* Returns the execution info, obtained from the rule's tags and the execution requirements
* provided. Only supported tags are included into the execution info, see {@link
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
semantics,
featureConfiguration,
ccToolchain,
fdoContext)
fdoContext,
TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.fromCommon(common, /* additionalCopts= */ ImmutableList.of())
.addPrivateHeaders(common.getPrivateHeaders())
.addSources(common.getSources())
Expand Down Expand Up @@ -408,7 +409,8 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
fdoContext,
ruleContext.getConfiguration(),
cppConfiguration,
ruleContext.getSymbolGenerator())
ruleContext.getSymbolGenerator(),
TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.fromCommon(ruleContext, common)
.setGrepIncludes(CppHelper.getGrepIncludes(ruleContext))
.setIsStampingEnabled(AnalysisUtils.isStampingEnabled(ruleContext))
Expand Down Expand Up @@ -714,7 +716,8 @@ public static Pair<CcLinkingOutputs, CcLauncherInfo> createTransitiveLinkingActi
fdoContext,
ruleContext.getConfiguration(),
cppConfiguration,
ruleContext.getSymbolGenerator())
ruleContext.getSymbolGenerator(),
TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.setGrepIncludes(CppHelper.getGrepIncludes(ruleContext))
.setIsStampingEnabled(AnalysisUtils.isStampingEnabled(ruleContext))
.setTestOrTestOnlyTarget(ruleContext.isTestTarget() || ruleContext.isTestOnlyTarget())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ public CcCompilationContext getCcCompilationContext() {

private final CppSemantics semantics;
private final BuildConfiguration configuration;
private final ImmutableMap<String, String> executionInfo;
private final CppConfiguration cppConfiguration;

private final List<Artifact> publicHeaders = new ArrayList<>();
Expand Down Expand Up @@ -276,16 +277,17 @@ public CcCompilationContext getCcCompilationContext() {

/** Creates a CcCompilationHelper that outputs artifacts in a given configuration. */
public CcCompilationHelper(
ActionRegistry actionRegistry,
ActionConstructionContext actionConstructionContext,
Label label,
@Nullable Artifact grepIncludes,
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
SourceCategory sourceCategory,
CcToolchainProvider ccToolchain,
FdoContext fdoContext,
BuildConfiguration buildConfiguration) {
ActionRegistry actionRegistry,
ActionConstructionContext actionConstructionContext,
Label label,
@Nullable Artifact grepIncludes,
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
SourceCategory sourceCategory,
CcToolchainProvider ccToolchain,
FdoContext fdoContext,
BuildConfiguration buildConfiguration,
ImmutableMap<String, String> executionInfo) {
this.semantics = Preconditions.checkNotNull(semantics);
this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
this.sourceCategory = Preconditions.checkNotNull(sourceCategory);
Expand All @@ -304,18 +306,20 @@ public CcCompilationHelper(
this.actionRegistry = Preconditions.checkNotNull(actionRegistry);
this.label = Preconditions.checkNotNull(label);
this.grepIncludes = grepIncludes;
this.executionInfo = executionInfo;
}

/** Creates a CcCompilationHelper for cpp source files. */
public CcCompilationHelper(
ActionRegistry actionRegistry,
ActionConstructionContext actionConstructionContext,
Label label,
@Nullable Artifact grepIncludes,
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
CcToolchainProvider ccToolchain,
FdoContext fdoContext) {
ActionRegistry actionRegistry,
ActionConstructionContext actionConstructionContext,
Label label,
@Nullable Artifact grepIncludes,
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
CcToolchainProvider ccToolchain,
FdoContext fdoContext,
ImmutableMap<String, String> executionInfo) {
this(
actionRegistry,
actionConstructionContext,
Expand All @@ -326,7 +330,8 @@ public CcCompilationHelper(
SourceCategory.CC,
ccToolchain,
fdoContext,
actionConstructionContext.getConfiguration());
actionConstructionContext.getConfiguration(),
executionInfo);
}

/** Sets fields that overlap for cc_library and cc_binary rules. */
Expand Down Expand Up @@ -1548,6 +1553,7 @@ private CppCompileActionBuilder initializeCompileAction(Artifact sourceArtifact)
builder.setCcCompilationContext(ccCompilationContext);
builder.setCoptsFilter(coptsFilter);
builder.setFeatureConfiguration(featureConfiguration);
builder.addExecutionInfo(executionInfo);
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
Expand Down Expand Up @@ -160,7 +161,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
semantics,
featureConfiguration,
ccToolchain,
ccToolchain.getFdoContext())
ccToolchain.getFdoContext(),
TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.addPublicHeaders(common.getHeaders())
.setHeadersCheckingMode(HeadersCheckingMode.STRICT)
.setCodeCoverageEnabled(CcCompilationHelper.isCodeCoverageEnabled(ruleContext))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
Expand Down Expand Up @@ -157,7 +158,8 @@ public static void init(
semantics,
featureConfiguration,
ccToolchain,
fdoContext)
fdoContext,
TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.fromCommon(common, additionalCopts)
.addSources(common.getSources())
.addPrivateHeaders(common.getPrivateHeaders())
Expand All @@ -183,7 +185,8 @@ public static void init(
fdoContext,
ruleContext.getConfiguration(),
ruleContext.getFragment(CppConfiguration.class),
ruleContext.getSymbolGenerator())
ruleContext.getSymbolGenerator(),
TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.fromCommon(ruleContext, common)
.setGrepIncludes(CppHelper.getGrepIncludes(ruleContext))
.setTestOrTestOnlyTarget(ruleContext.isTestOnlyTarget())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionRegistry;
import com.google.devtools.build.lib.actions.Artifact;
Expand Down Expand Up @@ -124,6 +125,7 @@ public CcLinkingOutputs getCcLinkingOutputs() {
private final ActionRegistry actionRegistry;
private final RuleErrorConsumer ruleErrorConsumer;
private final SymbolGenerator<?> symbolGenerator;
private final ImmutableMap<String, String> executionInfo;

private Artifact grepIncludes;
private boolean isStampingEnabled;
Expand All @@ -141,19 +143,21 @@ public CcLinkingOutputs getCcLinkingOutputs() {
* @param ccToolchain the C++ toolchain provider for the build
* @param fdoContext the C++ FDO optimization support provider for the build
* @param configuration the configuration that gives the directory of output artifacts
* @param executionInfo the execution info data associated with a rule
*/
public CcLinkingHelper(
RuleErrorConsumer ruleErrorConsumer,
Label label,
ActionRegistry actionRegistry,
ActionConstructionContext actionConstructionContext,
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
CcToolchainProvider ccToolchain,
FdoContext fdoContext,
BuildConfiguration configuration,
CppConfiguration cppConfiguration,
SymbolGenerator<?> symbolGenerator) {
RuleErrorConsumer ruleErrorConsumer,
Label label,
ActionRegistry actionRegistry,
ActionConstructionContext actionConstructionContext,
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
CcToolchainProvider ccToolchain,
FdoContext fdoContext,
BuildConfiguration configuration,
CppConfiguration cppConfiguration,
SymbolGenerator<?> symbolGenerator,
ImmutableMap<String, String> executionInfo) {
this.semantics = Preconditions.checkNotNull(semantics);
this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
this.ccToolchain = Preconditions.checkNotNull(ccToolchain);
Expand All @@ -165,6 +169,7 @@ public CcLinkingHelper(
this.actionRegistry = actionRegistry;
this.actionConstructionContext = actionConstructionContext;
this.symbolGenerator = symbolGenerator;
this.executionInfo = executionInfo;
}

/** Sets fields that overlap for cc_library and cc_binary rules. */
Expand Down Expand Up @@ -837,7 +842,8 @@ private CppLinkActionBuilder newLinkActionBuilder(
? ccToolchain.getArFiles()
: ccToolchain.getLinkerFiles())
.setLinkArtifactFactory(linkArtifactFactory)
.setUseTestOnlyFlags(useTestOnlyFlags);
.setUseTestOnlyFlags(useTestOnlyFlags)
.addExecutionInfo(executionInfo);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.SkylarkInfo;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ActionConfig;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ArtifactNamePattern;
Expand Down Expand Up @@ -1432,7 +1433,8 @@ public Tuple<Object> createLinkingContextFromCompilationOutputs(
.getActionConstructionContext()
.getConfiguration()
.getFragment(CppConfiguration.class),
((BazelStarlarkContext) starlarkContext).getSymbolGenerator())
((BazelStarlarkContext) starlarkContext).getSymbolGenerator(),
TargetUtils.getExecutionInfo(actions.getRuleContext().getRule(), actions.getRuleContext().isAllowTagsPropagation()))
.setGrepIncludes(convertFromNoneable(grepIncludes, /* defaultValue= */ null))
.addNonCodeLinkerInputs(additionalInputs)
.setShouldCreateStaticLibraries(!disallowStaticLibraries)
Expand Down Expand Up @@ -1527,7 +1529,7 @@ protected Tuple<Object> compile(
SkylarkList<Artifact> headersForClifDoNotUseThisParam,
Location location,
@Nullable Environment environment)
throws EvalException {
throws EvalException, InterruptedException {
if (environment != null) {
CcCommon.checkLocationWhitelisted(
environment.getSemantics(),
Expand Down Expand Up @@ -1572,7 +1574,8 @@ protected Tuple<Object> compile(
getSemantics(),
featureConfiguration.getFeatureConfiguration(),
ccToolchainProvider,
fdoContext)
fdoContext,
TargetUtils.getExecutionInfo(actions.getRuleContext().getRule(), actions.getRuleContext().isAllowTagsPropagation()))
.addPublicHeaders(publicHeaders)
.addPrivateHeaders(privateHeaders)
.addSources(sources)
Expand Down Expand Up @@ -1680,7 +1683,8 @@ protected CcLinkingOutputs link(
fdoContext,
actions.getActionConstructionContext().getConfiguration(),
cppConfiguration,
((BazelStarlarkContext) starlarkContext).getSymbolGenerator())
((BazelStarlarkContext) starlarkContext).getSymbolGenerator(),
TargetUtils.getExecutionInfo(actions.getRuleContext().getRule(), actions.getRuleContext().isAllowTagsPropagation()))
.setGrepIncludes(convertFromNoneable(grepIncludes, /* defaultValue= */ null))
.setLinkingMode(linkDepsStatically ? LinkingMode.STATIC : LinkingMode.DYNAMIC)
.addNonCodeLinkerInputs(additionalInputs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ public Artifact create(
// of them.
private boolean isTestOrTestOnlyTarget;
private boolean isStampingEnabled;
private final Map<String, String> executionInfo = new LinkedHashMap<>();

/**
* Creates a builder that builds {@link CppLinkAction}s.
Expand Down Expand Up @@ -1022,15 +1023,14 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
// 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.
Map<String, String> executionRequirements = new LinkedHashMap<>();

if (featureConfiguration.actionIsConfigured(getActionName())) {
for (String req : featureConfiguration.getToolRequirementsForAction(getActionName())) {
executionRequirements.put(req, "");
executionInfo.put(req, "");
}
}
configuration.modifyExecutionInfo(
executionRequirements, CppLinkAction.getMnemonic(mnemonic, isLtoIndexing));
executionInfo, CppLinkAction.getMnemonic(mnemonic, isLtoIndexing));

if (!isLtoIndexing) {
for (Map.Entry<Linkstamp, Artifact> linkstampEntry : linkstampMap.entrySet()) {
Expand Down Expand Up @@ -1092,7 +1092,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
linkCommandLine,
configuration.getActionEnvironment(),
toolchainEnv,
ImmutableMap.copyOf(executionRequirements),
ImmutableMap.copyOf(executionInfo),
toolchain.getToolPathFragment(Tool.LD, ruleErrorConsumer),
toolchain.getHostSystemName(),
toolchain.getTargetCpu());
Expand Down Expand Up @@ -1582,4 +1582,9 @@ public CppLinkActionBuilder addActionOutput(Artifact output) {
this.linkActionOutputs.add(output);
return this;
}

public CppLinkActionBuilder addExecutionInfo(Map<String, String> executionInfo) {
this.executionInfo.putAll(executionInfo);
return this;
}
}
Loading