From 7a9a2f85e0606c2aa50d06518d370cf920418424 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 7 Feb 2023 03:00:23 -0800 Subject: [PATCH] Add a partial Starlark implementation of common.copts_filter. Variable expansion and some checks will be done in Starlark, however CoptsFilter constructor will be exposed to construct an object which is then passed to CcCompilationHelper. PiperOrigin-RevId: 507725876 Change-Id: Ifae7fc54db6e9ea20b1c11b0b47f8ffb078e7456 --- .../google/devtools/build/lib/rules/cpp/BUILD | 3 --- .../devtools/build/lib/rules/cpp/CcCommon.java | 3 +-- .../devtools/build/lib/rules/cpp/CcModule.java | 16 +++++++++++++++- .../build/lib/rules/cpp/CppConfiguration.java | 6 ++++++ .../cpp/CppConfigurationApi.java | 3 +++ .../builtins_bzl/common/cc/cc_binary.bzl | 2 +- .../builtins_bzl/common/cc/cc_helper.bzl | 14 ++++++++++++++ .../builtins_bzl/common/cc/cc_library.bzl | 2 +- 8 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD index 9af7038477bcb9..83baf2c00844dc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD @@ -66,7 +66,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:make_variable_supplier", "//src/main/java/com/google/devtools/build/lib/analysis:package_specification_provider", "//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration", - "//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context", "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment", "//src/main/java/com/google/devtools/build/lib/analysis:starlark/args", "//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_api_provider", @@ -95,14 +94,12 @@ java_library( "//src/main/java/com/google/devtools/build/lib/rules/apple", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value", - "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data", "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp", - "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/go", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index b8d4fa848bacc1..c50a2ea9061daa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -436,7 +436,7 @@ public ImmutableMap getAllMakeVariables() throws ExpansionExcept } /** A filter that removes copts from a c++ compile action according to a nocopts regex. */ - public static final class CoptsFilter implements StarlarkValue { + public static final class CoptsFilter { private final Pattern noCoptsPattern; private final boolean allPasses; @@ -469,7 +469,6 @@ public boolean passesFilter(String flag) { } /** Returns copts filter built from the make variable expanded nocopts attribute. */ - @StarlarkMethod(name = "copts_filter", structField = true, documented = false) public CoptsFilter getCoptsFilter() { return getCoptsFilter(ruleContext); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java index 1bf787e0b418cc..df9335fd740112 100755 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java @@ -84,6 +84,8 @@ import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; import com.google.errorprone.annotations.FormatMethod; import java.util.List; +import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; import javax.annotation.Nullable; import net.starlark.java.annot.Param; import net.starlark.java.annot.ParamType; @@ -2280,6 +2282,7 @@ private static boolean checkObjectsBound(Object... objects) { documented = false, positional = false, named = true, + allowedTypes = {@ParamType(type = String.class), @ParamType(type = NoneType.class)}, defaultValue = "unbound"), @Param( name = "separate_module_headers", @@ -2376,7 +2379,18 @@ public Tuple compile( ImmutableList additionalModuleMaps = asClassImmutableList(additionalModuleMapsNoneable); - CoptsFilter coptsFilter = convertFromNoneable(coptsFilterObject, /* defaultValue= */ null); + String coptsFilterRegex = convertFromNoneable(coptsFilterObject, /* defaultValue= */ null); + CoptsFilter coptsFilter = null; + if (Strings.isNullOrEmpty(coptsFilterRegex)) { + coptsFilter = CoptsFilter.alwaysPasses(); + } else { + try { + coptsFilter = CoptsFilter.fromRegex(Pattern.compile(coptsFilterRegex)); + } catch (PatternSyntaxException e) { + throw Starlark.errorf( + "invalid regular expression '%s': %s", coptsFilterRegex, e.getMessage()); + } + } Object textualHeadersObject = asClassImmutableListOrNestedSet( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index aae1d67b221435..d4b1affdc980d3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -786,6 +786,12 @@ public boolean disableNoCopts() { return cppOptions.disableNoCopts; } + @Override + public boolean disableNocoptsStarlark(StarlarkThread thread) throws EvalException { + CcModule.checkPrivateStarlarkificationAllowlist(thread); + return disableNoCopts(); + } + public boolean loadCcRulesFromBzl() { return cppOptions.loadCcRulesFromBzl; } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java index deed3df1c47a21..5caf565e139c05 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java @@ -203,4 +203,7 @@ boolean fissionActiveForCurrentCompilationModeStarlark(StarlarkThread thread) @StarlarkMethod(name = "share_native_deps", documented = false, useStarlarkThread = true) boolean shareNativeDepsStarlark(StarlarkThread thread) throws EvalException; + + @StarlarkMethod(name = "disable_nocopts", documented = false, useStarlarkThread = true) + boolean disableNocoptsStarlark(StarlarkThread thread) throws EvalException; } diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl index cb60ffe77e0440..9a1bc3c8b759e4 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl @@ -651,7 +651,7 @@ def cc_binary_impl(ctx, additional_linkopts): system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions), private_hdrs = cc_helper.get_private_hdrs(ctx), public_hdrs = cc_helper.get_public_hdrs(ctx), - copts_filter = common.copts_filter, + copts_filter = cc_helper.copts_filter(ctx, additional_make_variable_substitutions), srcs = cc_helper.get_srcs(ctx), compilation_contexts = compilation_context_deps, grep_includes = cc_helper.grep_includes_executable(ctx.attr._grep_includes), diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl index 17b78ff2126a50..bfad5bb726675e 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl @@ -1193,6 +1193,19 @@ def _linker_scripts(ctx): result.append(f) return result +def _copts_filter(ctx, additional_make_variable_substitutions): + nocopts = getattr(ctx.attr, "nocopts", None) + + if nocopts == None or len(nocopts) == 0: + return nocopts + + # Check if nocopts is disabled. + if ctx.fragments.cpp.disable_nocopts(): + fail("This attribute was removed. See https://github.com/bazelbuild/bazel/issues/8706 for details.", attr = "nocopts") + + # Expand nocopts and create CoptsFilter. + return _expand(ctx, nocopts, additional_make_variable_substitutions) + cc_helper = struct( merge_cc_debug_contexts = _merge_cc_debug_contexts, is_code_coverage_enabled = _is_code_coverage_enabled, @@ -1252,4 +1265,5 @@ cc_helper = struct( defines = _defines, local_defines = _local_defines, linker_scripts = _linker_scripts, + copts_filter = _copts_filter, ) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl index 7a3222333d643b..0417ae1cd28b53 100755 --- a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl @@ -58,7 +58,7 @@ def _cc_library_impl(ctx): local_defines = cc_helper.local_defines(ctx, additional_make_variable_substitutions) + cc_helper.get_local_defines_for_runfiles_lookup(ctx), loose_includes = common.loose_include_dirs, system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions), - copts_filter = common.copts_filter, + copts_filter = cc_helper.copts_filter(ctx, additional_make_variable_substitutions), purpose = "cc_library-compile", srcs = cc_helper.get_srcs(ctx), private_hdrs = cc_helper.get_private_hdrs(ctx),