Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make C++ toolchain optional #3390

Merged
merged 5 commits into from
Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,17 @@ tasks:
- tests/core/cgo/generate_imported_dylib.sh
build_targets:
- "//..."
- "--"
- "-//tests/core/cross:darwin_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:linux_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:windows_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/starlark/cgo/..." # Doesn't work before bazel 6
test_targets:
- "//..."
- "-//tests/core/cross:darwin_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:linux_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:windows_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/starlark/cgo/..." # Doesn't work before bazel 6
# Bzlmod tests require Bazel 6+
- "-//tests/core/nogo/bzlmod/..."
ubuntu2004:
Expand All @@ -28,6 +37,10 @@ tasks:
- "--config=incompatible"
build_targets:
- "//..."
- "--"
- "-//tests/core/cross:darwin_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:linux_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:windows_go_cross_cgo" # Doesn't work before bazel 6
test_targets:
- "//..."
debian11_zig_cc:
Expand Down Expand Up @@ -67,6 +80,10 @@ tasks:
- "--host_crosstool_top=@local_config_apple_cc//:toolchain"
build_targets:
- "//..."
- "--"
- "-//tests/core/cross:darwin_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:linux_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:windows_go_cross_cgo" # Doesn't work before bazel 6
test_flags:
- "--apple_crosstool_top=@local_config_apple_cc//:toolchain"
- "--crosstool_top=@local_config_apple_cc//:toolchain"
Expand All @@ -78,6 +95,10 @@ tasks:
- tests/core/cgo/generate_imported_dylib.sh
build_targets:
- "//..."
- "--"
- "-//tests/core/cross:darwin_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:linux_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:windows_go_cross_cgo" # Doesn't work before bazel 6
test_flags:
# Some tests depend on this feature being disabled. However, because it's
# enabled by default in the rbe_ubuntu1604 platform, we cannot simply remove
Expand Down Expand Up @@ -243,6 +264,9 @@ tasks:
- "-//tests/legacy/test_chdir:go_default_test"
- "-//tests/legacy/test_rundir:go_default_test"
- "-//tests/legacy/transitive_data:go_default_test"
- "-//tests/core/cross:darwin_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:linux_go_cross_cgo" # Doesn't work before bazel 6
- "-//tests/core/cross:windows_go_cross_cgo" # Doesn't work before bazel 6
test_flags:
- '--action_env=PATH=C:\tools\msys64\usr\bin;C:\tools\msys64\bin;C:\tools\msys64\mingw64\bin;C:\python3\Scripts\;C:\python3;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\tools\msys64\usr\bin;c:\openjdk\bin;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program Files\CMake\bin;c:\ninja;c:\bazel;c:\buildkite'
# On Windows CI, bazel (bazelisk) needs %LocalAppData% to find the cache directory.
Expand Down
1 change: 1 addition & 0 deletions go/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ bzl_library(
"//go/platform:apple",
"//go/private:go_toolchain",
"//go/private/rules:transition",
"@bazel_features//:features.bzl",
"@bazel_skylib//lib:paths",
"@bazel_skylib//rules:common_settings",
"@bazel_tools//tools/build_defs/cc:action_names.bzl",
Expand Down
18 changes: 14 additions & 4 deletions go/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("@bazel_features//:features.bzl", "bazel_features")
load(
"@bazel_tools//tools/cpp:toolchain_utils.bzl",
"find_cpp_toolchain",
Expand Down Expand Up @@ -643,8 +644,11 @@ def _cgo_context_data_impl(ctx):
# toolchain (to be inputs into actions that need it).
# 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:
if bazel_features.cc.find_cpp_toolchain_has_mandatory_param:
cc_toolchain = find_cpp_toolchain(ctx, mandatory = False)
else:
cc_toolchain = find_cpp_toolchain(ctx)
if not cc_toolchain or cc_toolchain.compiler in _UNSUPPORTED_C_COMPILERS:
return []

feature_configuration = cc_common.configure_features(
Expand Down Expand Up @@ -833,12 +837,18 @@ def _cgo_context_data_impl(ctx):
cgo_context_data = rule(
implementation = _cgo_context_data_impl,
attrs = {
"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"),
"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:optional_current_cc_toolchain" if bazel_features.cc.find_cpp_toolchain_has_mandatory_param else "@bazel_tools//tools/cpp:current_cc_toolchain"),
"_xcode_config": attr.label(
default = "@bazel_tools//tools/osx:current_xcode_config",
),
},
toolchains = ["@bazel_tools//tools/cpp:toolchain_type"],
toolchains = [
# In pure mode, a C++ toolchain isn't needed when transitioning.
# But if we declare a mandatory toolchain dependency here, a cross-compiling C++ toolchain is required at toolchain resolution time.
# So we make this toolchain dependency optional, so that it's only attempted to be looked up if it's actually needed.
illicitonion marked this conversation as resolved.
Show resolved Hide resolved
# Optional toolchain support was added in bazel 6.0.0.
config_common.toolchain_type("@bazel_tools//tools/cpp:toolchain_type", mandatory = False) if hasattr(config_common, "toolchain_type") else "@bazel_tools//tools/cpp:toolchain_type",
illicitonion marked this conversation as resolved.
Show resolved Hide resolved
illicitonion marked this conversation as resolved.
Show resolved Hide resolved
],
fragments = ["apple", "cpp"],
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
Expand Down
2 changes: 2 additions & 0 deletions go/private/mode.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ def _ternary(*values):

def get_mode(ctx, go_toolchain, cgo_context_info, go_config_info):
static = _ternary(go_config_info.static if go_config_info else "off")
if getattr(ctx.attr, "pure", None) == "off" and not cgo_context_info:
fail("{} has pure explicitly set to off, but no C++ toolchain could be found for its platform".format(ctx.label))
pure = _ternary(
"on" if not cgo_context_info else "auto",
go_config_info.pure if go_config_info else "off",
Expand Down
35 changes: 35 additions & 0 deletions go/private/polyfill_bazel_features.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# bazel_features when used from a WORKSPACE rather than bzlmod context requires a two-step set-up (loading bazel_features, then calling a function from inside it).
# rules_go only has one-step set-up, and it would be a breaking change to require a second step.
# Accordingly, we supply a polyfill implementation of bazel_features which is only used when using rules_go from a WORKSPACE file,
# to avoid complicating code in rules_go itself.
# We just implement the checks we've seen we actually need, and hope to delete this completely when we are in a pure-bzlmod world.

_POLYFILL_BAZEL_FEATURES = """bazel_features = struct(
cc = struct(
find_cpp_toolchain_has_mandatory_param = {find_cpp_toolchain_has_mandatory_param},
)
)
"""

def _polyfill_bazel_features_impl(rctx):
# An empty string is treated as a "dev version", which is greater than anything.
bazel_version = native.bazel_version or "999999.999999.999999"
version_parts = bazel_version.split(".")
if len(version_parts) != 3:
fail("invalid Bazel version '{}': got {} dot-separated segments, want 3".format(bazel_version, len(version_parts)))
major_version_int = int(version_parts[0])
minor_version_int = int(version_parts[1])

find_cpp_toolchain_has_mandatory_param = major_version_int > 6 or (major_version_int == 6 and minor_version_int >= 1)

rctx.file("BUILD.bazel", """exports_files(["features.bzl"])
""")
rctx.file("features.bzl", _POLYFILL_BAZEL_FEATURES.format(
find_cpp_toolchain_has_mandatory_param = repr(find_cpp_toolchain_has_mandatory_param),
))

polyfill_bazel_features = repository_rule(
implementation = _polyfill_bazel_features_impl,
# Force reruns on server restarts to keep native.bazel_version up-to-date.
local = True,
)
6 changes: 6 additions & 0 deletions go/private/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# Once nested repositories work, this file should cease to exist.

load("//go/private:common.bzl", "MINIMUM_BAZEL_VERSION")
load("//go/private:polyfill_bazel_features.bzl", "polyfill_bazel_features")
load("//go/private/skylib/lib:versions.bzl", "versions")
load("//go/private:nogo.bzl", "DEFAULT_NOGO", "go_register_nogo")
load("//proto:gogo.bzl", "gogo_special_proto")
Expand Down Expand Up @@ -284,6 +285,11 @@ def go_rules_dependencies(force = False):
nogo = DEFAULT_NOGO,
)

_maybe(
polyfill_bazel_features,
name = "bazel_features",
)

def _maybe(repo_rule, name, **kwargs):
if name not in native.existing_rules():
repo_rule(name = name, **kwargs)
Expand Down
24 changes: 24 additions & 0 deletions tests/core/cross/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,30 @@ go_cross_binary(
target = ":native_bin",
)

# Because pure = "on" on the underlying target, this doesn't actually need cgo (and won't try to use it).
# This target ensures that (from Bazel 6) we don't require a C++ toolchain if we're not actually going to use cgo.
go_cross_binary(
name = "windows_go_cross_cgo",
platform = "@io_bazel_rules_go//go/toolchain:windows_amd64_cgo",
target = ":native_bin",
)

# Because pure = "on" on the underlying target, this doesn't actually need cgo (and won't try to use it).
# This target ensures that (from Bazel 6) we don't require a C++ toolchain if we're not actually going to use cgo.
go_cross_binary(
name = "linux_go_cross_cgo",
platform = "@io_bazel_rules_go//go/toolchain:linux_amd64_cgo",
target = ":native_bin",
)

# Because pure = "on" on the underlying target, this doesn't actually need cgo (and won't try to use it).
# This target ensures that (from Bazel 6) we don't require a C++ toolchain if we're not actually going to use cgo.
go_cross_binary(
name = "darwin_go_cross_cgo",
platform = "@io_bazel_rules_go//go/toolchain:darwin_amd64_cgo",
target = ":native_bin",
)

go_library(
name = "platform_lib",
srcs = select({
Expand Down
25 changes: 25 additions & 0 deletions tests/core/starlark/cgo/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
load("@io_bazel_rules_go//go/private:common.bzl", "GO_TOOLCHAIN")
load(":cgo_test.bzl", "cgo_test_suite")

constraint_value(
name = "os_does_not_exist",
constraint_setting = "@platforms//os:os",
)

# Make a platform we know won't have a C++ toolchain registered for it.
platform(
name = "platform_has_no_cc_toolchain",
constraint_values = [":os_does_not_exist"],
)

# Make a fake Go toolchain for this platform
toolchain(
name = "fake_go_toolchain",
target_compatible_with = [
":os_does_not_exist",
],
toolchain = "@go_sdk//:go_linux_amd64-impl",
toolchain_type = GO_TOOLCHAIN,
)

cgo_test_suite()
43 changes: 43 additions & 0 deletions tests/core/starlark/cgo/cgo_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_cross_binary")

def _missing_cc_toolchain_explicit_pure_off_test(ctx):
env = analysistest.begin(ctx)

asserts.expect_failure(env, "has pure explicitly set to off, but no C++ toolchain could be found for its platform")

return analysistest.end(env)

missing_cc_toolchain_explicit_pure_off_test = analysistest.make(
_missing_cc_toolchain_explicit_pure_off_test,
expect_failure = True,
config_settings = {
"//command_line_option:extra_toolchains": str(Label("//tests/core/starlark/cgo:fake_go_toolchain")),
fmeum marked this conversation as resolved.
Show resolved Hide resolved
},
)

def cgo_test_suite():
go_binary(
name = "cross_impure",
srcs = ["main.go"],
pure = "off",
tags = ["manual"],
)

go_cross_binary(
name = "go_cross_impure_cgo",
platform = ":platform_has_no_cc_toolchain",
target = ":cross_impure",
tags = ["manual"],
)

missing_cc_toolchain_explicit_pure_off_test(
name = "missing_cc_toolchain_explicit_pure_off_test",
target_under_test = ":go_cross_impure_cgo",
)

"""Creates the test targets and test suite for cgo.bzl tests."""
native.test_suite(
name = "cgo_tests",
tests = [":missing_cc_toolchain_explicit_pure_off_test"],
)