-
Notifications
You must be signed in to change notification settings - Fork 443
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Remove stirling docker build containers.
Summary: The stirling docker containers for building specific go versions slow down builds especially when deleting the cache often (as was the case during the clang14 upgrade work). This diff removes the need for the build containers, by allowing building go binaries with different versions of the go SDK. To do this, the diff patches rules_go to allow specifying `gosdk` on golang build rules. Then all the stirling container builds are changed to just use go_binary/go_image instead of containerized builds. Note that it required some changes to the hardcoded offsets in some of the tests. I was worried that maybe my changes didn't work fully or there were significant issues with the new way of building things, so I looked into the differences between the binaries. The byte-by-byte differences are summarized here: https://phab.corp.pixielabs.ai/P197. The differences seems to be entirely (other than a few unaccounted for bytes) due to the fact that rules_go strips out certain build information as well as local paths from the built binaries in order to produce the same binary every build. This causes symbol offsets to be slightly offset (anywhere from 90 to 4096 bytes depending on where in the binary the symbol is), hence the changes to the hardcoded offsets in the tests. Test Plan: Ran tests. Relying on jenkins for bpf tests. Reviewers: #stirling, vihang, zasgar, #third_party_approvers Reviewed By: vihang, zasgar, #third_party_approvers Subscribers: oazizi Signed-off-by: James Bartlett <[email protected]> Differential Revision: https://phab.corp.pixielabs.ai/D11586 GitOrigin-RevId: 97f8b97
- Loading branch information
1 parent
0a28353
commit bd748ab
Showing
41 changed files
with
643 additions
and
852 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,335 @@ | ||
| diff --git a/go/private/BUILD.sdk.bazel b/go/private/BUILD.sdk.bazel | ||
| index 50d9127d..a24956c4 100644 | ||
| --- a/go/private/BUILD.sdk.bazel | ||
| +++ b/go/private/BUILD.sdk.bazel | ||
| @@ -62,6 +62,7 @@ declare_toolchains( | ||
| builder = ":builder", | ||
| host = "{goos}_{goarch}", | ||
| sdk = ":go_sdk", | ||
| + is_default_sdk = {is_default_sdk}, | ||
| ) | ||
|
|
||
| filegroup( | ||
| diff --git a/go/private/go_toolchain.bzl b/go/private/go_toolchain.bzl | ||
| index 0b94c9ac..c82b9d3d 100644 | ||
| --- a/go/private/go_toolchain.bzl | ||
| +++ b/go/private/go_toolchain.bzl | ||
| @@ -90,13 +90,23 @@ go_toolchain = rule( | ||
| provides = [platform_common.ToolchainInfo], | ||
| ) | ||
|
|
||
| -def declare_toolchains(host, sdk, builder): | ||
| +def declare_toolchains(host, sdk, builder, is_default_sdk): | ||
| """Declares go_toolchain and toolchain targets for each platform.""" | ||
|
|
||
| + native.constraint_value( | ||
| + name = "this_sdk_version", | ||
| + constraint_setting = "@io_bazel_rules_go//go/toolchain:sdk_version", | ||
| + ) | ||
| + default_sdk_constraint = "@io_bazel_rules_go//go/toolchain:default_sdk" | ||
| + | ||
| # keep in sync with generate_toolchain_names | ||
| host_goos, _, host_goarch = host.partition("_") | ||
| for p in PLATFORMS: | ||
| if p.cgo: | ||
| + native.platform( | ||
| + name = "platform_{}_{}_cgo".format(p.goos, p.goarch), | ||
| + constraint_values = [c for c in p.constraints if c != default_sdk_constraint] + [":this_sdk_version"], | ||
| + ) | ||
| # Don't declare separate toolchains for cgo_on / cgo_off. | ||
| # This is controlled by the cgo_context_data dependency of | ||
| # go_context_data, which is configured using constraint_values. | ||
| @@ -118,6 +128,8 @@ def declare_toolchains(host, sdk, builder): | ||
| ) | ||
| constraints = [c for c in p.constraints if c not in cgo_constraints] | ||
|
|
||
| + should_print = p.goos == "linux" and p.goarch == "amd64" | ||
| + | ||
| go_toolchain( | ||
| name = impl_name, | ||
| goos = p.goos, | ||
| @@ -129,13 +141,32 @@ def declare_toolchains(host, sdk, builder): | ||
| tags = ["manual"], | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
| + if is_default_sdk: | ||
| + native.toolchain( | ||
| + name = toolchain_name, | ||
| + toolchain_type = "@io_bazel_rules_go//go:toolchain", | ||
| + exec_compatible_with = [ | ||
| + "@io_bazel_rules_go//go/toolchain:" + host_goos, | ||
| + "@io_bazel_rules_go//go/toolchain:" + host_goarch, | ||
| + ], | ||
| + target_compatible_with = constraints, | ||
| + toolchain = ":" + impl_name, | ||
| + ) | ||
| + | ||
| + constraints_no_sdk = [c for c in constraints if c != default_sdk_constraint] | ||
| native.toolchain( | ||
| - name = toolchain_name, | ||
| + name = toolchain_name + "_version_constrained", | ||
| toolchain_type = "@io_bazel_rules_go//go:toolchain", | ||
| exec_compatible_with = [ | ||
| "@io_bazel_rules_go//go/toolchain:" + host_goos, | ||
| "@io_bazel_rules_go//go/toolchain:" + host_goarch, | ||
| ], | ||
| - target_compatible_with = constraints, | ||
| + target_compatible_with = constraints_no_sdk + [":this_sdk_version"], | ||
| toolchain = ":" + impl_name, | ||
| ) | ||
| + # Add a platform for this target with this specific SDK version. | ||
| + native.platform( | ||
| + name = "platform_{}_{}".format(p.goos, p.goarch), | ||
| + constraint_values = constraints_no_sdk + ["@io_bazel_rules_go//go/toolchain:cgo_off", ":this_sdk_version"], | ||
| + ) | ||
| + | ||
| diff --git a/go/private/platforms.bzl b/go/private/platforms.bzl | ||
| index 21a62e90..cee6c8b6 100644 | ||
| --- a/go/private/platforms.bzl | ||
| +++ b/go/private/platforms.bzl | ||
| @@ -154,6 +154,7 @@ def _generate_platforms(): | ||
| constraints = [ | ||
| GOOS_CONSTRAINTS[goos], | ||
| GOARCH_CONSTRAINTS[goarch], | ||
| + "@io_bazel_rules_go//go/toolchain:default_sdk", | ||
| ] | ||
| platforms.append(struct( | ||
| name = goos + "_" + goarch, | ||
| @@ -180,4 +181,4 @@ PLATFORMS = _generate_platforms() | ||
|
|
||
| def generate_toolchain_names(): | ||
| # keep in sync with declare_toolchains | ||
| - return ["go_" + p.name for p in PLATFORMS if not p.cgo] | ||
| + return ["go_" + p.name for p in PLATFORMS if not p.cgo] + ["go_" + p.name + "_version_constrained" for p in PLATFORMS if not p.cgo] | ||
| diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl | ||
| index 6499caec..0c235e3b 100644 | ||
| --- a/go/private/rules/transition.bzl | ||
| +++ b/go/private/rules/transition.bzl | ||
| @@ -31,6 +31,7 @@ load( | ||
| "GoArchive", | ||
| "GoLibrary", | ||
| "GoSource", | ||
| + "GoSDK", | ||
| ) | ||
| load( | ||
| "//go/platform:crosstool.bzl", | ||
| @@ -89,7 +90,7 @@ def go_transition_wrapper(kind, transition_kind, name, **kwargs): | ||
| regular rule. This prevents targets from being rebuilt for an alternative | ||
| configuration identical to the default configuration. | ||
| """ | ||
| - transition_keys = ("goos", "goarch", "pure", "static", "msan", "race", "gotags", "linkmode") | ||
| + transition_keys = ("goos", "goarch", "gosdk", "pure", "static", "msan", "race", "gotags", "linkmode") | ||
| need_transition = any([key in kwargs for key in transition_keys]) | ||
| if need_transition: | ||
| transition_kind(name = name, **kwargs) | ||
| @@ -108,6 +109,9 @@ def go_transition_rule(**kwargs): | ||
| default = "auto", | ||
| values = ["auto"] + {goarch: None for _, goarch in GOOS_GOARCH}.keys(), | ||
| ), | ||
| + "gosdk": attr.string( | ||
| + default = "auto", | ||
| + ), | ||
| "pure": attr.string( | ||
| default = "auto", | ||
| values = ["auto", "on", "off"], | ||
| @@ -171,6 +175,7 @@ def _go_transition_impl(settings, attr): | ||
|
|
||
| goos = getattr(attr, "goos", "auto") | ||
| goarch = getattr(attr, "goarch", "auto") | ||
| + gosdk = getattr(attr, "gosdk", "auto") | ||
| crosstool_top = settings.pop("//command_line_option:crosstool_top") | ||
| cpu = settings.pop("//command_line_option:cpu") | ||
| _check_ternary("pure", pure) | ||
| @@ -183,8 +188,13 @@ def _go_transition_impl(settings, attr): | ||
| fail("invalid goos, goarch pair: {}, {}".format(goos, goarch)) | ||
| if cgo and (goos, goarch) not in CGO_GOOS_GOARCH: | ||
| fail('pure is "off" but cgo is not supported on {} {}'.format(goos, goarch)) | ||
| - platform = "@io_bazel_rules_go//go/toolchain:{}_{}{}".format(goos, goarch, "_cgo" if cgo else "") | ||
| + if gosdk != "auto": | ||
| + platform = str(Label(gosdk).relative("//:platform_{}_{}{}".format(goos, goarch, "_cgo" if cgo else ""))) | ||
| + else: | ||
| + platform = "@io_bazel_rules_go//go/toolchain:{}_{}{}".format(goos, goarch, "_cgo" if cgo else "") | ||
| settings["//command_line_option:platforms"] = platform | ||
| + elif gosdk != "auto": | ||
| + fail("must set goos and goarch if gosdk is set") | ||
| else: | ||
| # If not auto, try to detect the platform the inbound crosstool/cpu. | ||
| platform = platform_from_crosstool(crosstool_top, cpu) | ||
| diff --git a/go/private/sdk.bzl b/go/private/sdk.bzl | ||
| index 4189d819..460f0cc7 100644 | ||
| --- a/go/private/sdk.bzl | ||
| +++ b/go/private/sdk.bzl | ||
| @@ -30,17 +30,30 @@ MIN_SUPPORTED_VERSION = (1, 14, 0) | ||
| def _go_host_sdk_impl(ctx): | ||
| goroot = _detect_host_sdk(ctx) | ||
| platform = _detect_sdk_platform(ctx, goroot) | ||
| - _sdk_build_file(ctx, platform) | ||
| + _sdk_build_file(ctx, platform, ctx.attr.is_default_sdk) | ||
| _local_sdk(ctx, goroot) | ||
|
|
||
| _go_host_sdk = repository_rule( | ||
| implementation = _go_host_sdk_impl, | ||
| environ = ["GOROOT"], | ||
| + attrs = { | ||
| + "is_default_sdk": attr.bool(), | ||
| + }, | ||
| ) | ||
|
|
||
| +def _existing_sdk_rules(): | ||
| + sdk_kinds = ("_go_download_sdk", "_go_host_sdk", "_go_local_sdk", "_go_wrap_sdk") | ||
| + existing_rules = native.existing_rules() | ||
| + sdk_rules = [r for r in existing_rules.values() if r["kind"] in sdk_kinds] | ||
| + if len(sdk_rules) == 0 and "go_sdk" in existing_rules: | ||
| + # may be local_repository in bazel_tests. | ||
| + sdk_rules.append(existing_rules["go_sdk"]) | ||
| + return sdk_rules | ||
| + | ||
| def go_host_sdk(name, **kwargs): | ||
| - _go_host_sdk(name = name, **kwargs) | ||
| - _register_toolchains(name) | ||
| + is_default_sdk = len(_existing_sdk_rules()) == 0 | ||
| + _go_host_sdk(name = name, is_default_sdk=is_default_sdk, **kwargs) | ||
| + _register_toolchains(name, is_default_sdk) | ||
|
|
||
| def _go_download_sdk_impl(ctx): | ||
| if not ctx.attr.goos and not ctx.attr.goarch: | ||
| @@ -52,7 +65,7 @@ def _go_download_sdk_impl(ctx): | ||
| fail("goos set but goarch not set") | ||
| goos, goarch = ctx.attr.goos, ctx.attr.goarch | ||
| platform = goos + "_" + goarch | ||
| - _sdk_build_file(ctx, platform) | ||
| + _sdk_build_file(ctx, platform, ctx.attr.is_default_sdk) | ||
|
|
||
| version = ctx.attr.version | ||
| sdks = ctx.attr.sdks | ||
| @@ -122,29 +135,33 @@ _go_download_sdk = repository_rule( | ||
| "urls": attr.string_list(default = ["https://dl.google.com/go/{}"]), | ||
| "version": attr.string(), | ||
| "strip_prefix": attr.string(default = "go"), | ||
| + "is_default_sdk": attr.bool(), | ||
| }, | ||
| ) | ||
|
|
||
| def go_download_sdk(name, **kwargs): | ||
| - _go_download_sdk(name = name, **kwargs) | ||
| - _register_toolchains(name) | ||
| + is_default_sdk = len(_existing_sdk_rules()) == 0 | ||
| + _go_download_sdk(name = name, is_default_sdk=is_default_sdk, **kwargs) | ||
| + _register_toolchains(name, is_default_sdk) | ||
|
|
||
| def _go_local_sdk_impl(ctx): | ||
| goroot = ctx.attr.path | ||
| platform = _detect_sdk_platform(ctx, goroot) | ||
| - _sdk_build_file(ctx, platform) | ||
| + _sdk_build_file(ctx, platform, ctx.attr.is_default_sdk) | ||
| _local_sdk(ctx, goroot) | ||
|
|
||
| _go_local_sdk = repository_rule( | ||
| implementation = _go_local_sdk_impl, | ||
| attrs = { | ||
| "path": attr.string(), | ||
| + "is_default_sdk": attr.bool(), | ||
| }, | ||
| ) | ||
|
|
||
| def go_local_sdk(name, **kwargs): | ||
| - _go_local_sdk(name = name, **kwargs) | ||
| - _register_toolchains(name) | ||
| + is_default_sdk = len(_existing_sdk_rules()) == 0 | ||
| + _go_local_sdk(name = name, is_default_sdk=is_default_sdk, **kwargs) | ||
| + _register_toolchains(name, is_default_sdk) | ||
|
|
||
| def _go_wrap_sdk_impl(ctx): | ||
| if not ctx.attr.root_file and not ctx.attr.root_files: | ||
| @@ -161,7 +178,7 @@ def _go_wrap_sdk_impl(ctx): | ||
| root_file = Label(ctx.attr.root_files[platform]) | ||
| goroot = str(ctx.path(root_file).dirname) | ||
| platform = _detect_sdk_platform(ctx, goroot) | ||
| - _sdk_build_file(ctx, platform) | ||
| + _sdk_build_file(ctx, platform, ctx.attr.is_default_sdk) | ||
| _local_sdk(ctx, goroot) | ||
|
|
||
| _go_wrap_sdk = repository_rule( | ||
| @@ -175,17 +192,20 @@ _go_wrap_sdk = repository_rule( | ||
| mandatory = False, | ||
| doc = "A set of mappings from the host platform to a file in the SDK's root directory", | ||
| ), | ||
| + "is_default_sdk": attr.bool(), | ||
| }, | ||
| ) | ||
|
|
||
| def go_wrap_sdk(name, **kwargs): | ||
| - _go_wrap_sdk(name = name, **kwargs) | ||
| - _register_toolchains(name) | ||
| + is_default_sdk = len(_existing_sdk_rules()) == 0 | ||
| + _go_wrap_sdk(name = name, is_default_sdk=is_default_sdk, **kwargs) | ||
| + _register_toolchains(name, is_default_sdk) | ||
|
|
||
| -def _register_toolchains(repo): | ||
| +def _register_toolchains(repo, is_default_sdk): | ||
| labels = [ | ||
| "@{}//:{}".format(repo, name) | ||
| for name in generate_toolchain_names() | ||
| + if is_default_sdk or "version_constrained" in name | ||
| ] | ||
| native.register_toolchains(*labels) | ||
|
|
||
| @@ -222,7 +242,7 @@ def _local_sdk(ctx, path): | ||
| for entry in ["src", "pkg", "bin", "lib"]: | ||
| ctx.symlink(path + "/" + entry, entry) | ||
|
|
||
| -def _sdk_build_file(ctx, platform): | ||
| +def _sdk_build_file(ctx, platform, is_default_sdk): | ||
| ctx.file("ROOT") | ||
| goos, _, goarch = platform.partition("_") | ||
| ctx.template( | ||
| @@ -234,6 +254,7 @@ def _sdk_build_file(ctx, platform): | ||
| "{goarch}": goarch, | ||
| "{exe}": ".exe" if goos == "windows" else "", | ||
| "{rules_go_repo_name}": Label("//go/private:BUILD.sdk.bazel").workspace_name, | ||
| + "{is_default_sdk}": "True" if is_default_sdk else "False", | ||
| }, | ||
| ) | ||
|
|
||
| @@ -451,12 +472,7 @@ def go_register_toolchains(version = None, nogo = None, go_version = None): | ||
| if not version: | ||
| version = go_version # old name | ||
|
|
||
| - sdk_kinds = ("_go_download_sdk", "_go_host_sdk", "_go_local_sdk", "_go_wrap_sdk") | ||
| - existing_rules = native.existing_rules() | ||
| - sdk_rules = [r for r in existing_rules.values() if r["kind"] in sdk_kinds] | ||
| - if len(sdk_rules) == 0 and "go_sdk" in existing_rules: | ||
| - # may be local_repository in bazel_tests. | ||
| - sdk_rules.append(existing_rules["go_sdk"]) | ||
| + sdk_rules = _existing_sdk_rules() | ||
|
|
||
| if version and len(sdk_rules) > 0: | ||
| fail("go_register_toolchains: version set after go sdk rule declared ({})".format(", ".join([r["name"] for r in sdk_rules]))) | ||
| diff --git a/go/toolchain/toolchains.bzl b/go/toolchain/toolchains.bzl | ||
| index b4e070cf..63565b02 100644 | ||
| --- a/go/toolchain/toolchains.bzl | ||
| +++ b/go/toolchain/toolchains.bzl | ||
| @@ -61,6 +61,11 @@ def declare_constraints(): | ||
| name = "cgo_constraint", | ||
| ) | ||
|
|
||
| + native.constraint_setting( | ||
| + name = "sdk_version", | ||
| + default_constraint_value = ":default_sdk", | ||
| + ) | ||
| + | ||
| native.constraint_value( | ||
| name = "cgo_on", | ||
| constraint_setting = ":cgo_constraint", | ||
| @@ -71,6 +76,11 @@ def declare_constraints(): | ||
| constraint_setting = ":cgo_constraint", | ||
| ) | ||
|
|
||
| + native.constraint_value( | ||
| + name = "default_sdk", | ||
| + constraint_setting = ":sdk_version", | ||
| + ) | ||
| + | ||
| for p in PLATFORMS: | ||
| native.platform( | ||
| name = p.name, |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.