search

bazel-contrib / rules_go

Go rules for Bazel
Apache License 2.0
1.38k stars 659 forks source link

Bazel 0.26 break iOS and perhaps Android cross compilation #2079

Closed steeve closed 5 years ago

steeve commented 5 years ago

What version of rules_go are you using?

ddea1ade19b0f7cab64d859d95fa6494714019d7

What version of gazelle are you using?

325c4cf53b65148f5392f5f65462970a7e5cb6fa

What version of Bazel are you using?

Build label: 0.26.0
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Tue May 28 08:38:24 2019 (1559032704)
Build timestamp: 1559032704
Build timestamp as int: 1559032704

Does this issue reproduce with the latest releases of all the above?

Yes.

What operating system and processor architecture are you using?

Darwin XXXX 18.2.0 Darwin Kernel Version 18.2.0: Thu Dec 20 20:46:53 PST 2018; root:xnu-4903.241.1~1/RELEASE_X86_64 x86_64

The problem

Bazel 0.26 ships with new platform enabled toolchains, which are defined like this:

OSX_TOOLS_CONSTRAINTS = {
    "darwin_x86_64": ["@bazel_tools//platforms:osx", "@bazel_tools//platforms:x86_64"],
    "ios_i386": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:x86_32"],
    "ios_x86_64": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:x86_64"],
    "watchos_i386": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:x86_32"],
    "watchos_x86_64": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:x86_64"],
    "tvos_x86_64": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:x86_64"],
    "armeabi-v7a": ["@bazel_tools//platforms:arm"],
    "ios_armv7": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],
    "ios_arm64": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],
    "ios_arm64e": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],
    "watchos_armv7k": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],
    "watchos_arm64_32": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],
    "tvos_arm64": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],
}

Rules go uses the following constraints:

GOOS = {
    "android": None,
    "darwin": "@bazel_tools//platforms:osx",
    "dragonfly": None,
    "freebsd": "@bazel_tools//platforms:freebsd",
    "linux": "@bazel_tools//platforms:linux",
    "nacl": None,
    "netbsd": None,
    "openbsd": None,
    "plan9": None,
    "solaris": None,
    "windows": "@bazel_tools//platforms:windows",
    "js": None,
}

GOARCH = {
    "386": "@bazel_tools//platforms:x86_32",
    "amd64": "@bazel_tools//platforms:x86_64",
    "amd64p32": None,
    "arm": "@bazel_tools//platforms:arm",
    "arm64": "@bazel_tools//platforms:aarch64",
    "mips": None,
    "mips64": None,
    "mips64le": None,
    "mipsle": None,
    "ppc64": None,
    "ppc64le": "@bazel_tools//platforms:ppc",
    "s390x": "@bazel_tools//platforms:s390x",
    "wasm": None,
}

Now the problem is that it then becomes impossible to build for iOS since the toolchain requires @bazel_tools//platforms:ios and @bazel_tools//platforms:arm but compiling for darwin_arm64 requires/enforces @bazel_tools//platforms:osx and @bazel_tools//platforms:aarch64.

I'm not sure what to do here since we can't apply both osx and ios constraints on the same constaint_setting.

steeve commented 5 years ago

Actually, I'm not sure how we can tell bazel how to use multiple toolchains

steeve commented 5 years ago

Adding --toolchain_resolution_debug shows the problem:

INFO: ToolchainResolution:   Considering toolchain @local_config_cc//:cc-compiler-ios_arm64...
INFO: ToolchainResolution:     Toolchain constraint @bazel_tools//platforms:os has value @bazel_tools//platforms:ios, which does not match value @bazel_tools//platforms:osx from the target platform @bazel_tools//platforms:target_platform
INFO: ToolchainResolution:     Toolchain constraint @bazel_tools//platforms:cpu has value @bazel_tools//platforms:arm, which does not match value @bazel_tools//platforms:x86_64 from the target platform @bazel_tools//platforms:target_platform
INFO: ToolchainResolution:   Rejected toolchain @local_config_cc//:cc-compiler-tvos_arm64, because of target platform mismatch
jayconrod commented 5 years ago

As you said, the problem seems to be that the platform rule @io_bazel_rules_go//go/toolchain:darwin_amd64 depends on constraint_value @bazel_tools//platforms:osx, not @bazel_tools//platforms:ios.

darwin is ambiguous: as far as Go is concerned darwin/amd64 is macOS and darwin/arm64 is iOS. We should define our darwin/amd64 toolchain to be compatible with @bazel_tools//platforms:ios. You'll probably need to define a custom platform, but the toolchain should be resolved automatically.

Go considers android to be a separate operating system, so I don't think we'll have the same ambiguity there.

jayconrod commented 5 years ago

@steeve Could you try applying the patch below? I apparently don't have a C/C++ toolchain installed for iOS, so I'm not able to test this. It's probably not complete, but it's a start.

diff --git a/go/private/go_toolchain.bzl b/go/private/go_toolchain.bzl
index 9e98fde8ba..cccfec456f 100644
--- a/go/private/go_toolchain.bzl
+++ b/go/private/go_toolchain.bzl
@@ -100,10 +100,16 @@ def go_toolchain(name, target, sdk, host = None, constraints = [], **kwargs):
     if not host:
         host = target
     goos, _, goarch = target.partition("_")
-    target_constraints = constraints + [
-        "@io_bazel_rules_go//go/toolchain:" + goos,
-        "@io_bazel_rules_go//go/toolchain:" + goarch,
-    ]
+    if goos == "darwin" and goarch == "arm64":
+        target_constraints = [
+            "@bazel_tools//platforms:ios",
+            "@bazel_tools//platforms:aarch64",
+        ]
+    else:
+        target_constraints = constraints + [
+            "@io_bazel_rules_go//go/toolchain:" + goos,
+            "@io_bazel_rules_go//go/toolchain:" + goarch,
+        ]
     host_goos, _, host_goarch = host.partition("_")
     exec_constraints = [
         "@io_bazel_rules_go//go/toolchain:" + host_goos,
diff --git a/go/toolchain/toolchains.bzl b/go/toolchain/toolchains.bzl
index 4094bc733e..70033792b1 100644
--- a/go/toolchain/toolchains.bzl
+++ b/go/toolchain/toolchains.bzl
@@ -45,10 +45,19 @@ def declare_constraints():
                 constraint_setting = "@bazel_tools//platforms:cpu",
             )
     for goos, goarch in GOOS_GOARCH:
-        native.platform(
-            name = goos + "_" + goarch,
-            constraint_values = [
-                ":" + goos,
-                ":" + goarch,
-            ],
-        )
+        if goos == "darwin" and goarch == "arm64":
+            native.platform(
+                name = "darwin_arm64",
+                constraint_values = [
+                    "@bazel_tools//platforms:aarch64",
+                    "@bazel_tools//platforms:ios",
+                ],
+            )
+        else:
+            native.platform(
+                name = goos + "_" + goarch,
+                constraint_values = [
+                    ":" + goos,
+                    ":" + goarch,
+                ],
+            )
steeve commented 5 years ago

darwin is ambiguous: as far as Go is concerned darwin/amd64 is macOS and darwin/arm64 is iOS.

That's even more twisted than that: darwin_386 and darwin_amd64 can also mean iOS (iPhoneSimulator). We differentiate based on the cpu after (in https://github.com/bazelbuild/rules_go/blob/master/go/platform/apple.bzl). Perhaps defining an ios virtual GOOS would do the trick ?

Also, bazel defines both archs as being arm (perhaps that's a bug though):

    "ios_armv7": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],
    "ios_arm64": ["@bazel_tools//platforms:ios", "@bazel_tools//platforms:arm"],

That would mean darwin_386 and darwin_arm64 would define the same constraint values, so I'm not sure how the Go toolchain would properly resolve?

Thank you for the diff! I'll try that and so how I can go from that approach.

jayconrod commented 5 years ago

That's even more twisted than that: darwin_386 and darwin_amd64 can also mean iOS (iPhoneSimulator). We differentiate based on the cpu after (in https://github.com/bazelbuild/rules_go/blob/master/go/platform/apple.bzl). Perhaps defining an ios virtual GOOS would do the trick ?

I think it would be fine to define platform and toolchain that depend on the ios constraint_value but set GOOS=darwin for the purpose of Go compile and linking. I'd much prefer that to the special case in the patch above, I mostly meant that to be diagnostic.

Also, bazel defines both archs as being arm (perhaps that's a bug though):

Seems like a bug. Maybe that's why I couldn't get it to work yesterday? Can you reproduce that just with objc_library or cc_library? If so, please report in bazelbuild/bazel.

steeve commented 5 years ago

Seems like a bug.

I've opened https://github.com/bazelbuild/bazel/issues/8520 just in case

I'm not sure what you want me to reproduce though?

steeve commented 5 years ago

I think it would be fine to define platform and toolchain that depend on the ios constraint_value but set GOOS=darwin for the purpose of Go compile and linking. I'd much prefer that to the special case in the patch above, I mostly meant that to be diagnostic.

I agree that's the way to go. Actually I find it rather nicer than how Go itself does it to be honest.

steeve commented 5 years ago

So far I have this

diff --git a/go/platform/list.bzl b/go/platform/list.bzl
index ec6ec797..a0dad29e 100644
--- a/go/platform/list.bzl
+++ b/go/platform/list.bzl
@@ -13,8 +13,9 @@
 # limitations under the License.

 GOOS = {
-    "android": None,
+    "android": "@bazel_tools//platforms:android",
     "darwin": "@bazel_tools//platforms:osx",
+    "ios": "@bazel_tools//platforms:ios",
     "dragonfly": None,
     "freebsd": "@bazel_tools//platforms:freebsd",
     "linux": "@bazel_tools//platforms:linux",
@@ -52,6 +53,10 @@ GOOS_GOARCH = (
     ("darwin", "amd64"),
     ("darwin", "arm"),
     ("darwin", "arm64"),
+    ("ios", "386"),
+    ("ios", "amd64"),
+    ("ios", "arm"),
+    ("ios", "arm64"),
     ("dragonfly", "amd64"),
     ("freebsd", "386"),
     ("freebsd", "amd64"),

Where would you like to see the ios -> darwin substitution happen ? I was thinking in _go_toolchain_impl but it can be deeper perhaps ?

steeve commented 5 years ago

So this works, save for the aarch64 bug:

diff --git a/go/platform/list.bzl b/go/platform/list.bzl
index ec6ec797..a0dad29e 100644
--- a/go/platform/list.bzl
+++ b/go/platform/list.bzl
@@ -13,8 +13,9 @@
 # limitations under the License.

 GOOS = {
-    "android": None,
+    "android": "@bazel_tools//platforms:android",
     "darwin": "@bazel_tools//platforms:osx",
+    "ios": "@bazel_tools//platforms:ios",
     "dragonfly": None,
     "freebsd": "@bazel_tools//platforms:freebsd",
     "linux": "@bazel_tools//platforms:linux",
@@ -52,6 +53,10 @@ GOOS_GOARCH = (
     ("darwin", "amd64"),
     ("darwin", "arm"),
     ("darwin", "arm64"),
+    ("ios", "386"),
+    ("ios", "amd64"),
+    ("ios", "arm"),
+    ("ios", "arm64"),
     ("dragonfly", "amd64"),
     ("freebsd", "386"),
     ("freebsd", "amd64"),
diff --git a/go/private/go_toolchain.bzl b/go/private/go_toolchain.bzl
index 9e98fde8..ee4e7933 100644
--- a/go/private/go_toolchain.bzl
+++ b/go/private/go_toolchain.bzl
@@ -31,12 +31,15 @@ load("@io_bazel_rules_go//go/private:actions/stdlib.bzl", "emit_stdlib")

 def _go_toolchain_impl(ctx):
     sdk = ctx.attr.sdk[GoSDK]
-    cross_compile = ctx.attr.goos != sdk.goos or ctx.attr.goarch != sdk.goarch
+    goos = ctx.attr.goos
+    if goos == "ios":
+        goos = "darwin"
+    cross_compile = goos != sdk.goos or ctx.attr.goarch != sdk.goarch
     return [platform_common.ToolchainInfo(
         # Public fields
         name = ctx.label.name,
         cross_compile = cross_compile,
-        default_goos = ctx.attr.goos,
+        default_goos = goos,
         default_goarch = ctx.attr.goarch,
         actions = struct(
             archive = emit_archive,

I tried to push the luck into having the ios -> darwin substitution happen only on go.env, but it would fail in weird ways, probably because of installsuffix

steeve commented 5 years ago

Also, ios_arm64 and android_arm64 are broken because of https://github.com/bazelbuild/bazel/issues/8520. I've opened https://github.com/bazelbuild/bazel/pull/8522 to fix that.

jayconrod commented 5 years ago

I've started to work on a fix for this. Currently, we have a GOOS_GOARCH table of valid platforms, and everything is generated from that. I'm working on expanding this to be a wider table with struct elements. The GOOS and GOARCH may be independent from the constraint_values.

I'd rather not introduce ios as anything other than a prefix for a few platform and toolchain targets. It's not a real GOOS value. I haven't figured out how "@io_bazel_rules_go//go/platform:darwin" is going to work yet in select. Need to experiment more.

steeve commented 5 years ago

By the way, I've tested 0.26.1-rc1 and constraining on @bazel_tools//platforms:ios and @bazel_tools//platforms:arm/aarch64 works. Save for the select() issue of course.

steeve commented 5 years ago

As I said in the PR: A gazelle change may be required as now this select doesn't work:

    deps = select({
        "@io_bazel_rules_go//go/platform:darwin": [
            "//vendor/golang.org/x/sys/unix:go_default_library",
        ],

Perhaps it should be:

    deps = select({
        "@io_bazel_rules_go//go/platform:darwin": [
            "//vendor/golang.org/x/sys/unix:go_default_library",
        ],
        "@io_bazel_rules_go//go/platform:ios": [
            "//vendor/golang.org/x/sys/unix:go_default_library",
        ],