Android NDK 32 bit Arm should be marked "armv7" not "arm"

[ttiurani]

Description of the problem / feature request:

When building Android NDK with --fat_apk_cpu=armeabi-v7a the platform it hits is cpu arm and not cpu armv7, even though "armv7" is claimed to be the right value here:

https://github.com/bazelbuild/rules_android/blob/master/android/platforms/BUILD#L6

Feature requests: what underlying problem are you trying to solve with this feature?

Android platforms should work right, this came up with trying to get rust to work with Android, and this kind of special platform was needed. This problem makes it hard to target the right rust target thumbv7neon-linux-androideabi in rules_rust, which could be mapped if the right armv7 was set.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

This repository shows the needed special platform workaround needed:
https://github.com/ttiurani/examples/tree/main/android/ndk-rust

What operating system are you running Bazel on?

Same issue on OSX and linux.

What's the output of bazel info release?

release 5.0.0 (but also latest 6.0.0 preview has the problem)

Have you found anything relevant by searching the web?

This was talked about on slack in this thread:

https://bazelbuild.slack.com/archives/CSV56UT0F/p1645029913244809

[gregestren]

Are you setting --platforms or --android_platforms on your build? Or only --fat_apk_cpu?

I'm asking because we're moving toward the former via https://bazel.build/concepts/platforms-intro but it's Work In Progress. So users have to actively opt into making platform matter, and that extra context can help us evaluate.

[ttiurani]

I'm setting platforms via platform_mappings:

https://github.com/ttiurani/examples/blob/main/android/ndk-rust/platform_mappings

This is because rules_rust uses platforms and the stable android doesn't. The README here:

https://github.com/ttiurani/examples/blob/main/android/ndk-rust

describes also the build command I use for the project, i.e. just --fat_apk_cpu.

I was aware of the platforms WIP branch, but didn't use it here.

[keith]

We hit this same case with armeabi-v7a, specifically rules_rust does the right thing and uses the triple to decide CPU constraints, and therefore created a constraint with armv7, which then doesn't match the C++ constraint if you want to build rust and C++ for android. I've submitted a workaround there bazelbuild/rules_rust#1259 but ideally we would fix it here.

[keith]

I think this fixes this issue, it does in my repro with rules_rust, as in I'd no longer need the above patch. Please test your case as well! I'm interested to hear from the bazel folks what issues this would cause

#15248

[gregestren]

@katre suggested CC'ing @cpsauer regarding effective platform mappings.

@katre also said he's looking at a related issue at https://github.com/bazelbuild/platforms, although I'm not sure offhand which.

[cpsauer]

Looks great to me. Go Keith! Probably a breaking change for the few using it, but IMO the right one if we're looking to quickly move away from @platforms//cpu:arm in favor of specific ones (per the source).

I for one would be happy to switch when this lands--and I love that Keith is leading the fix at the root so we aren't forcing rules_rust to do the wrong thing long term.

From some quick testing, it looks like Apple's iOS toolchains work with either arm and armv7, so no need for a parallel fix there. Probably, you know, thanks to @keith, who'd know more, I'm sure.

More generally on platform_mappings:

• @ttiurani, I'd advocate for using --Android configuration distinguisher=android instead of crosstool_top. Harder to find initially, but makes things a little bit more robust, I think.
• Once this is all fixed, IMO cpu:arm should be a general category that matches any arm CPU. See Platform Commonalities: e.g. Apple, POSIX, etc. platforms#37
• I'd still advocate strongly that we ship a some default (lowest priority) platform mappings embedded in Bazel that make os and cpu selectable with the transitions used on these common platforms--rather that heaving everyone roll their own. Having batteries included would radically lower the barrier to entry and get people selecting on platforms. Useful stuff! Happy to contribute if bazel team members are down to help get it through.

Cheers!
Chris

[keith]

From some quick testing, it looks like Apple's iOS toolchains work with either arm and armv7, so no need for a parallel fix there

Is there more context on this somewhere?

[cpsauer]

^ Figured you'd be the guy to ask :) All I did was quickly change my platform_mappings over to armv7 to see if anything broke on iOS--and didn't seem to have any problems building. Sorry to not have more info.

[keith]

Are you building for 32 bit iOS

[cpsauer]

Yes. Definitely. I'll reapply the stash, and play around a little bit more. Anything in particular you'd like me to poke at?

[keith]

I guess toolchain resolution, I would expect those to match for sure. Also with the recent-ish iOS fixes around it

[cpsauer]

Matches all look okay. So:

w/ armv7
Type @bazel_tools//tools/cpp:toolchain_type: target platform //Bazel/Platforms:_ios-armv7: execution @local_config_platform//:host: Selected toolchain @local_config_cc//:cc-compiler-ios_armv7
And I can see it skip what looks like an arm (not v7) duplicate:
INFO: ToolchainResolution: Type @bazel_tools//tools/cpp:toolchain_type: target platform //Bazel/Platforms:_ios-armv7: Rejected toolchain @local_config_cc//:cc-compiler-armeabi-v7a; mismatching values: arm

[cpsauer]

And then, conversely, if I revert out a switch to armv7, it selects the other:
INFO: ToolchainResolution: Target platform //Bazel/Platforms:_ios-arm: Selected execution platform @local_config_platform//:host, type @bazel_tools//tools/cpp:toolchain_type -> toolchain @local_config_cc//:cc-compiler-armeabi-v7a
Rejecting the one it previously selected:
INFO: ToolchainResolution: Type @bazel_tools//tools/cpp:toolchain_type: target platform //Bazel/Platforms:_ios-arm: Rejected toolchain @local_config_cc//:cc-compiler-ios_armv7; mismatching values: armv7

[keith]

Ah this could be because there are now 2 toolchains that resolve to this in the Apple configuration.

bazel/tools/osx/crosstool/BUILD.tpl

Lines 96 to 112 in 6872fd2

	# Android tooling requires a default toolchain for the armeabi-v7a cpu.
	cc_toolchain(
	name = "cc-compiler-armeabi-v7a",
	toolchain_identifier = "stub_armeabi-v7a",
	toolchain_config = ":stub_armeabi-v7a",
	all_files = ":empty",
	ar_files = ":empty",
	as_files = ":empty",
	compiler_files = ":empty",
	dwp_files = ":empty",
	linker_files = ":empty",
	objcopy_files = ":empty",
	strip_files = ":empty",
	supports_param_files = 1,
	)
	armeabi_cc_toolchain_config(name = "stub_armeabi-v7a")

[keith]

I just pushed an update to #15248, can you try that? Previously there was no CPU conflict, but now there is, since bazel doesn't choose the optimal match, just the first

[cpsauer]

Wait, just to be clear that we aren't talking past each other somehow: This was a quick test to see if there seemed to be a parallel problem on the iOS side, which there didn't. Isn't that PR about Android?

[cpsauer]

Oh, I see. It's selecting some android dummy toolchain we think?

[cpsauer]

I'll point Bazelisk at it and report back.

[cpsauer]

I think I need to wait until CI finishes for it to not 404, right? Or is there another, better way of my getting it?

[keith]

Probably not, short of building yourself

[cpsauer]

Don't think I'll be able to beat it by much, so I'll wait and report back. You want to know whether builds succeed either way and if different toolchains are selected, right?

Also, Keith, thanks for chiming in on the Apple platforms selector with the useful Envoy example. Looked like you'd just cooked up platform mappings over there. Related to that and this: @katre and I were discussing more (over email) the idea of baking in a default platform_mappings so people can start selecting on platforms without having to do roll their own. What do you think about that?

[keith]

Yea it shouldn't impact your ability to build for 32 bit macOS for sure. Yes I think providing the default platform_mappings would be great.

[cpsauer]

Green, but still 404ing. It's my first time doing this; any guidance?
2022/04/19 15:26:11 could not download Bazel: HTTP GET https://storage.googleapis.com/bazel-builds/artifacts/macos/120e43e0a2b777adb80e1cc8fe72a3dc87338eb0/bazel failed with error 404

Also, anything in particular you'd like me to play around with? And to confirm, selecting cc-compiler-armeabi-v7a for iOS would be a problem?

[keith]

Maybe PRs don't push releases here? you might have to build it. I might have misunderstood your original issue, but I just want to verify 32 bit iOS still works, along with the armv7 android behavior

[cpsauer]

Oh, well the original thing was intended to be a non-issue; that iOS things had looked good on quick inspection. But it seems like you think otherwise--for reasons I don't understand yet.

[cpsauer]

Have built your PR's branch from source and run it. Things build cleanly with armv7--and indeed now there's no android default toolchain to (erroneously) match iOS on arm (non-v7).

Thanks, Keith!

[keith]

Awesome, thanks for testing!

[katre]

The fix is now merged, thanks for figuring this out!

[brentleyjones]

@bazel-io flag

[ckolli5]

@bazel-io fork 5.2.0