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

Add iOS toolchains, platforms, and constraint_values #2090

Merged
merged 7 commits into from
Jun 13, 2019

Conversation

jayconrod
Copy link
Contributor

Refactored the description of target platforms. The source of truth is
now the PLATFORMS table go/private/platforms.bzl. Declarations for
config_settings in //go/platform; constraint_values and aliases in
//go/toolchain; and toolchains in @go_sdk are generated from this
table.

In addition to the normal GOOS_GOARCH list, there are some additional
entries for iOS. iOS platforms still have GOOS=darwin, but they are
compatible with a different constraint_value
(@bazel_tools//platforms:ios instead of @bazel_tools//platforms:osx).

Fixes #2079

Refactored the description of target platforms. The source of truth is
now the PLATFORMS table go/private/platforms.bzl. Declarations for
config_settings in //go/platform; constraint_values and aliases in
//go/toolchain; and toolchains in @go_sdk are generated from this
table.

In addition to the normal GOOS_GOARCH list, there are some additional
entries for iOS. iOS platforms still have GOOS=darwin, but they are
compatible with a different constraint_value
(@bazel_tools//platforms:ios instead of @bazel_tools//platforms:osx).

Fixes bazel-contrib#2079
@jayconrod jayconrod requested a review from ianthehat as a code owner June 4, 2019 18:50
@jayconrod
Copy link
Contributor Author

@steeve Please take a look. This should make it possible to bazel build --platforms=@io_bazel_rules_go//go/toolchain:ios_arm.

This is incomplete. I don't have a solution for select yet. Although it seems to compile correctly, it won't link (but my toolchain is probably just messed up).

@jayconrod
Copy link
Contributor Author

I added a commit that should fix @io_bazel_rules_go//go/platform:darwin. It should be true if target platform is any Go darwin or ios platform or if the target platform is the default (not cross-compiling) and the host OS is macOS.

@steeve
Copy link
Contributor

steeve commented Jun 5, 2019

thank you!
@jayconrod it seems I can't use the @io_bazel_rules_go//go/toolchain:ios constraint with your patch. I suspect only the os_arch combination exist ?
The reason is we have our own platform that used to apply rules_go constraints.

@steeve
Copy link
Contributor

steeve commented Jun 5, 2019

I tried constraining on @bazel_tools//platforms:ios, and I'm happy to report that it works!

@steeve
Copy link
Contributor

steeve commented Jun 5, 2019

Now I'm having issues on Android:
With android_arm64:

ERROR: While resolving toolchains for target //XXXX: no matching toolchains found for types @bazel_tools//tools/cpp:toolchain_type
ERROR: Analysis of target '//XXXX' failed; build aborted: no matching toolchains found for types @bazel_tools//tools/cpp:toolchain_type

I suspect this is a problem in the NDK rules of Bazel.

And on android_arm:

ERROR: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/external/org_golang_google_grpc/internal/syscall/BUILD.bazel:3:1: Illegal ambiguous match on configurable attribute "deps" in @org_golang_google_grpc//internal/syscall:go_default_library:
@io_bazel_rules_go//go/platform:android
@io_bazel_rules_go//go/platform:darwin
Multiple matches are not allowed unless one is unambiguously more specialized.
INFO: SHA256 (http://jcenter.bintray.com/org/ow2/asm/asm/6.2/asm-6.2.jar) = 917bda888bc543187325d5fbc1034207eed152574ef78df1734ca0aee40b7fc8
ERROR: Analysis of target '//android:publish_snapshot' failed; build aborted:

/private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/external/org_golang_google_grpc/internal/syscall/BUILD.bazel:3:1: Illegal ambiguous match on configurable attribute "deps" in @org_golang_google_grpc//internal/syscall:go_default_library:
@io_bazel_rules_go//go/platform:android
@io_bazel_rules_go//go/platform:darwin
Multiple matches are not allowed unless one is unambiguously more specialized.
INFO: Elapsed time: 6.105s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (23 packages loaded, 8805 targets configured)

@steeve
Copy link
Contributor

steeve commented Jun 5, 2019

Note that this happens also on bazel 0.25.3
android_arm

/private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/external/org_golang_google_grpc/internal/channelz/BUILD.bazel:3:1: Illegal ambiguous match on configurable attribute "deps" in @org_golang_google_grpc//internal/channelz:go_default_library:
@io_bazel_rules_go//go/platform:darwin
@io_bazel_rules_go//go/platform:android
Multiple matches are not allowed unless one is unambiguously more specialized.

android_arm64:

/private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/external/org_golang_google_grpc/internal/channelz/BUILD.bazel:3:1: Illegal ambiguous match on configurable attribute "deps" in @org_golang_google_grpc//internal/channelz:go_default_library:
@io_bazel_rules_go//go/platform:darwin
@io_bazel_rules_go//go/platform:android
Multiple matches are not allowed unless one is unambiguously more specialized.

@steeve
Copy link
Contributor

steeve commented Jun 5, 2019

cc @jin since he made the NDK rules
@jin could it be that the NDK rules suffered from the same bug from bazelbuild/bazel#8522 ?

@steeve
Copy link
Contributor

steeve commented Jun 5, 2019

Hum this is weird, when I constrain on @bazel_tools//platforms:aarch64 instead of @io_bazel_rules_go//go/toolchain:arm64, I get the same issue as android_arm.

@jayconrod
Copy link
Contributor Author

it seems I can't use the @io_bazel_rules_go//go/toolchain:ios constraint with your patch. I suspect only the os_arch combination exist ?
The reason is we have our own platform that used to apply rules_go constraints

Some explanation about the naming here: //go/platform contains config_setting targets for each GOOS, GOARCH, and GOOS_GOARCH pair. //go/toolchain contains constraint_setting, constraint_value, and platform targets. Ideally platform targets would be in //go/platform, but that package predates Bazel support for platforms and toolchains by quite a bit, and displacing the old config_settings would break builds.

Anyway, you should be able to define a platform like this:

platform(
    name = "custom_ios_arm",
    constraint_values = [
        "@bazel_tools//platforms:ios",
        "@bazel_tools//platforms:arm',
        "@io_bazel_rules_go//go/toolchain:is_darwin",  # needed for @io_bazel_rules_go//platform:darwin to work
        # any other values relevant to your toolchains
    ],
)

I think the Android problems you're experiencing are #2089, which is affecting cross-compilation in general. I'm planning to fix that in another PR before the next release.

@steeve steeve mentioned this pull request Jun 7, 2019
Jay Conrod added 3 commits June 7, 2019 18:52
This change makes the C/C++ toolchain dependency optional. This should
enable cross-compilation when Bazel C/C++ toolchain selection is
enabled.

* There is a new constraint, //go/toolchain:cgo_constraint with values
  cgo_on and cgo_off. cgo_on is the default.
* A platform is declared for each goos/goarch/cgo mode. For example,
  //go/toolchain:linux_amd64 and linux_amd64_cgo.
* A toolchain is declared for each goos/goarch/cgo mode. For example,
  @go_sdk//:go_linux_amd64 and go_linux_amd64_cgo.
* go_toolchains now optionally depend on //:cgo_context_data, a target
  that extracts information from a required C/C++ toolchain. This is
  the only direct contact with the C/C++ toolchain.
* get_mode is slightly adjusted to select pure mode when there is no
  C/C++ toolchain or when it is likely to be misconfigured.
* Everything else is fixed not to expect the C/C++ toolchain to exist
  in pure mode.
@jayconrod jayconrod merged commit f2373c9 into bazel-contrib:master Jun 13, 2019
@jayconrod jayconrod deleted the ios-compat branch June 13, 2019 20:45
jayconrod pushed a commit that referenced this pull request Jul 8, 2019
This is a partial cherry-pick of #2090 for release branches only.

Fixes #1987
jayconrod pushed a commit that referenced this pull request Jul 8, 2019
This is a partial cherry-pick of #2090 for release branches only.

Fixes #1987
jayconrod pushed a commit that referenced this pull request Jul 8, 2019
This is a partial cherry-pick of #2090 for release branches only.

Fixes #1987
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bazel 0.26 break iOS and perhaps Android cross compilation
3 participants