Skip to content

Commit

Permalink
propagated for cc_library and cc_binary
Browse files Browse the repository at this point in the history
Part of #8830

RELNOTES[NEW]: tags: use `--experimental_allow_tags_propagation` flag to propagate tags to the action's execution requirements from cc_library or cc_binary targets. Such tags should start with: `no-`, `requires-`, `supports-`, `block-`, `disable-`, `cpu:`. See #8830 for details.

Closes #9267.

PiperOrigin-RevId: 266353642
  • Loading branch information
ishikhman authored and copybara-github committed Aug 30, 2019
1 parent a32bc59 commit 94c24b3
Show file tree
Hide file tree
Showing 15 changed files with 310 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@
public final class RuleContext extends TargetContext
implements ActionConstructionContext, ActionRegistry, RuleErrorConsumer {

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 @@ -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,24 @@ public static Map<String, String> getExecutionInfo(Rule rule) {
return ImmutableMap.copyOf(map);
}

/**
* Returns the execution info from the tags declared on the target. These include only some tags
* {@link #legalExecInfoKeys} as keys with empty values.
*
* @param rule a rule instance to get tags from
* @param allowTagsPropagation if set to true, tags will be propagated from a target to the
* actions' execution requirements, for more details {@see
* SkylarkSematicOptions#experimentalAllowTagsPropagation}
*/
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,9 @@ 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 +410,9 @@ 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 +718,9 @@ 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 @@ -285,7 +286,8 @@ public CcCompilationHelper(
SourceCategory sourceCategory,
CcToolchainProvider ccToolchain,
FdoContext fdoContext,
BuildConfiguration buildConfiguration) {
BuildConfiguration buildConfiguration,
ImmutableMap<String, String> executionInfo) {
this.semantics = Preconditions.checkNotNull(semantics);
this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
this.sourceCategory = Preconditions.checkNotNull(sourceCategory);
Expand All @@ -304,6 +306,7 @@ public CcCompilationHelper(
this.actionRegistry = Preconditions.checkNotNull(actionRegistry);
this.label = Preconditions.checkNotNull(label);
this.grepIncludes = grepIncludes;
this.executionInfo = Preconditions.checkNotNull(executionInfo);
}

/** Creates a CcCompilationHelper for cpp source files. */
Expand All @@ -315,7 +318,8 @@ public CcCompilationHelper(
CppSemantics semantics,
FeatureConfiguration featureConfiguration,
CcToolchainProvider ccToolchain,
FdoContext fdoContext) {
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,9 @@ 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,9 @@ 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 +186,9 @@ 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,6 +143,7 @@ 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,
Expand All @@ -153,7 +156,8 @@ public CcLinkingHelper(
FdoContext fdoContext,
BuildConfiguration configuration,
CppConfiguration cppConfiguration,
SymbolGenerator<?> symbolGenerator) {
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,10 @@ 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 +1531,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 +1576,10 @@ 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 +1687,10 @@ 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault;
import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.AspectLegalCppSemantics;
import com.google.devtools.build.lib.rules.cpp.CcCommon;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper;
Expand Down Expand Up @@ -300,7 +301,8 @@ private FeatureConfiguration getFeatureConfiguration() {
}

private CcCompilationHelper initializeCompilationHelper(
FeatureConfiguration featureConfiguration, List<TransitiveInfoCollection> deps) {
FeatureConfiguration featureConfiguration, List<TransitiveInfoCollection> deps)
throws InterruptedException {
CcToolchainProvider toolchain = ccToolchain(ruleContext);
CcCompilationHelper helper =
new CcCompilationHelper(
Expand All @@ -311,7 +313,9 @@ private CcCompilationHelper initializeCompilationHelper(
cppSemantics,
featureConfiguration,
toolchain,
toolchain.getFdoContext())
toolchain.getFdoContext(),
TargetUtils.getExecutionInfo(
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.addCcCompilationContexts(CppHelper.getCompilationContextsFromDeps(deps))
.addCcCompilationContexts(
ImmutableList.of(CcCompilationHelper.getStlCcCompilationContext(ruleContext)));
Expand Down Expand Up @@ -344,7 +348,8 @@ private CcCompilationHelper initializeCompilationHelper(
}

private CcLinkingHelper initializeLinkingHelper(
FeatureConfiguration featureConfiguration, ImmutableList<TransitiveInfoCollection> deps) {
FeatureConfiguration featureConfiguration, ImmutableList<TransitiveInfoCollection> deps)
throws InterruptedException {
CcToolchainProvider toolchain = ccToolchain(ruleContext);
CcLinkingHelper helper =
new CcLinkingHelper(
Expand All @@ -358,7 +363,9 @@ private CcLinkingHelper initializeLinkingHelper(
toolchain.getFdoContext(),
ruleContext.getConfiguration(),
ruleContext.getFragment(CppConfiguration.class),
ruleContext.getSymbolGenerator())
ruleContext.getSymbolGenerator(),
TargetUtils.getExecutionInfo(
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.setGrepIncludes(CppHelper.getGrepIncludes(ruleContext))
.setTestOrTestOnlyTarget(ruleContext.isTestOnlyTarget());
helper.addCcLinkingContexts(CppHelper.getLinkingContextsFromDeps(deps));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.CcCommon;
import com.google.devtools.build.lib.rules.cpp.CcCompilationOutputs;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
Expand Down Expand Up @@ -247,7 +248,9 @@ public static NativeDepsRunfiles createNativeDepsAction(
fdoContext,
configuration,
ruleContext.getFragment(CppConfiguration.class),
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
Loading

0 comments on commit 94c24b3

Please sign in to comment.