Add automatic platform detection from inbound crosstool_top and cpu

[steeve]

When an inbound crosstool_top and cpu are specified, some platforms
can be automatically inferred/detected.

Some legacy rules still implement configuration splits on
crosstool_top and cpu instead of platform constaints. This is the
case most notably for rules_apple and rules_android.

Add a generic platform matching system based on crostool_top/cpu.

Finally, add Apple and Android matchers.

Refs #2443

[steeve]

Ideally this goes hand in hand with #2445. I'll update the README once you're satisfied jay.

[jayconrod]

It looks like this logic will only be used when _go_transition_impl is used. That will only happen if one of the mode-changing attributes like goos, race, or pure is used. So you could have a regular go_binary that's built for the host goos / goarch, then you could have another go_binary with pure = "on" that's built for Android / iOS. That seems like a weird experience.

I'd rather not activate the transition unconditionally in go_binary. I tried that earlier on. Bazel will treat it as a separate configuration, even if the transition doesn't change anything. So for example, a library built directory with bazel build won't be reused when it's a dependency of a go_binary.

Also, are there any tracking issues for making the Android / Apple rules use platforms and toolchains? I know this code probably won't change much over time, but I don't want to maintain it indefinitely.

[achew22]

I'd rather not activate the transition unconditionally in go_binary. I tried that earlier on. Bazel will treat it as a separate configuration, even if the transition doesn't change anything. So for example, a library built directory with bazel build won't be reused when it's a dependency of a go_binary.

Isn't that a bug in Bazel's implementation of transitions?

[steeve]

It looks like this logic will only be used when _go_transition_impl is used. That will only happen if one of the mode-changing attributes like goos, race, or pure is used. So you could have a regular go_binary that's built for the host goos / goarch, then you could have another go_binary with pure = "on" that's built for Android / iOS. That seems like a weird experience.
It looks like this logic will only be used when _go_transition_impl is used. That will only happen if one of the mode-changing attributes like goos, race, or pure is used. So you could have a regular go_binary that's built for the host goos / goarch, then you could have another go_binary with pure = "on" that's built for Android / iOS. That seems like a weird experience.

Thinking about it, that detection only makes sense for c-shared/c-archive and when the go_binary is used as a dep in another target. That means the configuration doesn't have to be applied universally, and detection could perhaps only happen for those linkmodes. That would reduce the surface without sacrificing UX.

Also, are there any tracking issues for making the Android / Apple rules use platforms and toolchains? I know this code probably won't change much over time, but I don't want to maintain it indefinitely.

For rules_android, rumor had it at bazelcon that the starlark rewrite that they were "mostly done". I'd wager the new ones are constraint ready, and if not, much easier to contribute.
For rules_apple, I checked and they are still using apple_common.multi_arch_split.

I have opened bazelbuild/bazel#11181 for Apple. For Android I asked @jin but he doesn't have a timeline, and obviously the current situation doesn't help.

[jin]

Also, are there any tracking issues for making the Android / Apple rules use platforms and toolchains? I know this code probably won't change much over time, but I don't want to maintain it indefinitely.

cc @gregce / @katre

[katre]

Is there a reason you can't accomplish this with [platform_mapping(https://docs.bazel.build/versions/master/platforms-intro.html#platform-mappings)? or is the concern that you'd need to get every user of rules_go to use the mapping file?

We do plan to work with the android and apple teams to migrate to platforms, but I don't know yet what the timeframe on that will end up being.

[steeve]

@katre this is very cool! I didn't know it existed.

Bad news is it requires users to specify that flag. If the mappings file has to live in rules_go (or at the very least something in the documentation regarding it), then I'm thinking it's better to have something that just works without specifying a bazel flag.

With #2445 go_binary will export CcInfo, so I feel having yet another step for it to work in the iOS/Android case is not a good idea, unless necessary of course.

[jayconrod]

Huh, platform_mapping is cool. I haven't seen that before.

I think I'd much rather recommend that. It does require users to opt in, but it seems like that's just a matter of checking in the file in the root directory or adding a line to .bazelrc.

[jayconrod]

Isn't that a bug in Bazel's implementation of transitions?

@achew22 Maybe?

@gregestren @katre If a transition doesn't make any changes to its settings, could that be treated as a no-op? Currently, I think that creates another configuration, and files in separate but identical configurations are still considered separate (correct me if I'm wrong about that).

If this were a no-op, it would enable a simplification. I currently have go_binary and go_transition_binary with a wrapper that decides which one to use, based on arguments. If no-op transitions were cheap, I could just have one rule with no wrapper.

[katre]

That should be a no-op currently. Configurations are identified by a checksum of the flags, so if none are changed there should be no effect. If you're seeing differently we would want to investigate that.

[jayconrod]

Hmm, okay, that's not what I'm seeing. I might be doing something wrong though.

I opened bazelbuild/bazel#11196 to investigate. It has a standalone example that reproduces this.

[steeve]

I just tested platform_mappings. It does work, but it does require the mappings file to live inside the user's own project, as the flag doesn't take a label, but a relative path to a file. IMHO the experience isn't as nice, doesn't save much maintainability wise, has to be explained somewhere and could conflict with other rules requiring the same (although I'm not aware of any).

Of course I'd understand if you preferred that route, although I'm not really seduced by the approach.

Here are the mappings:

flags:
  --crosstool_top=@androidndk//:toolchain-libcpp
  --cpu=arm64-v8a
    @io_bazel_rules_go//go/toolchain:android_arm64_cgo

  --crosstool_top=@androidndk//:toolchain-libcpp
  --cpu=armeabi-v7a
    @io_bazel_rules_go//go/toolchain:android_arm_cgo

  --crosstool_top=@androidndk//:toolchain-libcpp
  --cpu=x86
    @io_bazel_rules_go//go/toolchain:android_386_cgo

  --crosstool_top=@androidndk//:toolchain-libcpp
  --cpu=x86_64
    @io_bazel_rules_go//go/toolchain:android_amd64_cgo

  --crosstool_top=@androidndk//:default_crosstool
  --cpu=arm64-v8a
    @io_bazel_rules_go//go/toolchain:android_arm64_cgo

  --crosstool_top=@androidndk//:default_crosstool
  --cpu=armeabi-v7a
    @io_bazel_rules_go//go/toolchain:android_arm_cgo

  --crosstool_top=@androidndk//:default_crosstool
  --cpu=x86
    @io_bazel_rules_go//go/toolchain:android_386_cgo

  --crosstool_top=@androidndk//:default_crosstool
  --cpu=x86_64
    @io_bazel_rules_go//go/toolchain:android_amd64_cgo

  --crosstool_top=//external:android/crosstool
  --cpu=arm64-v8a
    @io_bazel_rules_go//go/toolchain:android_arm64_cgo

  --crosstool_top=//external:android/crosstool
  --cpu=armeabi-v7a
    @io_bazel_rules_go//go/toolchain:android_arm_cgo

  --crosstool_top=//external:android/crosstool
  --cpu=x86
    @io_bazel_rules_go//go/toolchain:android_386_cgo

  --crosstool_top=//external:android/crosstool
  --cpu=x86_64
    @io_bazel_rules_go//go/toolchain:android_amd64_cgo

  --cpu=darwin_x86_64
    @io_bazel_rules_go//go/toolchain:darwin_amd64_cgo

  --cpu=ios_arm64
    @io_bazel_rules_go//go/toolchain:ios_arm64_cgo

  --cpu=ios_armv7
    @io_bazel_rules_go//go/toolchain:ios_arm_cgo

  --cpu=ios_i386
    @io_bazel_rules_go//go/toolchain:ios_386_cgo

  --cpu=ios_x86_64
    @io_bazel_rules_go//go/toolchain:ios_amd64_cgo

[jayconrod]

Yeah, platform_mappings isn't perfect. If a better solution will be possible in a quarter or two, I think documenting this in the FAQ or wiki would be an acceptable short-term solution though.

My main issue here is that the behavior where cross-compilation only kicks in on a transition is too surprising. Even if linkmode will usually be set in this situation, it's too surprising that setting linkmode can change the target OS / architecture.

If bazelbuild/bazel#11196 is fixed, then we could use the transition unconditionally (it would usually be a no-op). I think that would require a higher minimum version of Bazel though, but that could potentially be done for rules_go 0.24 (Q3).

I wonder if some other approach is possible? get_mode could check the value of cpu and crosstool_top, gathered perhaps by go_context_data using config_settings. If they are set to known Android / iOS values, it would override the goos, goarch that comes from the toolchain.

This would break cross-compilation to other platforms though (goos and goarch settings would be ignored), so maybe that behavior could be opted into with another flag.

Again, this requires some action from each user, but I don't yet see a solution clean enough to be enabled by default.

[jayconrod]

Closing inactive PRs.

I'm not sure I remember all the context for this one, but it looks like bazelbuild/bazel#11196 is fixed, so this PR may not be necessary.

Also, there is now an API to detect constraints within rules: https://docs.bazel.build/versions/3.6.0/skylark/lib/ctx.html#target_platform_has_constraint

[steeve]

I have some spare time, so coming back to this.

Since bazelbuild/bazel#11196 is now fixed for Bazel >= 3.6, we might as well apply the transition regardless, WDYT ? It wouldn't require action from the user, too.

Also, for some reason, I cannot make platform mapping files work on 4.0.0 anymore.

[steeve]

I have rebased the branch, perhaps we can reopen this PR or should I make a new one?

[jayconrod]

Sorry again for the latency.

So if there's a no-op transition, is there any observable effect? I guess it would change file paths to something with a hash. Anything else?

If there isn't any other drawback, this seems fine to reopen. I'm always nervous about global changes though. Curious what other @bazelbuild/go-maintainers think.

[steeve]

I'm curious as well.

Also, the PR only changes the platform if there are grounds to do so based on inbound cpu and crosstool_top. It doesn't change when/if the transition happens.

[gregestren]

I'm not sure if there's anything for me (as a core Bazel dev) to add at this point. FYI I'm happy to support any further needs or diagnsosis w.r.t. no-op transition behavior.

Note that no-op transitions actually caused some problems for projects that needed the opposite effect (they need to show a difference for reasons I can't remember). You're on the right side of that functionality now, and I'm not inclined to solve those problems by un-no-oping no-ops!

[steeve]

IMHO we can reopen this PR as it doesn't change wether or not the transition is applied, but only what it does based on inbound crosstool_top and cpu.

[jayconrod]

@steeve Go ahead and reopen or recreate. GitHub says the branch was force-pushed or recreated, so I'm not able to reopen the PR.

[steeve]

Me neither, I'll open a new PR

[steeve]

I have reopened it as #2859