Add visionOS support

[keith]

No description provided.

[keith]

CI can't be green here until we update to include bazelbuild/platforms#71

[keith]

Ok this is green! sounds like @nglevin is going to review

[nglevin]

Overall looks good to me.

I have some feedback regarding where bits will go, and some concern about introducing a minimum OS version CLI flag for visionOS specifically when there's a shared attribute that we have an opportunity to adopt before it has users depending on it.

[nglevin]

Have you considered using the existing --minimum_os_version flag instead of establishing a new --visionos_minimum_os flag, here?

Context; the flag was established for Android Crosstool purposes, to be shared by Apple Crosstools. You can access it from Starlark via the cpp fragment.

Ideally every Apple platform should source from a shared flag instead of platform-specific --*_minimum_os-es since the Apple transitions set it based on rule-level attributes, the minimum_os_version is typically set from an Apple bundling rule, and as far as building a _library rule for a given SDK with support for a minimum OS version, there shouldn't be a need to separate that for iOS vs watchOS between a _library and its explicit deps.

There's quite a few folks around Bazel who support the effort, and it'd be easier to go straight for that as we introduce the new Apple platform rather than attempt to add a new --*_minimum_os command line option that we'd then have to try to rapidly depreciate.

[keith]

Definitely an option, the first I heard about the desire to not have these platform specific flags was from TVL after I submitted this so if you think we should avoid adding it here that's fine. I think as far as bazel users go there aren't many folks relying on this for building library rules (doing that for a specific platform is not easy given the intermediate state around migrating to platforms). Do you think we should commit and not add this flag?

[nglevin]

I think it'd be best to avoid adding this flag in this PR, absolutely.

At least that would keep the scope of changes smaller, until a better solution for sourcing the minimum OS version for visionOS is available.

[keith]

I looked into removing this and it's not super straight-forward. The current issue is that the CppOptions / CppConfiguration depend on the AppleConfiguration, which is where we compute the minimum OS and we would need to reference CppOptions.minimumOsVersion. Unless we want to move the Apple specific logic / flags from that into the CppConfiguration, which I don't think we would? Although we have done that for some other flags like --apple_generate_dsym

[nglevin]

We spoke about this in another shared channel. I see the issue, and I've relayed it to folks who own those options.

As agreed, we shouldn't do anything about that in this PR, but it would also be best not to introduce a --visionos_minimum_os flag in this PR.

For now, in the absence of any versions of visionOS to support besides "1.0" until shipping products come sometime next year, we'll stub this out and I'll look into addressing --minimum_os_version as the source of truth for visionOS builds in follow up work for myself.

[nglevin]

This is for J2ObjC, correct? Just making sure I understand the existing client usage.

[keith]

I think this one is on its way out but it's still used here as well

bazel/src/main/java/com/google/devtools/build/lib/bazel/rules/objc/BazelObjcLibraryRule.java

Lines 27 to 32 in 896b358

	@Override
	public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
	return builder
	.cfg(AppleCrosstoolTransition.APPLE_CROSSTOOL_TRANSITION)
	.build();
	}

[nglevin]

Added two more comments as I was really looking up and down to see what the other CLI options were being used for these days... some of which might just be historical inertia.

If there's an odd dependency of Bazel Apple crosstool or a similar service that needs it, that may be alright for now.

[comius]

This broke downstream builds: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3225#0189d812-974f-43a7-8c36-2b8ee3698598

[comius]

Also https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3225#0189d812-a177-499c-b0b7-3ba37659e73f

[nglevin]

All of those downstream projects should update to the platforms repo at 0.0.7 , as the change @ bazelbuild/platforms#71 was rolled in to that release.

Is there a common process for that? Or are all downstream builds expected to manually update to the latest platforms repo independently?

[lberki]

Downstream projects we track are on BuildKite are expected to update themselves, cc @meteorcloudy

[aiuto]

It feels like we could do better on this sort of problem in a few ways. To summarize the core issue: - the built in apple rules in Bazel reference a label ***@***.***//os:visionos" - this creates an implicit dependency that the *user* is using platforms at at least 0.0.7 - rules_apple can depend on the appropriate platforms release, so it can mask the problem Just throwing out some ideas... - Allow the LHS of a select() entry to be non-existent. The big downside is that typos never get caught - We could have a place in bazel, post workspace construction, where modules can check repositories for various properties (like version). They could fail out if various constraints were not met. This would still fail the build, but the reason would be clearer. - src/main/starlark/builtins_bzl/common/cc/cc_test.bzl could be rewritten to only depend on visionos if the label exists.

[keith]

6.x cherry pick #19436

[iancha1992]

The changes in this PR have been included in Bazel 6.4.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!