Review comments

.bazelci/presubmit.yml

@@ -18,8 +18,13 @@ tasks:
       - "-//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:

go/private/context.bzl

@@ -644,11 +644,10 @@ 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.
-    find_cpp_toolchain_kwargs = {}
     if bazel_features.cc.find_cpp_toolchain_has_mandatory_param:
-        find_cpp_toolchain_kwargs["mandatory"] = False
-
-    cc_toolchain = find_cpp_toolchain(ctx, **find_cpp_toolchain_kwargs)
+        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 []
 
@@ -848,7 +847,7 @@ cgo_context_data = rule(
         # 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.
         # Optional toolchain support was added in bazel 6.0.0.
-        config_common.toolchain_type("@bazel_tools//tools/cpp:toolchain_type", mandatory = False) if bazel_features.cc.find_cpp_toolchain_has_mandatory_param else "@bazel_tools//tools/cpp:toolchain_type",
+        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",
     ],
     fragments = ["apple", "cpp"],
     doc = """Collects information about the C/C++ toolchain. The C/C++ toolchain

go/private/mode.bzl

@@ -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",

go/private/fake_bazel_features.bzl → go/private/polyfill_bazel_features.bzl

@@ -1,17 +1,17 @@
 # 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 fake implementation of bazel_features which is only used when using rules_go from a WORKSPACE file,
+# 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.
 
-_FAKE_BAZEL_FEATURES = """bazel_features = struct(
+_POLYFILL_BAZEL_FEATURES = """bazel_features = struct(
   cc = struct(
     find_cpp_toolchain_has_mandatory_param = {find_cpp_toolchain_has_mandatory_param},
   )
 )
 """
 
-def _fake_bazel_features_impl(rctx):
+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(".")
@@ -24,12 +24,12 @@ def _fake_bazel_features_impl(rctx):
 
     rctx.file("BUILD.bazel", """exports_files(["features.bzl"])
 """)
-    rctx.file("features.bzl", _FAKE_BAZEL_FEATURES.format(
+    rctx.file("features.bzl", _POLYFILL_BAZEL_FEATURES.format(
         find_cpp_toolchain_has_mandatory_param = repr(find_cpp_toolchain_has_mandatory_param),
     ))
 
-fake_bazel_features = repository_rule(
-    implementation = _fake_bazel_features_impl,
+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,
 )

go/private/repositories.bzl

@@ -15,7 +15,7 @@
 # Once nested repositories work, this file should cease to exist.
 
 load("//go/private:common.bzl", "MINIMUM_BAZEL_VERSION")
-load("//go/private:fake_bazel_features.bzl", "fake_bazel_features")
+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")
@@ -286,7 +286,7 @@ def go_rules_dependencies(force = False):
     )
 
     _maybe(
-        fake_bazel_features,
+        polyfill_bazel_features,
         name = "bazel_features",
     )
 

tests/core/cross/BUILD.bazel

@@ -24,21 +24,6 @@ go_binary(
     deps = [":platform_lib"],
 )
 
-go_binary(
-    name = "linux_cross_impure",
-    srcs = ["main.go"],
-    goarch = "amd64",
-    goos = "linux",
-    pure = "off",
-    deps = [":platform_lib"],
-)
-
-go_cross_binary(
-    name = "linux_go_cross_impure_cgo",
-    platform = "@io_bazel_rules_go//go/toolchain:linux_amd64_cgo",
-    target = ":linux_cross_impure",
-)
-
 go_binary(
     name = "darwin_cross",
     srcs = ["main.go"],

tests/core/starlark/cgo/BUILD.bazel

@@ -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()

tests/core/starlark/cgo/cgo_test.bzl

@@ -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")),
+    },
+)
+
+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"],
+    )