Skip to content

Commit

Permalink
Disable cgo by default with unsupported compilers (#3493)
Browse files Browse the repository at this point in the history
MSVC-style compilers are explicitly not supported by cgo. If the
current C/C++ toolchain is detected to be of that type, disable cgo by
default unless explicitly requested.

Without this commit, building the standard library on Windows fails even
with #3491 without forcing use of mingw-gcc, which we don't want to ask
of our Go-only users.
  • Loading branch information
fmeum authored Mar 27, 2023
1 parent a0fb771 commit 2592d81
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 20 deletions.
45 changes: 31 additions & 14 deletions go/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,20 @@ load(
"request_nogo_transition",
)

_COMPILER_OPTIONS_BLACKLIST = {
# cgo requires a gcc/clang style compiler.
# We use a denylist instead of an allowlist:
# - Bazel's auto-detected toolchains used to set the compiler name to "compiler"
# for gcc (fixed in 6.0.0), which defeats the purpose of an allowlist.
# - The compiler name field is free-form and user-defined, so we would have to
# provide a way to override this list.
# TODO: Convert to a denylist once we can assume Bazel 6.0.0 or later and have a
# way for users to extend the list.
_UNSUPPORTED_C_COMPILERS = {
"msvc-cl": None,
"clang-cl": None,
}

_COMPILER_OPTIONS_DENYLIST = {
# cgo parses the error messages from the compiler. It can't handle colors.
# Ignore both variants of the diagnostics color flag.
"-fcolor-diagnostics": None,
Expand All @@ -94,7 +107,7 @@ _COMPILER_OPTIONS_BLACKLIST = {
"-fprofile-arcs": None,
}

_LINKER_OPTIONS_BLACKLIST = {
_LINKER_OPTIONS_DENYLIST = {
"-Wl,--gc-sections": None,
}

Expand All @@ -115,11 +128,11 @@ def _match_option(option, pattern):
else:
return option == pattern

def _filter_options(options, blacklist):
def _filter_options(options, denylist):
return [
option
for option in options
if not any([_match_option(option, pattern) for pattern in blacklist])
if not any([_match_option(option, pattern) for pattern in denylist])
]

def _child_name(go, path, ext, name):
Expand Down Expand Up @@ -574,8 +587,8 @@ go_context_data = rule(
mandatory = True,
providers = [GoStdLib],
),
"_whitelist_function_transition": attr.label(
default = "@bazel_tools//tools/whitelists/function_transition_whitelist",
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
doc = """go_context_data gathers information about the build configuration.
Expand All @@ -590,6 +603,9 @@ def _cgo_context_data_impl(ctx):
# ctx.files._cc_toolchain won't work when cc toolchain resolution
# is switched on.
cc_toolchain = find_cpp_toolchain(ctx)
if cc_toolchain.compiler in _UNSUPPORTED_C_COMPILERS:
return []

feature_configuration = cc_common.configure_features(
ctx = ctx,
cc_toolchain = cc_toolchain,
Expand All @@ -614,7 +630,7 @@ def _cgo_context_data_impl(ctx):
action_name = C_COMPILE_ACTION_NAME,
variables = c_compile_variables,
) + ctx.fragments.cpp.copts + ctx.fragments.cpp.conlyopts,
_COMPILER_OPTIONS_BLACKLIST,
_COMPILER_OPTIONS_DENYLIST,
)
env.update(cc_common.get_environment_variables(
feature_configuration = feature_configuration,
Expand All @@ -632,7 +648,7 @@ def _cgo_context_data_impl(ctx):
action_name = CPP_COMPILE_ACTION_NAME,
variables = cxx_compile_variables,
) + ctx.fragments.cpp.copts + ctx.fragments.cpp.cxxopts,
_COMPILER_OPTIONS_BLACKLIST,
_COMPILER_OPTIONS_DENYLIST,
)
env.update(cc_common.get_environment_variables(
feature_configuration = feature_configuration,
Expand All @@ -650,7 +666,7 @@ def _cgo_context_data_impl(ctx):
action_name = OBJC_COMPILE_ACTION_NAME,
variables = objc_compile_variables,
),
_COMPILER_OPTIONS_BLACKLIST,
_COMPILER_OPTIONS_DENYLIST,
)
env.update(cc_common.get_environment_variables(
feature_configuration = feature_configuration,
Expand All @@ -668,7 +684,7 @@ def _cgo_context_data_impl(ctx):
action_name = OBJCPP_COMPILE_ACTION_NAME,
variables = objcxx_compile_variables,
),
_COMPILER_OPTIONS_BLACKLIST,
_COMPILER_OPTIONS_DENYLIST,
)
env.update(cc_common.get_environment_variables(
feature_configuration = feature_configuration,
Expand All @@ -691,7 +707,7 @@ def _cgo_context_data_impl(ctx):
action_name = CPP_LINK_EXECUTABLE_ACTION_NAME,
variables = ld_executable_variables,
) + ctx.fragments.cpp.linkopts,
_LINKER_OPTIONS_BLACKLIST,
_LINKER_OPTIONS_DENYLIST,
)
env.update(cc_common.get_environment_variables(
feature_configuration = feature_configuration,
Expand Down Expand Up @@ -732,7 +748,7 @@ def _cgo_context_data_impl(ctx):
action_name = CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME,
variables = ld_dynamic_lib_variables,
) + ctx.fragments.cpp.linkopts,
_LINKER_OPTIONS_BLACKLIST,
_LINKER_OPTIONS_DENYLIST,
)

env.update(cc_common.get_environment_variables(
Expand Down Expand Up @@ -783,15 +799,16 @@ cgo_context_data = rule(
},
toolchains = ["@bazel_tools//tools/cpp:toolchain_type"],
fragments = ["apple", "cpp"],
provides = [CgoContextInfo],
doc = """Collects information about the C/C++ toolchain. The C/C++ toolchain
is needed to build cgo code, but is generally optional. Rules can't have
optional toolchains, so instead, we have an optional dependency on this
rule.""",
)

def _cgo_context_data_proxy_impl(ctx):
return [ctx.attr.actual[CgoContextInfo]] if ctx.attr.actual else []
if ctx.attr.actual and CgoContextInfo in ctx.attr.actual:
return [ctx.attr.actual[CgoContextInfo]]
return []

cgo_context_data_proxy = rule(
implementation = _cgo_context_data_proxy_impl,
Expand Down
4 changes: 2 additions & 2 deletions go/private/rules/nogo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ _nogo = rule(
"_cgo_context_data": attr.label(default = "//:cgo_context_data_proxy"),
"_go_config": attr.label(default = "//:go_config"),
"_stdlib": attr.label(default = "//:stdlib"),
"_whitelist_function_transition": attr.label(
default = "@bazel_tools//tools/whitelists/function_transition_whitelist",
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
toolchains = [GO_TOOLCHAIN],
Expand Down
3 changes: 1 addition & 2 deletions go/private/rules/stdlib.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ load(
)
load(
"//go/private:providers.bzl",
"CgoContextInfo",
"GoConfigInfo",
)
load(
Expand All @@ -39,7 +38,7 @@ stdlib = rule(
implementation = _stdlib_impl,
cfg = go_stdlib_transition,
attrs = {
"cgo_context_data": attr.label(providers = [CgoContextInfo]),
"cgo_context_data": attr.label(),
"_go_config": attr.label(
default = "//:go_config",
providers = [GoConfigInfo],
Expand Down
4 changes: 2 additions & 2 deletions tests/core/stdlib/stdlib_files.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ stdlib_files = rule(
providers = [GoStdLib],
cfg = pure_transition, # force recompilation
),
"_whitelist_function_transition": attr.label(
default = "@bazel_tools//tools/whitelists/function_transition_whitelist",
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
)

0 comments on commit 2592d81

Please sign in to comment.