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

Bazel 0.26 break iOS and perhaps Android cross compilation #2079

Closed
steeve opened this issue May 28, 2019 · 14 comments · Fixed by #2090
Closed

Bazel 0.26 break iOS and perhaps Android cross compilation #2079

steeve opened this issue May 28, 2019 · 14 comments · Fixed by #2090
Labels
bug darwin Affects macOS / iOS hosts or targets go

Comments

@steeve
Copy link
Contributor

steeve commented May 28, 2019

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
Copy link
Contributor Author

steeve commented May 28, 2019

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

@steeve
Copy link
Contributor Author

steeve commented May 29, 2019

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
Copy link
Contributor

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 jayconrod added bug darwin Affects macOS / iOS hosts or targets go labels May 29, 2019
@jayconrod
Copy link
Contributor

@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
Copy link
Contributor Author

steeve commented May 30, 2019

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
Copy link
Contributor

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
Copy link
Contributor Author

steeve commented May 30, 2019

Seems like a bug.

I've opened bazelbuild/bazel#8520 just in case

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

@steeve
Copy link
Contributor Author

steeve commented May 30, 2019

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
Copy link
Contributor Author

steeve commented May 30, 2019

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
Copy link
Contributor Author

steeve commented May 30, 2019

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
Copy link
Contributor Author

steeve commented May 30, 2019

Also, ios_arm64 and android_arm64 are broken because of bazelbuild/bazel#8520. I've opened bazelbuild/bazel#8522 to fix that.

@jayconrod
Copy link
Contributor

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
Copy link
Contributor Author

steeve commented Jun 4, 2019

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
Copy link
Contributor Author

steeve commented Jun 4, 2019

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",
        ],

jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Jun 4, 2019
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 pushed a commit that referenced this issue Jun 13, 2019
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).

There are also separate platforms for pure mode, and cgo (with _cgo suffix).
Cross compilation may still be done with (for example)
--platforms=@io_bazel_rules_go//go/platforms:linux_amd64, and this
no longer requires a configured C/C++ toolchain. To cross-compile with cgo,
use --platforms=@io_bazel_rules_go//go/platforms:linux_amd64_cgo.
A compatible configured C/C++ toolchain is required in that case.

Fixes #2079
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Jun 18, 2019
jayconrod pushed a commit that referenced this issue Jun 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug darwin Affects macOS / iOS hosts or targets go
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants