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

Ensure compatibility with incompatible flags close to flipping #3319

Merged
merged 1 commit into from
Oct 14, 2022
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
Ensure compatibility with incompatible flags close to flipping
  • Loading branch information
fmeum committed Oct 14, 2022
commit ef1699a6def914541ea5bf1063bbfbc5dc738915
6 changes: 2 additions & 4 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@ tasks:
shell_commands:
- tests/core/cgo/generate_imported_dylib.sh
build_flags:
- "--incompatible_load_proto_rules_from_bzl"
- "--incompatible_enable_cc_toolchain_resolution"
- "--config=incompatible"
test_flags:
- "--incompatible_load_proto_rules_from_bzl"
- "--incompatible_enable_cc_toolchain_resolution"
- "--config=incompatible"
build_targets:
- "//..."
test_targets:
Expand Down
9 changes: 9 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,12 @@ build:ci --spawn_strategy=standalone
build:ci --genrule_strategy=standalone
test:ci --test_strategy=standalone
test:ci --test_output=errors

# Incompatible flags to test in a dedicated CI pipeline.
build:incompatible --incompatible_load_proto_rules_from_bzl
build:incompatible --incompatible_enable_cc_toolchain_resolution
build:incompatible --incompatible_config_setting_private_default_visibility
build:incompatible --incompatible_enforce_config_setting_visibility
build:incompatible --incompatible_disallow_empty_glob
# Also enable all incompatible flags in go_bazel_test by default.
test:incompatible --test_env=GO_BAZEL_TEST_BAZELFLAGS=--incompatible_load_proto_rules_from_bzl --incompatible_enable_cc_toolchain_resolution --incompatible_config_setting_private_default_visibility --incompatible_enforce_config_setting_visibility --incompatible_disallow_empty_glob
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a meta flag that we could add that means "all the information ones"?

Copy link
Collaborator Author

@fmeum fmeum Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There used to be, but it also flipped a bunch of stuff that breaks everything (and is broken itself). The flag has since between deprecated (or even removed). There are regular downstream tests against some other flag set, but I don't know of a way to get it programmatically.

3 changes: 3 additions & 0 deletions go/platform/list.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ def declare_config_settings():
native.config_setting(
name = goos,
constraint_values = [Label("//go/toolchain:" + goos)],
visibility = ["//visibility:public"],
)
for goarch in GOARCH:
native.config_setting(
name = goarch,
constraint_values = [Label("//go/toolchain:" + goarch)],
visibility = ["//visibility:public"],
)
for goos, goarch in GOOS_GOARCH:
native.config_setting(
Expand All @@ -52,6 +54,7 @@ def declare_config_settings():
Label("//go/toolchain:" + goos),
Label("//go/toolchain:" + goarch),
],
visibility = ["//visibility:public"],
)

# Setting that determines whether cgo is enabled.
Expand Down
17 changes: 2 additions & 15 deletions go/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,10 @@ filegroup(
visibility = ["//visibility:public"],
)

config_setting(
name = "strip-always",
values = {"strip": "always"},
)

config_setting(
name = "strip-sometimes",
values = {"strip": "sometimes"},
)

config_setting(
name = "strip-never",
values = {"strip": "never"},
)

config_setting(
name = "stamp",
values = {"stamp": "true"},
visibility = ["//:__pkg__"],
)

bzl_library(
Expand Down Expand Up @@ -187,6 +173,7 @@ config_setting(
":bootstrap_nogo": "False",
":request_nogo": "True",
},
visibility = ["//visibility:public"],
)

bool_setting(
Expand Down
7 changes: 7 additions & 0 deletions go/private/BUILD.sdk.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -66,27 +66,31 @@ config_setting(
flag_values = {
sdk_version_label: "",
},
visibility = ["//visibility:private"],
)

config_setting(
name = "match_major_version",
flag_values = {
sdk_version_label: "{major_version}",
},
visibility = ["//visibility:private"],
)

config_setting(
name = "match_major_minor_version",
flag_values = {
sdk_version_label: "{major_version}.{minor_version}",
},
visibility = ["//visibility:private"],
)

config_setting(
name = "match_patch_version",
flag_values = {
sdk_version_label: "{major_version}.{minor_version}.{patch_version}",
},
visibility = ["//visibility:private"],
)

# If prerelease version is "", this will be the same as ":match_patch_version", but that's fine since we use match_any in config_setting_group.
Expand All @@ -95,13 +99,15 @@ config_setting(
flag_values = {
sdk_version_label: "{major_version}.{minor_version}.{patch_version}{prerelease_suffix}",
},
visibility = ["//visibility:private"],
)

config_setting(
name = "match_sdk_type",
flag_values = {
sdk_version_label: "{sdk_type}",
},
visibility = ["//visibility:private"],
)

selects.config_setting_group(
Expand All @@ -114,6 +120,7 @@ selects.config_setting_group(
":match_prerelease_version",
":match_sdk_type",
],
visibility = ["//visibility:private"],
)

declare_toolchains(
Expand Down