From 34ce6a23f5a2be58bb59661dd5fc9ab586ea1703 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 24 Aug 2022 06:56:20 -0700 Subject: [PATCH] Remove repository_ctx.flag_enabled and --incompatible_linkopts_to_linklibs The incompatible flag was flipped in 2020 (#10905) and is the only flag for which repository_ctx.flag_enabled is used. This CL removes the flag, the Starlark code that checks the flag, and the flag_enabled method itself. PiperOrigin-RevId: 469715344 Change-Id: I0e7198c6d5a99b1bd947c8fda830a2c394eb38b1 --- .../starlark/StarlarkRepositoryContext.java | 45 ------------------- .../semantics/BuildLanguageOptions.java | 14 ------ .../packages/semantics/ConsistencyTest.java | 2 - src/test/shell/bazel/cc_integration_test.sh | 31 ------------- tools/cpp/unix_cc_configure.bzl | 6 +-- 5 files changed, 2 insertions(+), 96 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java index 5728e16926618b..c78f240614b898 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java @@ -16,7 +16,6 @@ import com.github.difflib.patch.PatchFailedException; import com.google.common.base.Ascii; -import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -39,7 +38,6 @@ import com.google.devtools.build.lib.packages.StarlarkInfo; import com.google.devtools.build.lib.packages.StructImpl; import com.google.devtools.build.lib.packages.StructProvider; -import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -93,13 +91,6 @@ + " as an argument to the implementation function when you create a" + " repository rule.") public class StarlarkRepositoryContext extends StarlarkBaseExternalContext { - private static final ImmutableList WHITELISTED_REPOS_FOR_FLAG_ENABLED = - ImmutableList.of("@rules_cc", "@bazel_tools"); - private static final ImmutableList WHITELISTED_PATHS_FOR_FLAG_ENABLED = - ImmutableList.of( - "rules_cc/cc/private/toolchain/unix_cc_configure.bzl", - "bazel_tools/tools/cpp/unix_cc_configure.bzl"); - private final Rule rule; private final PathPackageLocator packageLocator; private final Path workspaceRoot; @@ -1031,42 +1022,6 @@ public StructImpl downloadAndExtract( return downloadResult; } - @StarlarkMethod( - name = "flag_enabled", - doc = - "This method is present temporarily for a migration. It can be used only by a few " - + "whitelisted bzl files embedded in Bazel.", - useStarlarkThread = true, - documented = false, - parameters = { - @Param(name = "flag", doc = "Flag to get the value for."), - }) - public boolean flagEnabled(String flag, StarlarkThread starlarkThread) throws EvalException { - if (WHITELISTED_PATHS_FOR_FLAG_ENABLED.stream() - .noneMatch(x -> !starlarkThread.getCallerLocation().toString().endsWith(x))) { - throw Starlark.errorf( - "flag_enabled() is restricted to: '%s'.", - Joiner.on(", ").join(WHITELISTED_REPOS_FOR_FLAG_ENABLED)); - } - - // This function previously exposed the names of the StarlarkSemantics - // options, which have historically been *almost* the same as the corresponding - // flag names, but for minor accidental differences (e.g. case, plurals, underscores). - // But now that we have decoupled Bazel's Starlarksemantics features from the core - // interpreter, there is no reliable way to look up a semantics key by flag name. - // (For booleans, the key must encode its default value). - // So, for now, we'll special-case this to a single flag. - // Thank goodness (and laurentlb) it is access-restricted to a single .bzl file that we control. - // If this hack is really necessary, we could add logic to BuildLanguageOptions to extract - // values from StarlarkSemantics based on flag name. - switch (flag) { - case "incompatible_linkopts_to_linklibs": - return starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_LINKOPTS_TO_LINKLIBS); - default: - throw Starlark.errorf("flag_enabled: unsupported key: %s", flag); - } - } - private Checksum calculateChecksum(Optional originalChecksum, Path path) throws IOException, InterruptedException { if (originalChecksum.isPresent()) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 25a376377b1d4f..2bdc4c7254c8a2 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -550,17 +550,6 @@ public final class BuildLanguageOptions extends OptionsBase { + "returns a depset instead.") public boolean incompatibleDepsetForLibrariesToLinkGetter; - @Option( - name = "incompatible_linkopts_to_linklibs", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, - effectTags = {OptionEffectTag.ACTION_COMMAND_LINES}, - metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, - help = - "If set to true the default linkopts in the default toolchain are passed as linklibs " - + "instead of linkopts to cc_toolchain_config") - public boolean incompatibleLinkoptsToLinklibs; - @Option( name = "incompatible_java_common_parameters", defaultValue = "true", @@ -698,7 +687,6 @@ public StarlarkSemantics toStarlarkSemantics() { INCOMPATIBLE_DEPSET_FOR_LIBRARIES_TO_LINK_GETTER, incompatibleDepsetForLibrariesToLinkGetter) .setBool(INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API, incompatibleRequireLinkerInputCcApi) - .setBool(INCOMPATIBLE_LINKOPTS_TO_LINKLIBS, incompatibleLinkoptsToLinklibs) .set(MAX_COMPUTATION_STEPS, maxComputationSteps) .set(NESTED_SET_DEPTH_LIMIT, nestedSetDepthLimit) .setBool( @@ -764,8 +752,6 @@ public StarlarkSemantics toStarlarkSemantics() { "+incompatible_do_not_split_linking_cmdline"; public static final String INCOMPATIBLE_JAVA_COMMON_PARAMETERS = "+incompatible_java_common_parameters"; - public static final String INCOMPATIBLE_LINKOPTS_TO_LINKLIBS = - "+incompatible_linkopts_to_linklibs"; public static final String INCOMPATIBLE_NEW_ACTIONS_API = "+incompatible_new_actions_api"; public static final String INCOMPATIBLE_NO_ATTR_LICENSE = "+incompatible_no_attr_license"; public static final String INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT = diff --git a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java index fafe898e439c48..31f5d22cd930bf 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java @@ -141,7 +141,6 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep "--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(), "--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean(), "--incompatible_java_common_parameters=" + rand.nextBoolean(), - "--incompatible_linkopts_to_linklibs=" + rand.nextBoolean(), "--incompatible_new_actions_api=" + rand.nextBoolean(), "--incompatible_no_attr_license=" + rand.nextBoolean(), "--incompatible_no_implicit_file_export=" + rand.nextBoolean(), @@ -193,7 +192,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { BuildLanguageOptions.INCOMPATIBLE_DISALLOW_STRUCT_PROVIDER_SYNTAX, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_DO_NOT_SPLIT_LINKING_CMDLINE, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS, rand.nextBoolean()) - .setBool(BuildLanguageOptions.INCOMPATIBLE_LINKOPTS_TO_LINKLIBS, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_NEW_ACTIONS_API, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_NO_ATTR_LICENSE, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT, rand.nextBoolean()) diff --git a/src/test/shell/bazel/cc_integration_test.sh b/src/test/shell/bazel/cc_integration_test.sh index c50c12f3063d69..f681290fde745a 100755 --- a/src/test/shell/bazel/cc_integration_test.sh +++ b/src/test/shell/bazel/cc_integration_test.sh @@ -1127,37 +1127,6 @@ EOF "could not find 'undeclared inclusion' error message in bazel output" } -function test_incompatible_linkopts_to_linklibs() { - if is_darwin; then - # This only applies to the Unix toolchain. - return 0 - fi - mkdir -p foo - cat << 'EOF' > foo/BUILD -cc_library( - name = "foo", - srcs = ["foo.cc"], -) -EOF - touch foo/foo.cc - - local -r object_file=".*foo.pic.o" - local stdcpp="-lstdc++" - local lm="-lm" - - bazel build --incompatible_linkopts_to_linklibs //foo \ - || fail "Build failed but should have succeeded" - tr -d '\n' < bazel-bin/foo/libfoo.so-2.params > "$TEST_log" - - expect_log "$object_file$stdcpp$lm" - - bazel build --noincompatible_linkopts_to_linklibs //foo \ - || fail "Build failed but should have succeeded" - tr -d '\n' < bazel-bin/foo/libfoo.so-2.params > "$TEST_log" - - expect_log "$stdcpp$lm$object_file" -} - function test_sibling_repository_layout_include_external_repo_output() { mkdir test cat > test/BUILD <<'EOF' diff --git a/tools/cpp/unix_cc_configure.bzl b/tools/cpp/unix_cc_configure.bzl index 45d5648d33185c..f6421f369cdebb 100644 --- a/tools/cpp/unix_cc_configure.bzl +++ b/tools/cpp/unix_cc_configure.bzl @@ -411,10 +411,8 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): ), ":") use_libcpp = darwin or bsd - bazel_linkopts = "-lc++:-lm" if use_libcpp else "-lstdc++:-lm" - bazel_linklibs = "" - if repository_ctx.flag_enabled("incompatible_linkopts_to_linklibs"): - bazel_linkopts, bazel_linklibs = bazel_linklibs, bazel_linkopts + bazel_linklibs = "-lc++:-lm" if use_libcpp else "-lstdc++:-lm" + bazel_linkopts = "" link_opts = split_escaped(get_env_var( repository_ctx,