Skip to content

Commit

Permalink
Add --incompatible_require_ctx_in_configure_features
Browse files Browse the repository at this point in the history
    In order to migrate C++ rules to platforms, we need the access to the C++ configuration fragment from the caller rule in Starlark APIs. All existing APIs have already access to it, but cc_common.configure_features doesn't. This incompatible change adds a ctx argument to configure_features.

    bazelbuild/bazel#7793
    bazelbuild/bazel#6516

    RELNOTES: Added `--incompatible_require_ctx_in_configure_features`, see bazelbuild/bazel#7793 for details.
    PiperOrigin-RevId: 240099411
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 46e2ea0 commit d0d4f72
Show file tree
Hide file tree
Showing 12 changed files with 222 additions and 326 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -857,19 +857,19 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
// according to compilation mode.
if (requestedFeatures.contains(CppRuleClasses.STATIC_LINK_MSVCRT)) {
allRequestedFeaturesBuilder.add(
cppConfiguration.getCompilationMode() == CompilationMode.DBG
toolchain.getCompilationMode() == CompilationMode.DBG
? CppRuleClasses.STATIC_LINK_MSVCRT_DEBUG
: CppRuleClasses.STATIC_LINK_MSVCRT_NO_DEBUG);
} else {
allRequestedFeaturesBuilder.add(
cppConfiguration.getCompilationMode() == CompilationMode.DBG
toolchain.getCompilationMode() == CompilationMode.DBG
? CppRuleClasses.DYNAMIC_LINK_MSVCRT_DEBUG
: CppRuleClasses.DYNAMIC_LINK_MSVCRT_NO_DEBUG);
}

ImmutableList.Builder<String> allFeatures =
new ImmutableList.Builder<String>()
.addAll(ImmutableSet.of(cppConfiguration.getCompilationMode().toString()))
.addAll(ImmutableSet.of(toolchain.getCompilationMode().toString()))
.addAll(
cppConfiguration.disableLegacyCrosstoolFields()
? ImmutableList.of()
Expand All @@ -886,9 +886,7 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
}
}

if (cppConfiguration.fissionIsActiveForCurrentCompilationMode()
&& toolchain.supportsFission()
&& !cppConfiguration.disableLegacyCrosstoolFields()) {
if (toolchain.useFission() && !cppConfiguration.disableLegacyCrosstoolFields()) {
allFeatures.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
}

Expand All @@ -900,7 +898,7 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
}

FdoContext.BranchFdoProfile branchFdoProvider = toolchain.getFdoContext().getBranchFdoProfile();
if (branchFdoProvider != null && cppConfiguration.getCompilationMode() == CompilationMode.OPT) {
if (branchFdoProvider != null && toolchain.getCompilationMode() == CompilationMode.OPT) {
if (branchFdoProvider.isLlvmFdo()
&& !allUnsupportedFeatures.contains(CppRuleClasses.FDO_OPTIMIZE)) {
allFeatures.add(CppRuleClasses.FDO_OPTIMIZE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ public CcToolchainVariables getCompileBuildVariables(
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
/* includes= */ ImmutableList.of(),
userFlagsToIterable(userCompileFlags),
userFlagsToIterable(ccToolchainProvider.getCppConfiguration(), userCompileFlags),
/* cppModuleMap= */ null,
usePic,
/* fakeOutputFile= */ null,
Expand Down Expand Up @@ -260,7 +260,7 @@ public CcToolchainVariables getLinkBuildVariables(
featureConfiguration,
useTestOnlyFlags,
/* isLtoIndexing= */ false,
userFlagsToIterable(userLinkFlags),
userFlagsToIterable(ccToolchainProvider.getCppConfiguration(), userLinkFlags),
/* interfaceLibraryBuilder= */ null,
/* interfaceLibraryOutput= */ null,
/* ltoOutputRootPrefix= */ null,
Expand Down Expand Up @@ -311,14 +311,26 @@ protected ImmutableList<String> asStringImmutableList(Object o) {
}
}

/** Converts an object that represents user flags as either SkylarkList or None into Iterable. */
protected Iterable<String> userFlagsToIterable(Object o) throws EvalException {
if (o instanceof SkylarkList) {
/**
* Converts an object that represents user flags and can be either SkylarkNestedSet , SkylarkList,
* or None into Iterable.
*/
protected Iterable<String> userFlagsToIterable(CppConfiguration cppConfiguration, Object o)
throws EvalException {
if (o instanceof SkylarkNestedSet) {
if (cppConfiguration.disableDepsetInUserFlags()) {
throw new EvalException(
Location.BUILTIN,
"Passing depset into user flags is deprecated (see "
+ "--incompatible_disable_depset_in_cc_user_flags), use list instead.");
}
return asStringNestedSet(o);
} else if (o instanceof SkylarkList) {
return asStringImmutableList(o);
} else if (o instanceof NoneType) {
return ImmutableList.of();
} else {
throw new EvalException(Location.BUILTIN, "Only list is allowed.");
throw new EvalException(Location.BUILTIN, "Only depset and list is allowed.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,21 @@ public boolean usePicForDynamicLibraries(FeatureConfiguration featureConfigurati
|| featureConfiguration.isEnabled(CppRuleClasses.SUPPORTS_PIC);
}

/**
* Deprecated since it uses legacy crosstool fields.
*
* <p>See {link {@link #usePicForDynamicLibraries(FeatureConfiguration)} for docs}
*
* @return
*/
@Deprecated
@Override
public boolean usePicForDynamicLibrariesUsingLegacyFields() {
return forcePic
|| toolchainNeedsPic()
|| FeatureConfiguration.EMPTY.isEnabled(CppRuleClasses.SUPPORTS_PIC);
}

/**
* Returns true if Fission is specified and supported by the CROSSTOOL for the build implied by
* the given configuration and toolchain.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,10 @@ public static String getLegacyCrosstoolFieldErrorMessage(String field) {
+ "migration instructions).";
}

public boolean disableDepsetInUserFlags() {
return cppOptions.disableDepsetInUserFlags;
}

public boolean removeCpuCompilerCcToolchainAttributes() {
return cppOptions.removeCpuCompilerCcToolchainAttributes;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,21 @@ public Label getFdoPrefetchHintsLabel() {
+ "across subpackage boundaries.")
public boolean experimentalIncludesAttributeSubpackageTraversal;

@Option(
name = "incompatible_disable_depset_in_cc_user_flags",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If true, C++ toolchain Starlark API will not accept depset in `user_compile_flags` "
+ "param of `create_compile_variables`, and in `user_link_flags` of "
+ "`create_link_variables`. Use list instead.")
public boolean disableDepsetInUserFlags;

@Option(
name = "experimental_do_not_use_cpu_transformer",
defaultValue = "false",
Expand Down Expand Up @@ -899,6 +914,7 @@ public FragmentOptions getHost() {

host.doNotUseCpuTransformer = doNotUseCpuTransformer;
host.disableGenruleCcToolchainDependency = disableGenruleCcToolchainDependency;
host.disableDepsetInUserFlags = disableDepsetInUserFlags;
host.disableExpandIfAllAvailableInFlagSet = disableExpandIfAllAvailableInFlagSet;
host.disableLegacyCcProvider = disableLegacyCcProvider;
host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes;
Expand Down
Loading

0 comments on commit d0d4f72

Please sign in to comment.