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.

Sign up for GitHub

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

Use configuration flags for //src/conditions:darwin_arm64 #12653

Closed
wants to merge 3 commits into from
Closed

Use configuration flags for //src/conditions:darwin_arm64 #12653

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a01142c
Use configuration flags for //src/conditions:darwin_arm64
thii Dec 7, 2020
4561d6e
Add darwin_arm64 config_setting_group target
thii Dec 7, 2020
3daa32a
Link to the dummy cc toolchain issue
thii Dec 8, 2020
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
21 changes: 20 additions & 1 deletion src/conditions/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
load("@bazel_skylib//lib:selects.bzl", "selects")

filegroup(
name = "srcs",
srcs = glob(["**"]),
Expand Down Expand Up @@ -89,11 +91,28 @@ config_setting(
)

config_setting(
name = "darwin_arm64",
name = "darwin_arm64_constraint",
constraint_values = [
"@platforms//os:macos",
"@platforms//cpu:arm64",
],
)

config_setting(
name = "darwin_arm64_flag",
values = {"cpu": "darwin_arm64"},
Copy link
Member

Choose a reason for hiding this comment

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

Instead of removing the constraint version entirely, use https://github.com/bazelbuild/bazel-skylib/blob/master/docs/selects_doc.md#selectsconfig_setting_group to use either setting.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't whatever you're doing already work by setting both --cpu and --platforms=//:target with platform(name="target", constraint_values=[ "@platforms//os:macos", "@platforms//cpu:arm64"])?

I don't know what state of CC crosscompilation platformization is. But specifying both platform and flags should get you to the same state as before conditions package was modified.

Copy link
Member Author

Choose a reason for hiding this comment

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

Building with both --cpu=darwin_arm64 and --platforms=//:target gets me this:

INFO: Invocation ID: 7514df6d-c786-4d10-8592-179657b0750b
DEBUG: /private/var/tmp/_bazel_admin/058bf0aeee6f9b65e93a9de41f4a4c2c/external/bazel_toolchains/rules/rbe_repo/checked_in.bzl:103:14: rbe_ubuntu1804_java11 not using checked in configs as detect_java_home was set to True
DEBUG: /private/var/tmp/_bazel_admin/058bf0aeee6f9b65e93a9de41f4a4c2c/external/bazel_toolchains/rules/rbe_repo/checked_in.bzl:103:14: rbe_ubuntu1604_java8 not using checked in configs as detect_java_home was set to True
INFO: Build option --platforms has changed, discarding analysis cache.
ERROR: While resolving toolchains for target //src/main/cpp:client: No matching toolchains found for types @bazel_tools//tools/cpp:toolchain_type. Maybe --incompatible_use_cc_configure_from_rules_cc has been flipped and there is no default C++ toolchain added in the WORKSPACE file? See https://github.com/bazelbuild/bazel/issues/10134 for details and migration instructions.
ERROR: Analysis of target '//src:bazel' failed; build aborted: No matching toolchains found for types @bazel_tools//tools/cpp:toolchain_type. Maybe --incompatible_use_cc_configure_from_rules_cc has been flipped and there is no default C++ toolchain added in the WORKSPACE file? See https://github.com/bazelbuild/bazel/issues/10134 for details and migration instructions.
INFO: Elapsed time: 0.475s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (47 packages loaded, 179 targets configured)

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the dummy cc toolchain isn't being picked up. Would you mind filing this as a separate issue so I can investigate? This PR looks fine to me with the recent change

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather see the problem with dummy cc toolchain fixed than a workaroud.
What happens if you set --incompatible_enable_cc_toolchain_resolution?
Please at least open an issue for dummy cc toolchain and add a TODO here, that is
TODO(issue): remove flag based select when the issue is resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm getting the same error even building with --incompatible_enable_cc_toolchain_resolution.

)

# Workaround for an issue where the dummy cc toolchain isn't being picked up
# when cross-compile from darwin_x86_64 to darwin_arm64 cpu.
# TODO(https://github.com/bazelbuild/bazel/issues/12655): Remove the flag based
# select when the issue is resolved.
selects.config_setting_group(
name = "darwin_arm64",
match_any = [
":darwin_arm64_constraint",
":darwin_arm64_flag",
],
visibility = ["//visibility:public"],
)

Expand Down