diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index a4e36cf4ce..51e9cf4b7c 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -31,7 +31,7 @@ tasks: - "--allow_yanked_versions=all" build_targets: - "//..." - - "@go_sdk//..." + - "@go_default_sdk//..." test_targets: - "//..." macos: diff --git a/MODULE.bazel b/MODULE.bazel index 025c98b776..083596b5c2 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -22,9 +22,9 @@ go_sdk.download( name = "go_default_sdk", version = "1.18.3", ) -use_repo(go_sdk, "go_default_sdk_toolchains") +use_repo(go_sdk, "go_toolchains") -register_toolchains("@go_default_sdk_toolchains//:all") +register_toolchains("@go_toolchains//:all") bazel_dep(name = "gazelle", version = "0.27.0") diff --git a/go/private/BUILD.toolchains.bazel b/go/private/BUILD.toolchains.bazel deleted file mode 100644 index 84defdd94a..0000000000 --- a/go/private/BUILD.toolchains.bazel +++ /dev/null @@ -1,16 +0,0 @@ -load("@io_bazel_rules_go//go/private:go_toolchain.bzl", "declare_bazel_toolchains") - -{version_constants} - -package(default_visibility = ["//visibility:public"]) - -declare_bazel_toolchains( - go_toolchain_repo = "@{sdk_repo}", - host_goarch = "{goarch}", - host_goos = "{goos}", - major = MAJOR_VERSION, - minor = MINOR_VERSION, - patch = PATCH_VERSION, - prerelease = PRERELEASE_SUFFIX, - sdk_type = "{sdk_type}", -) diff --git a/go/private/extensions.bzl b/go/private/extensions.bzl index c4b8e65e1f..dd1166ec3b 100644 --- a/go/private/extensions.bzl +++ b/go/private/extensions.bzl @@ -1,9 +1,9 @@ -load("//go/private:sdk.bzl", "go_download_sdk", "go_host_sdk") +load("//go/private:sdk.bzl", "go_download_sdk_rule", "go_host_sdk_rule", "go_multiple_toolchains") load("//go/private:repositories.bzl", "go_rules_dependencies") _download_tag = tag_class( attrs = { - "name": attr.string(mandatory = True), + "name": attr.string(), "goos": attr.string(), "goarch": attr.string(), "sdks": attr.string_list_dict(), @@ -15,40 +15,136 @@ _download_tag = tag_class( _host_tag = tag_class( attrs = { - "name": attr.string(mandatory = True), + "name": attr.string(), "version": attr.string(), }, ) +# This limit can be increased essentially arbitrarily, but doing so will cause a rebuild of all +# targets using any of these toolchains due to the changed repository name. +_MAX_NUM_TOOLCHAINS = 9999 +_TOOLCHAIN_INDEX_PAD_LENGTH = len(str(_MAX_NUM_TOOLCHAINS)) + def _go_sdk_impl(ctx): + multi_version_module = {} + for module in ctx.modules: + if module.name in multi_version_module: + multi_version_module[module.name] = True + else: + multi_version_module[module.name] = False + + toolchains = [] for module in ctx.modules: - for download_tag in module.tags.download: + for index, download_tag in enumerate(module.tags.download): # SDKs without an explicit version are fetched even when not selected by toolchain # resolution. This is acceptable if brought in by the root module, but transitive # dependencies should not slow down the build in this way. if not module.is_root and not download_tag.version: fail("go_sdk.download: version must be specified in non-root module " + module.name) - go_download_sdk( - name = download_tag.name, + + # SDKs with an explicit name are at risk of colliding with those from other modules. + # This is acceptable if brought in by the root module as the user is responsible for any + # conflicts that arise. rules_go itself provides "go_default_sdk", which is used by + # Gazelle to bootstrap itself. + # TODO(https://github.com/bazelbuild/bazel-gazelle/issues/1469): Investigate whether + # Gazelle can use the first user-defined SDK instead to prevent unnecessary downloads. + if (not module.is_root and not module.name == "rules_go") and download_tag.name: + fail("go_sdk.download: name must not be specified in non-root module " + module.name) + + name = download_tag.name or _default_go_sdk_name( + module = module, + multi_version = multi_version_module[module.name], + tag_type = "download", + index = index, + ) + go_download_sdk_rule( + name = name, goos = download_tag.goos, goarch = download_tag.goarch, sdks = download_tag.sdks, urls = download_tag.urls, version = download_tag.version, - register_toolchains = False, ) - for host_tag in module.tags.host: + + toolchains.append(struct( + goos = download_tag.goos, + goarch = download_tag.goarch, + sdk_repo = name, + sdk_type = "remote", + sdk_version = download_tag.version, + )) + + for index, host_tag in enumerate(module.tags.host): # Dependencies can rely on rules_go providing a default remote SDK. They can also # configure a specific version of the SDK to use. However, they should not add a # dependency on the host's Go SDK. if not module.is_root: fail("go_sdk.host: cannot be used in non-root module " + module.name) - go_host_sdk( - name = host_tag.name, + + name = host_tag.name or _default_go_sdk_name( + module = module, + multi_version = multi_version_module[module.name], + tag_type = "host", + index = index, + ) + go_host_sdk_rule( + name = name, version = host_tag.version, - register_toolchains = False, ) + toolchains.append(struct( + goos = "", + goarch = "", + sdk_repo = name, + sdk_type = "host", + sdk_version = host_tag.version, + )) + + if len(toolchains) > _MAX_NUM_TOOLCHAINS: + fail("more than {} go_sdk tags are not supported".format(_MAX_NUM_TOOLCHAINS)) + + # Toolchains in a BUILD file are registered in the order given by name, not in the order they + # are declared: + # https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/packages/Package.java;drc=8e41dce65b97a3d466d6b1e65005abc52a07b90b;l=156 + # We pad with an index that lexicographically sorts in the same order as if these toolchains + # were registered using register_toolchains in their MODULE.bazel files. + go_multiple_toolchains( + name = "go_toolchains", + prefixes = [ + _toolchain_prefix(index, toolchain.sdk_repo) + for index, toolchain in enumerate(toolchains) + ], + geese = [toolchain.goos for toolchain in toolchains], + goarchs = [toolchain.goarch for toolchain in toolchains], + sdk_repos = [toolchain.sdk_repo for toolchain in toolchains], + sdk_types = [toolchain.sdk_type for toolchain in toolchains], + sdk_versions = [toolchain.sdk_version for toolchain in toolchains], + ) + +def _default_go_sdk_name(*, module, multi_version, tag_type, index): + # Keep the version out of the repository name if possible to prevent unnecessary rebuilds when + # it changes. + return "{name}_{version}_{tag_type}_{index}".format( + name = module.name, + version = module.version if multi_version else "", + tag_type = tag_type, + index = index, + ) + +def _toolchain_prefix(index, name): + """Prefixes the given name with the index, padded with zeros to ensure lexicographic sorting. + + Examples: + _toolchain_prefix( 2, "foo") == "_0002_foo_" + _toolchain_prefix(2000, "foo") == "_2000_foo_" + """ + return "_{}_{}_".format(_left_pad_zero(index, _TOOLCHAIN_INDEX_PAD_LENGTH), name) + +def _left_pad_zero(index, length): + if index < 0: + fail("index must be non-negative") + return ("0" * length + str(index))[-length:] + go_sdk = module_extension( implementation = _go_sdk_impl, tag_classes = { diff --git a/go/private/sdk.bzl b/go/private/sdk.bzl index 496ee7e199..5955747ff6 100644 --- a/go/private/sdk.bzl +++ b/go/private/sdk.bzl @@ -30,7 +30,7 @@ def _go_host_sdk_impl(ctx): _sdk_build_file(ctx, platform, version, experiments = ctx.attr.experiments) _local_sdk(ctx, goroot) -_go_host_sdk = repository_rule( +go_host_sdk_rule = repository_rule( implementation = _go_host_sdk_impl, environ = ["GOROOT"], attrs = { @@ -42,7 +42,7 @@ _go_host_sdk = repository_rule( ) def go_host_sdk(name, register_toolchains = True, **kwargs): - _go_host_sdk(name = name, **kwargs) + go_host_sdk_rule(name = name, **kwargs) _go_toolchains( name = name + "_toolchains", sdk_repo = name, @@ -127,7 +127,7 @@ def _go_download_sdk_impl(ctx): } return None -_go_download_sdk = repository_rule( +go_download_sdk_rule = repository_rule( implementation = _go_download_sdk_impl, attrs = { "goos": attr.string(), @@ -142,73 +142,159 @@ _go_download_sdk = repository_rule( }, ) -def _define_version_constants(version): +def _define_version_constants(version, prefix = ""): pv = _parse_version(version) if pv == None or len(pv) < 3: fail("error parsing sdk version: " + version) major, minor, patch = pv[0], pv[1], pv[2] prerelease = pv[3] if len(pv) > 3 else "" return """ -MAJOR_VERSION = "{major}" -MINOR_VERSION = "{minor}" -PATCH_VERSION = "{patch}" -PRERELEASE_SUFFIX = "{prerelease}" +{prefix}MAJOR_VERSION = "{major}" +{prefix}MINOR_VERSION = "{minor}" +{prefix}PATCH_VERSION = "{patch}" +{prefix}PRERELEASE_SUFFIX = "{prerelease}" """.format( + prefix = prefix, major = major, minor = minor, patch = patch, prerelease = prerelease, ) -def _go_toolchains_impl(ctx): - if not ctx.attr.goos and not ctx.attr.goarch: +def _to_constant_name(s): + # Prefix with _ as identifiers are not allowed to start with numbers. + return "_" + "".join([c if c.isalnum() else "_" for c in s.elems()]).upper() + +def go_toolchains_single_definition(ctx, *, prefix, goos, goarch, sdk_repo, sdk_type, sdk_version): + if not goos and not goarch: goos, goarch = _detect_host_platform(ctx) else: - if not ctx.attr.goos: + if not goos: fail("goarch set but goos not set") - if not ctx.attr.goarch: + if not goarch: fail("goos set but goarch not set") - goos, goarch = ctx.attr.goos, ctx.attr.goarch - sdk_repo = ctx.attr.sdk_repo - sdk_type = ctx.attr.sdk_type + chunks = [] + loads = [] + identifier_prefix = _to_constant_name(prefix) # If a sdk_version attribute is provided, use that version. This avoids # eagerly fetching the SDK repository. But if it's not provided, we have # no choice and must load version constants from the version.bzl file that # _sdk_build_file creates. This will trigger an eager fetch. - version = ctx.attr.sdk_version - if version: - version_constants = _define_version_constants(version) + if sdk_version: + chunks.append(_define_version_constants(sdk_version, prefix = identifier_prefix)) else: - version_constants = 'load("@{}//:version.bzl", "MAJOR_VERSION", "MINOR_VERSION", "PATCH_VERSION", "PRERELEASE_SUFFIX")'.format(sdk_repo) + loads.append("""load( + "@{sdk_repo}//:version.bzl", + {identifier_prefix}MAJOR_VERSION = "MAJOR_VERSION", + {identifier_prefix}MINOR_VERSION = "MINOR_VERSION", + {identifier_prefix}PATCH_VERSION = "PATCH_VERSION", + {identifier_prefix}PRERELEASE_SUFFIX = "PRERELEASE_SUFFIX", +) +""".format( + sdk_repo = sdk_repo, + identifier_prefix = identifier_prefix, + )) + + chunks.append("""declare_bazel_toolchains( + prefix = "{prefix}", + go_toolchain_repo = "@{sdk_repo}", + host_goarch = "{goarch}", + host_goos = "{goos}", + major = {identifier_prefix}MAJOR_VERSION, + minor = {identifier_prefix}MINOR_VERSION, + patch = {identifier_prefix}PATCH_VERSION, + prerelease = {identifier_prefix}PRERELEASE_SUFFIX, + sdk_type = "{sdk_type}", +) +""".format( + prefix = prefix, + identifier_prefix = identifier_prefix, + sdk_repo = sdk_repo, + goarch = goarch, + goos = goos, + sdk_type = sdk_type, + )) + + return struct( + loads = loads, + chunks = chunks, + ) - ctx.template( +def go_toolchains_build_file_content( + ctx, + prefixes, + geese, + goarchs, + sdk_repos, + sdk_types, + sdk_versions): + if not _have_same_length(prefixes, geese, goarchs, sdk_repos, sdk_types, sdk_versions): + fail("all lists must have the same length") + + loads = [ + """load("@io_bazel_rules_go//go/private:go_toolchain.bzl", "declare_bazel_toolchains")""", + ] + chunks = [ + """package(default_visibility = ["//visibility:public"])""", + ] + + for i in range(len(geese)): + definition = go_toolchains_single_definition( + ctx, + prefix = prefixes[i], + goos = geese[i], + goarch = goarchs[i], + sdk_repo = sdk_repos[i], + sdk_type = sdk_types[i], + sdk_version = sdk_versions[i], + ) + loads.extend(definition.loads) + chunks.extend(definition.chunks) + + return "\n".join(loads + chunks) + +def _go_multiple_toolchains_impl(ctx): + ctx.file( "BUILD.bazel", - Label("//go/private:BUILD.toolchains.bazel"), + go_toolchains_build_file_content( + ctx, + prefixes = ctx.attr.prefixes, + geese = ctx.attr.geese, + goarchs = ctx.attr.goarchs, + sdk_repos = ctx.attr.sdk_repos, + sdk_types = ctx.attr.sdk_types, + sdk_versions = ctx.attr.sdk_versions, + ), executable = False, - substitutions = { - "{goos}": goos, - "{goarch}": goarch, - "{sdk_repo}": sdk_repo, - "{sdk_type}": sdk_type, - "{version_constants}": version_constants, - }, ) -_go_toolchains = repository_rule( - implementation = _go_toolchains_impl, +go_multiple_toolchains = repository_rule( + implementation = _go_multiple_toolchains_impl, attrs = { - "sdk_repo": attr.string(mandatory = True), - "sdk_type": attr.string(mandatory = True), - "sdk_version": attr.string(), - "goos": attr.string(), - "goarch": attr.string(), + "prefixes": attr.string_list(mandatory = True), + "sdk_repos": attr.string_list(mandatory = True), + "sdk_types": attr.string_list(mandatory = True), + "sdk_versions": attr.string_list(mandatory = True), + "geese": attr.string_list(mandatory = True), + "goarchs": attr.string_list(mandatory = True), }, ) +def _go_toolchains(name, sdk_repo, sdk_type, sdk_version = None, goos = None, goarch = None): + go_multiple_toolchains( + name = name, + prefixes = [""], + geese = [goos or ""], + goarchs = [goarch or ""], + sdk_repos = [sdk_repo], + sdk_types = [sdk_type], + sdk_versions = [sdk_version or ""], + ) + def go_download_sdk(name, register_toolchains = True, **kwargs): - _go_download_sdk(name = name, **kwargs) + go_download_sdk_rule(name = name, **kwargs) _go_toolchains( name = name + "_toolchains", sdk_repo = name, @@ -531,6 +617,11 @@ def _version_string(v): v = v[:-1] return ".".join([str(n) for n in v]) + suffix +def _have_same_length(*lists): + if not lists: + fail("expected at least one list") + return len({len(l): None for l in lists}) == 1 + def go_register_toolchains(version = None, nogo = None, go_version = None, experiments = None): """See /go/toolchains.rst#go-register-toolchains for full documentation.""" if not version: diff --git a/tests/bcr/MODULE.bazel b/tests/bcr/MODULE.bazel index 3f39f26987..151f522e63 100644 --- a/tests/bcr/MODULE.bazel +++ b/tests/bcr/MODULE.bazel @@ -1,6 +1,15 @@ module( name = "rules_go_bcr_tests", # Test that the default SDK is registered by not registering one from the test module. + version = "1.2.3", +) + +# Keep this dep first so that its toolchain takes precedence over the default SDK registered by +# rules_go. +bazel_dep(name = "other_module", version = "") +local_path_override( + module_name = "other_module", + path = "other_module", ) bazel_dep(name = "rules_go", version = "", repo_name = "my_rules_go") @@ -11,19 +20,15 @@ local_path_override( bazel_dep(name = "gazelle", version = "0.26.0") bazel_dep(name = "protobuf", version = "3.19.6") -bazel_dep(name = "other_module", version = "") -local_path_override( - module_name = "other_module", - path = "other_module", -) -# Test that this correctly downloads the SDK by requesting it from the commandline (see presubmit.yml). go_sdk = use_extension("@my_rules_go//go:extensions.bzl", "go_sdk") -go_sdk.download( - name = "go_sdk", - version = "1.17.5", -) -use_repo(go_sdk, "go_sdk") +go_sdk.download(version = "1.19.5") + +# Request an invalid SDK to verify that it isn't fetched since the first tag takes precedence. +go_sdk.host(version = "3.0.0") + +# Bring the default SDK into scope to verify that it exists. +use_repo(go_sdk, "go_default_sdk") go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps") go_deps.module( diff --git a/tests/bcr/other_module/MODULE.bazel b/tests/bcr/other_module/MODULE.bazel index f851b8d08b..f517abae0c 100644 --- a/tests/bcr/other_module/MODULE.bazel +++ b/tests/bcr/other_module/MODULE.bazel @@ -1,3 +1,9 @@ module(name = "other_module") bazel_dep(name = "rules_go", version = "") + +go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk") + +# Request an invalid SDK to verify that it isn't fetched since the test module registers a toolchain +# that takes precedence. +go_sdk.download(version = "3.0.0") diff --git a/tests/core/starlark/BUILD.bazel b/tests/core/starlark/BUILD.bazel index 6d36a25ef9..c26a6bea81 100644 --- a/tests/core/starlark/BUILD.bazel +++ b/tests/core/starlark/BUILD.bazel @@ -1,3 +1,6 @@ load(":common_tests.bzl", "common_test_suite") +load(":sdk_tests.bzl", "sdk_test_suite") common_test_suite() + +sdk_test_suite() diff --git a/tests/core/starlark/sdk_tests.bzl b/tests/core/starlark/sdk_tests.bzl new file mode 100644 index 0000000000..5339aec62b --- /dev/null +++ b/tests/core/starlark/sdk_tests.bzl @@ -0,0 +1,86 @@ +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//go/private:sdk.bzl", "go_toolchains_single_definition") + +def _go_toolchains_single_definition_with_version_test(ctx): + env = unittest.begin(ctx) + + result = go_toolchains_single_definition( + ctx = None, + prefix = "123_prefix_", + goos = "linux", + goarch = "amd64", + sdk_repo = "sdk_repo", + sdk_type = "download", + sdk_version = "1.20.2rc1", + ) + asserts.equals(env, [], result.loads) + asserts.equals(env, [ + """ +_123_PREFIX_MAJOR_VERSION = "1" +_123_PREFIX_MINOR_VERSION = "20" +_123_PREFIX_PATCH_VERSION = "2" +_123_PREFIX_PRERELEASE_SUFFIX = "rc1" +""", + """declare_bazel_toolchains( + prefix = "123_prefix_", + go_toolchain_repo = "@sdk_repo", + host_goarch = "amd64", + host_goos = "linux", + major = _123_PREFIX_MAJOR_VERSION, + minor = _123_PREFIX_MINOR_VERSION, + patch = _123_PREFIX_PATCH_VERSION, + prerelease = _123_PREFIX_PRERELEASE_SUFFIX, + sdk_type = "download", +) +""", + ], result.chunks) + + return unittest.end(env) + +go_toolchains_single_definition_with_version_test = unittest.make(_go_toolchains_single_definition_with_version_test) + +def _go_toolchains_single_definition_without_version_test(ctx): + env = unittest.begin(ctx) + + result = go_toolchains_single_definition( + ctx = None, + prefix = "123_prefix_", + goos = "linux", + goarch = "amd64", + sdk_repo = "sdk_repo", + sdk_type = "download", + sdk_version = None, + ) + asserts.equals(env, ["""load( + "@sdk_repo//:version.bzl", + _123_PREFIX_MAJOR_VERSION = "MAJOR_VERSION", + _123_PREFIX_MINOR_VERSION = "MINOR_VERSION", + _123_PREFIX_PATCH_VERSION = "PATCH_VERSION", + _123_PREFIX_PRERELEASE_SUFFIX = "PRERELEASE_SUFFIX", +) +"""], result.loads) + asserts.equals(env, [ + """declare_bazel_toolchains( + prefix = "123_prefix_", + go_toolchain_repo = "@sdk_repo", + host_goarch = "amd64", + host_goos = "linux", + major = _123_PREFIX_MAJOR_VERSION, + minor = _123_PREFIX_MINOR_VERSION, + patch = _123_PREFIX_PATCH_VERSION, + prerelease = _123_PREFIX_PRERELEASE_SUFFIX, + sdk_type = "download", +) +""", + ], result.chunks) + + return unittest.end(env) + +go_toolchains_single_definition_without_version_test = unittest.make(_go_toolchains_single_definition_without_version_test) + +def sdk_test_suite(): + unittest.suite( + "sdk_tests", + go_toolchains_single_definition_with_version_test, + go_toolchains_single_definition_without_version_test, + )