Add platforms to apple_universal_binary transition

[comius]

Fixes: bazelbuild/rules_swift#1103

[brentleyjones]

Doesn't this prevent a user's platform_mappings from taking precedence? (Similar to my comment about adding the platform in the apple transition in rules_apple)?

[comius]

No. It works with platform_mappings. Tested at Google. It should be in sync though. Setting unmatching cpu+plaform has unpredictable consequences.

[brentleyjones]

I mean if we have a platform_mappings that doesn't use the platforms specified here, just the constraints. Or has platforms that are children of the ones specified here (in order to add custom platform properties). Then this transition would transition to a wrong platform, right?

[thii]

I thought it should be fine as far as they have the same constraints?

[brentleyjones]

That's why I'm asking, since I'm confused on how a transition with a specific platform value works.

[brentleyjones]

Yes, I'm asking why it's not already working, but also after we fix that (adding --apple_platform_type), if a user had a platform_mapping that didn't use apple_support, but something like this, would it still work?

platforms:
  //my_custom_platforms:macos_x86_64
    --apple_platform_type=macos
    --cpu=darwin_x86_64

  //my_custom_platforms:macos_arm64
    --apple_platform_type=macos
    --cpu=darwin_arm64

  //my_custom_platforms:macos_arm64e
    --apple_platform_type=macos
    --cpu=darwin_arm64e

flags:
  --cpu=darwin_x86_64
  --apple_platform_type=macos
    //my_custom_platforms:macos_x86_64

  --cpu=darwin_arm64
  --apple_platform_type=macos
    //my_custom_platforms:macos_arm64

  --cpu=darwin_arm64e
  --apple_platform_type=macos
    //my_custom_platforms:macos_arm64e

[comius]

I think it would (new platform mapping + this change), but I wouldn't bet on it without testing it out.
I think it would also work with just the new platform mapping.

I'd recommend testing thoroughly anything using platform mapping. It's fragile, by experience. Effects are often unexpected.

I would bet on including platforms into transitions everywhere. We don't have problems with that. The difference is that "everywhere" part is probably easier in monorepo than Bazel.

[brentleyjones]

I would bet on including platforms into transitions everywhere

I'm just trying to make sure we properly handle people's custom platforms, and not force them into the ones defined here, with the transition.

[comius]

I would think 15 platform are a lot, that you don't need more :)

Possible solution is to finish Apple platformization. That is using --apple_platforms flag set to @platforms values, flipping --incompatible_enable_apple_toolchain_resolution.

I don't think you're far away from that actually and I think that is also a stable approach.

[brentleyjones]

I would think 15 platform are a lot, that you don't need more :)

It's not about quantity, it's about the other things that come with platforms, besides constraints. A user can define custom platforms that specify exec_properties, which can impact how remote caching or remote execution works. So the platforms defined in this repo are a nice base, but they could use parents on their custom platform to inherit from that base. Then in their platform_mappings they would use their custom platforms. So I want to make sure that if a transition sets platforms, that it selects their platform defined in platform_mappings, and not the specific one in the transition.

Possible solution is to finish Apple platformization. That is using --apple_platforms flag set to https://github.com/platforms values, flipping --incompatible_enable_apple_toolchain_resolution.

Yeah, possibly. I haven't looked at --apple_platforms to see if it solves the multi-platform issues I raised in https://docs.google.com/document/d/1OVagRkIArFv8N5fspUzhHrJ20od8zqWbOG51DTmuaD0/edit?usp=sharing. In particular, I don't believe it allows settings every platform that could be in a build (iOS and macOS at the same time), thus limiting you to a single platform per build.

[brentleyjones]

@keith I think before we merge this (or before we cut a release with the change in it), I should add the platform overrides we've been talking about.

[keith]

sounds good, although I do wonder if in this case they are a bit less necessary

[brentleyjones]

For macOS they are even more necessary, because of the remote exec properties.

[brentleyjones]

(I'm fine with merging this now, and I can come in with the other change after.)