Skip to content

Commit c74ae11

Browse files
Googlercopybara-github
authored andcommitted
Adds a new flag, --incompatible_enable_cc_test_feature which switches from the use of build variables to the feature of the same name.
RELNOTES: Adds a new flag, `--incompatible_enable_cc_test_feature` which switches from the use of build variables to the feature of the same name. PiperOrigin-RevId: 370905935
1 parent 863383a commit c74ae11

File tree

6 files changed

+75
-5
lines changed

6 files changed

+75
-5
lines changed

src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
import static com.google.common.collect.ImmutableList.toImmutableList;
1717
import static com.google.devtools.build.lib.rules.cpp.CppRuleClasses.DYNAMIC_LINKING_MODE;
18+
import static com.google.devtools.build.lib.rules.cpp.CppRuleClasses.IS_CC_TEST_FEATURE_NAME;
19+
import static com.google.devtools.build.lib.rules.cpp.CppRuleClasses.LEGACY_IS_CC_TEST_FEATURE_NAME;
1820
import static com.google.devtools.build.lib.rules.cpp.CppRuleClasses.STATIC_LINKING_MODE;
1921

2022
import com.google.auto.value.AutoValue;
@@ -378,13 +380,19 @@ public static void init(
378380
requestedFeaturesBuilder
379381
.addAll(ruleContext.getFeatures())
380382
.add(linkingMode == Link.LinkingMode.DYNAMIC ? DYNAMIC_LINKING_MODE : STATIC_LINKING_MODE);
383+
ImmutableSet.Builder<String> disabledFeaturesBuilder = new ImmutableSet.Builder<>();
384+
disabledFeaturesBuilder.addAll(ruleContext.getDisabledFeatures());
385+
if (TargetUtils.isTestRule(ruleContext.getRule()) && cppConfiguration.useCcTestFeature()) {
386+
requestedFeaturesBuilder.add(IS_CC_TEST_FEATURE_NAME);
387+
disabledFeaturesBuilder.add(LEGACY_IS_CC_TEST_FEATURE_NAME);
388+
}
381389

382390
FdoContext fdoContext = common.getFdoContext();
383391
FeatureConfiguration featureConfiguration =
384392
CcCommon.configureFeaturesOrReportRuleError(
385393
ruleContext,
386394
requestedFeaturesBuilder.build(),
387-
/* unsupportedFeatures= */ ruleContext.getDisabledFeatures(),
395+
/* unsupportedFeatures= */ disabledFeaturesBuilder.build(),
388396
ccToolchain,
389397
semantics);
390398

src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,10 @@ public boolean useArgsParamsFile() {
374374
return cppOptions.useArgsParamsFile;
375375
}
376376

377+
public boolean useCcTestFeature() {
378+
return cppOptions.enableCcTestFeature;
379+
}
380+
377381
/** Returns whether or not to strip the binaries. */
378382
public boolean shouldStripBinaries() {
379383
return stripBinaries;

src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,20 @@ public Label getPropellerOptimizeLabel() {
948948
+ " https://github.com/bazelbuild/bazel/issues/8706 for details.")
949949
public boolean disableNoCopts;
950950

951+
@Option(
952+
name = "incompatible_enable_cc_test_feature",
953+
defaultValue = "false",
954+
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
955+
effectTags = {OptionEffectTag.ACTION_COMMAND_LINES},
956+
metadataTags = {
957+
OptionMetadataTag.INCOMPATIBLE_CHANGE,
958+
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES,
959+
},
960+
help =
961+
"When enabled, it switches Crosstool to use feature 'is_cc_test' rather than"
962+
+ " the link-time build variable of the same name.")
963+
public boolean enableCcTestFeature;
964+
951965
@Option(
952966
name = "incompatible_load_cc_rules_from_bzl",
953967
defaultValue = "false",

src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,17 @@ public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) {
446446
*/
447447
public static final String NO_GENERATE_DEBUG_SYMBOLS_FEATURE_NAME = "no_generate_debug_symbols";
448448

449+
/**
450+
* A feature which indicates that this target is a test (rather than a binary). This can be used
451+
* to select test-only options.
452+
*/
453+
public static final String IS_CC_TEST_FEATURE_NAME = "is_cc_test";
454+
455+
/**
456+
* A feature which indicates whether we are using the legacy_is_cc_test build variable behavior.
457+
*/
458+
public static final String LEGACY_IS_CC_TEST_FEATURE_NAME = "legacy_is_cc_test";
459+
449460
/** Ancestor for all rules that do include scanning. */
450461
public static final class CcIncludeScanningRule implements RuleDefinition {
451462
@Override

src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,12 @@ public static CcToolchainVariables setupVariables(
142142
buildVariables.addStringVariable(IS_USING_FISSION.getVariableName(), "");
143143
}
144144

145-
if (useTestOnlyFlags) {
146-
buildVariables.addIntegerVariable(IS_CC_TEST.getVariableName(), 1);
147-
} else {
148-
buildVariables.addIntegerVariable(IS_CC_TEST.getVariableName(), 0);
145+
if (!cppConfiguration.useCcTestFeature()) {
146+
if (useTestOnlyFlags) {
147+
buildVariables.addIntegerVariable(IS_CC_TEST.getVariableName(), 1);
148+
} else {
149+
buildVariables.addIntegerVariable(IS_CC_TEST.getVariableName(), 0);
150+
}
149151
}
150152

151153
if (runtimeLibrarySearchDirectories != null) {

src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package com.google.devtools.build.lib.rules.cpp;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18+
import static org.junit.Assert.assertThrows;
1819

1920
import com.google.common.collect.ImmutableList;
2021
import com.google.common.collect.Iterables;
@@ -24,6 +25,7 @@
2425
import com.google.devtools.build.lib.analysis.RuleContext;
2526
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
2627
import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig;
28+
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException;
2729
import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.LibraryToLinkValue;
2830
import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.VariableValue;
2931
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
@@ -473,6 +475,35 @@ public void testIsCcTestLinkActionBuildVariable() throws Exception {
473475
.isFalse();
474476
}
475477

478+
@Test
479+
public void testDisableIsCcTestLinkActionBuildVariable() throws Exception {
480+
// The use of this feature (implied by the flag) replaces the use of the build variable.
481+
// This test ensures the build variable is never *set*.
482+
useConfiguration("--incompatible_enable_cc_test_feature");
483+
484+
scratch.file(
485+
"x/BUILD",
486+
"cc_test(name = 'foo_test', srcs = ['a.cc'])",
487+
"cc_binary(name = 'foo', srcs = ['a.cc'])");
488+
scratch.file("x/a.cc");
489+
490+
ConfiguredTarget testTarget = getConfiguredTarget("//x:foo_test");
491+
CcToolchainVariables testVariables =
492+
getLinkBuildVariables(testTarget, LinkTargetType.EXECUTABLE);
493+
494+
assertThrows(
495+
ExpansionException.class,
496+
() -> testVariables.getVariable(LinkBuildVariables.IS_CC_TEST.getVariableName()));
497+
498+
ConfiguredTarget binaryTarget = getConfiguredTarget("//x:foo");
499+
CcToolchainVariables binaryVariables =
500+
getLinkBuildVariables(binaryTarget, LinkTargetType.EXECUTABLE);
501+
502+
assertThrows(
503+
ExpansionException.class,
504+
() -> binaryVariables.getVariable(LinkBuildVariables.IS_CC_TEST.getVariableName()));
505+
}
506+
476507
@Test
477508
public void testStripBinariesIsEnabledWhenStripModeIsAlwaysNoMatterWhat() throws Exception {
478509
scratch.file("x/BUILD", "cc_binary(name = 'foo', srcs = ['a.cc'])");

0 commit comments

Comments
 (0)