Skip to content

Commit

Permalink
Remove repository_ctx.flag_enabled and --incompatible_linkopts_to_lin…
Browse files Browse the repository at this point in the history
…klibs

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
  • Loading branch information
Wyverald authored and copybara-github committed Aug 24, 2022
1 parent 6047b8d commit 34ce6a2
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -93,13 +91,6 @@
+ " as an argument to the <code>implementation</code> function when you create a"
+ " repository rule.")
public class StarlarkRepositoryContext extends StarlarkBaseExternalContext {
private static final ImmutableList<String> WHITELISTED_REPOS_FOR_FLAG_ENABLED =
ImmutableList.of("@rules_cc", "@bazel_tools");
private static final ImmutableList<String> 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;
Expand Down Expand Up @@ -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<Checksum> originalChecksum, Path path)
throws IOException, InterruptedException {
if (originalChecksum.isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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())
Expand Down
31 changes: 0 additions & 31 deletions src/test/shell/bazel/cc_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
6 changes: 2 additions & 4 deletions tools/cpp/unix_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 34ce6a2

Please sign in to comment.