Automatic mobile platforms detection (transition from cpu and crosstool_top)

[steeve]

As of Bazel 3.0, Android and iOS rules do not set constraints (android toolchain can be selected via a constraint, however).

I propose to add an automatic transition step for those, so that the use is seamless. The code is pretty simple and straightforward.

Perhaps could even be added to #2433.

I know ideally those rules shoud set constraints and rules_go would follow along, but from what I'd wager, it's really not there yet, so I think it could be a cool stopgap solution.

[steeve]

On the other hand, one could get away with select + goos/goarch now that #2414 is merged. But it wouldn't "just work".

[steeve]

Here is a quick draft. Basically, if goos/goarch == "auto", try to detect any of the two mobile platforms:

diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl
index 9c3b3583..39294519 100644
--- a/go/private/rules/transition.bzl
+++ b/go/private/rules/transition.bzl
@@ -100,6 +100,36 @@ def go_transition_rule(**kwargs):
     kwargs["cfg"] = go_transition
     return rule(**kwargs)
 
+_ANDROID_CPUS_TO_TOOLCHAIN = {
+    "arm64-v8a": "android_arm64",
+    "armeabi-v7a": "android_arm",
+    "x86": "android_386",
+    "x86_64": "android_amd64",
+}
+
+_IOS_CPUS_TO_TOOLCHAIN = {
+    "ios_arm64": "ios_arm64",
+    "ios_armv7": "ios_arm",
+    "ios_i386": "ios_386",
+    "ios_x86_64": "ios_amd64",
+}
+
+def android_toolchain(crosstool_top, cpu):
+    return \
+        (str(crosstool_top) == "//external:android/crosstool" or \
+            crosstool_top.workspace_name == "androidndk") and \
+        _ANDROID_CPUS_TO_TOOLCHAIN.get(cpu)
+
+def ios_toolchain(crosstool_top, cpu):
+    return _IOS_CPUS_TO_TOOLCHAIN.get(cpu)
+
+def detect_mobile_platforms(settings):
+    crosstool_top = settings.pop("//command_line_option:crosstool_top")
+    cpu = settings.pop("//command_line_option:cpu")
+    toolchain = android_toolchain(crosstool_top, cpu) or ios_toolchain(crosstool_top, cpu)
+    if toolchain:
+        settings["//command_line_option:platforms"] = "@io_bazel_rules_go//go/toolchain:%s_cgo" % toolchain
+
 def _go_transition_impl(settings, attr):
     settings = dict(settings)
     goos = getattr(attr, "goos", "auto")
@@ -118,6 +148,8 @@ def _go_transition_impl(settings, attr):
             fail('pure is "off" but cgo is not supported on {} {}'.format(goos, goarch))
         platform = "@io_bazel_rules_go//go/toolchain:{}_{}{}".format(goos, goarch, "_cgo" if cgo else "")
         settings["//command_line_option:platforms"] = platform
+    else:
+        detect_mobile_platform(settings)
     if pure != "auto":
         pure_label = _filter_transition_label("@io_bazel_rules_go//go/config:pure")
         settings[pure_label] = pure == "on"
@@ -138,6 +170,8 @@ def _go_transition_impl(settings, attr):
 go_transition = transition(
     implementation = _go_transition_impl,
     inputs = [_filter_transition_label(label) for label in [
+        "//command_line_option:cpu",
+        "//command_line_option:crosstool_top",
         "//command_line_option:platforms",
         "@io_bazel_rules_go//go/config:static",
         "@io_bazel_rules_go//go/config:msan",

[steeve]

When combined with #2445, adding a go_binary as a dep to an android_binary will correctly produce the multi-arch split:

$ bazel build //examples/helloworld:sampleapp
INFO: Analyzed target //examples/helloworld:sampleapp (10 packages loaded, 7133 targets configured).
INFO: Found 1 target...
[257 / 273] 4 actions running
    GoStdlib external/io_bazel_rules_go/android_386_c-shared/stdlib%/pkg; 15s darwin-sandbox
    GoStdlib external/io_bazel_rules_go/android_amd64_c-shared/stdlib%/pkg; 15s darwin-sandbox
    GoStdlib external/io_bazel_rules_go/android_arm_c-shared/stdlib%/pkg; 15s darwin-sandbox
    GoStdlib external/io_bazel_rules_go/android_arm64_c-shared/stdlib%/pkg; 15s darwin-sandbox

$ unzip -l bazel-bin/examples/helloworld/sampleapp.apk | grep \\.so
  2328792  01-01-2010 00:00   lib/arm64-v8a/libmain.so
  1863528  01-01-2010 00:00   lib/armeabi-v7a/libmain.so
  1830552  01-01-2010 00:00   lib/x86/libmain.so
  2347000  01-01-2010 00:00   lib/x86_64/libmain.so

Which is really cool IMHO.

[jayconrod]

Sounds like it could work, but I don't understand how Android / iOS targets are built well enough to implement or maintain it. I'd be very nervous about adding something like this.

Just to confirm my understanding, this would let you build an android_binary (or ios_binary?) that depends on a go_binary (built in c-archive or c-shared mode I assume). You'd set --cpu and --crosstool_top but not --platforms. The Go binary would be built multiple times, once for each target architecture (split transition).

Are there tracking issues for when Android / iOS would support --platforms? Ideally this should just work already, but I'd rather not maintain extra workarounds in rules_go indefinitely.

I wonder if, instead, it would make more sense to have a separate rule that does this transition while depending on a regular go_binary target, forwarding its providers?

android_binary(
    ...
    deps = [
        ":go_mobile_bin",
    ],
)

go_mobile_binary(
    name = "go_mobile_bin",
    dep = ":go_bin",
)

go_binary(
    name = "go_bin",
    ...
)

go_mobile_binary would look like:

def _go_mobile_transition_impl(settings, attr):
    # same thing you wrote above

go_mobile_transition = transition(
    implementation = _go_mobile_transition_impl,
    inputs = [
        "//command_line_option:cpu",
        "//command_line_option:crosstool_top",
    ],
    outputs = [
        "//command_line_option:platforms",
    ],
)

def _go_mobile_binary_impl(ctx):
    return [ctx.attr.dep[p] for p in [DefaultInfo, CcInfo, GoArchive, GoSource, GoLibrary]]

go_mobile_binary = rule(
    implementation = _go_mobile_binary,
    attrs = {"dep": attr.label(mandatory = True)},
    cfg = go_mobile_transition,
)

This would be less magical, but I imagine it would be a lot more straightforward for you to maintain, and probably useful in other situations as well.

[steeve]

Sounds like it could work, but I don't understand how Android / iOS targets are built well enough to implement or maintain it. I'd be very nervous about adding something like this.

ios_{application,framework,...} and android_binary can depend on CcInfo. Because those targets can be multi arch (fat binaries for macOS/iOS, multiple .so files for Android), they will split the graph for earch arch and build accordingly.
The same is actually also true for macOS binaries too, which can be fat.

I don't think having another target is the way to go. When #2445 is implemented, depending on go_binary becomes mainstream. For consistency's sake, IMHO it would be better if it just worked like it does with cc_*.

It seems the generic issue is transiting the platform based on inbound cpu and crosstool_top, and do it in a generic, isolated way. Obviously this is for legacy rules. However, because those values can be anything, it is difficult to define a generic interface for all.

I figure iOS and Android rules are so widely used that it makes sense to have special cases for them, such as what exists in platform/apple.bzl.

Impacted platforms would be macOS, iOS, TvOS, WatchOS and Android.

[steeve]

The maintainability is a good point, however. I can work on something more generic in the transition to separate inbound values from actual mobile stuff.

Something like goos, goarch = detect_platform_from_croostool(settings) and custom hooks inside that method to implement iOS and Android matching. All living in the platform/ package.

I expect maintainability to be low. crosstool_top and cpu values haven't changed in the ~3 years I've been using those rules. The next evolution for rules_apple and future rules_android is to move to platforms/constraints, at which point we can just remove the code.

[steeve]

A quick V2 to get the mood. This is the sort of organisation i'd like to have, separated in go/platform and having matchers. This way this becomes easy and separated.

diff --git a/go/platform/crosstool.bzl b/go/platform/crosstool.bzl
new file mode 100644
index 00000000..6839476c
--- /dev/null
+++ b/go/platform/crosstool.bzl
@@ -0,0 +1,32 @@
+def _match_apple(crosstool_top, cpu):
+    platform = {
+        "darwin_x86_64": "darwin_amd64",
+        "ios_arm64": "ios_arm64",
+        "ios_armv7": "ios_arm",
+        "ios_i386": "ios_386",
+        "ios_x86_64": "ios_amd64",
+    }.get(cpu)
+    if platform:
+        return "%s_cgo" % platform
+
+def _match_android(crosstool_top, cpu):
+    if str(crosstool_top) == "//external:android/crosstool" or \
+        crosstool_top.workspace_name == "androidndk":
+        platform_cpu = {
+            "arm64-v8a": "arm64",
+            "armeabi-v7a": "arm",
+            "x86": "386",
+            "x86_64": "amd64",
+        }.get(cpu)
+        if platform_cpu:
+            return "android_%s_cgo" % platform_cpu
+
+def platform_from_crosstool(crosstool_top, cpu):
+    matchers = [
+        _match_apple,
+        _match_android,
+    ]
+    for matcher in matchers:
+        platform = matcher(crosstool, cpu)
+        if platform:
+            return "@io_bazel_rules_go//go/toolchain:%s" % platform
diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl
index 9c3b3583..ed0584e5 100644
--- a/go/private/rules/transition.bzl
+++ b/go/private/rules/transition.bzl
@@ -25,6 +25,10 @@ load(
     "@io_bazel_rules_go_name_hack//:def.bzl",
     "IS_RULES_GO",
 )
+load(
+    "@io_bazel_rules_go//go/platform:crosstool.bzl",
+    "platform_from_crosstool",
+)
 
 def _filter_transition_label(label):
     """Transforms transition labels for the current workspace.
@@ -106,6 +110,8 @@ def _go_transition_impl(settings, attr):
     goarch = getattr(attr, "goarch", "auto")
     pure = getattr(attr, "pure", "auto")
     _check_ternary("pure", pure)
+    crosstool_top = settings.pop("//command_line_option:crosstool_top")
+    cpu = settings.pop("//command_line_option:cpu")
     if goos != "auto" or goarch != "auto":
         if goos == "auto":
             fail("goos must be set if goarch is set")
@@ -118,6 +124,10 @@ def _go_transition_impl(settings, attr):
             fail('pure is "off" but cgo is not supported on {} {}'.format(goos, goarch))
         platform = "@io_bazel_rules_go//go/toolchain:{}_{}{}".format(goos, goarch, "_cgo" if cgo else "")
         settings["//command_line_option:platforms"] = platform
+    else:
+        platform = platform_from_crosstool(crosstool_top, cpu)
+        if platform:
+            settings["//command_line_option:platforms"] = platform
     if pure != "auto":
         pure_label = _filter_transition_label("@io_bazel_rules_go//go/config:pure")
         settings[pure_label] = pure == "on"

[steeve]

Also, unfortunately this doesn't work:

def _go_mobile_binary_impl(ctx):
    return [ctx.attr.dep[p] for p in [DefaultInfo, CcInfo, GoArchive, GoSource, GoLibrary]]

bazel will complain that the executable in DefaultInfo is from another target :(