Remove objc_library transition

[keith]

Through the discussion on this issue it came up that we might want to remove the objc_library transition. I also think this might be a blocker to some changes like this where we would like to change the transition in rules_apple to support this case, but a similar transition is duplicated here and likely needs to be kept roughly in sync.

We heard at bazelcon that some folks at google might be looking into removing this, so I wanted to file this tracking issue for discussion

[keith]

I started looking at providing an incompatible flag for removing this, but unsurprisingly it breaks some tests, I haven't started debugging since I figure the real blocker will be making google compatible, in which case I'm not sure how feasible this is to do from the outside

[keith]

cc @katre @googlewalt @gregestren @susinmotion

[katre]

@trybka This is relevant to your interests.

[trybka]

It's not terrible to fix internally, but not trivial, either.
I'd really like to get rid of this, but at the moment it's mostly just difficult to justify the time/effort for said depot-wide cleanup.

Most issues were fairly straightforward: like migrating from a build_test like rule to one which does the transition (e.g. ios_build_test).

There were some harder ones where folks are doing clever things with the unconfigured target graph and expecting objc_library to not break, but don't need anything from it either. In those cases, the top-level target might not be appropriate to transition itself. I think it would be sufficient to allow the target to analyze as it would not be needing the result.

I was considering some kind of tristate where the transition is either on/soft-off/hard-off. For soft/hard, it either reports an error if it cannot configure, say, objc-compile actions, or silently returns an empty CcInfo provider. This would allow us to get errors to find build tests that need fixing, and maybe we can force our build test rules to hard error once the migration is complete, and we can leave these other things that just "happen to" depend on objc_library to silently just not configure their actions.

WDYT?

[keith]

I guess that solution is at a slightly different level that what I was thinking about in the transition itself, which I was mostly just making a no-op with the incompatible flag. At what point would the starlark implementation know enough about whether or not it would be configurable to be able to make the decision of soft/hard fail?

[trybka]

I was being somewhat imprecise there.
Making the transition a no-op (optionally) is a good first step (that would control one piece).

Then, in the Starlark rule impl, we can just check to see whether the action is enabled or not.

Something like this:

    if not cc_common.action_is_enabled(
        feature_configuration = feature_configuration,
        action_name = "objc-compile",
    ):  
        if ctx.fragments.objc.enable_unconfigured_library():
            return [CcInfo()]
        else:
            fail("objc_library is not configured for this platform.")

[gregestren]

Whatever the details, I think when the phrase "depot cleanup" comes up it's perfectly reasonable to flag-gate functionality and allow Bazel and Google to migrate on their own timelines. Depot cleanups are often involved, long, and by nature org-specific. I'm still struggling with this for config_setting visibility.

For Bazel, we'd have to similarly check we don't get downstream breakages?

[sventiffe]

@meteorcloudy for Greg's question about downstream breakages

[meteorcloudy]

For Bazel, we'd have to similarly check we don't get downstream breakages?

If this change can be enabled by a flag, you can follow our breaking change process to test it in https://buildkite.com/bazel/bazelisk-plus-incompatible-flags.

You can also run a downstream testing against your custom version of Bazel: https://github.com/bazelbuild/continuous-integration/blob/master/docs/downstream-testing.md

[keith]

I submitted a flag for an incompatible change here #19256

and actually @trybka in the change moving the toolchain to apple_support I did something similar as you mention above, so I think we've already solved that case.

bazel/src/main/starlark/builtins_bzl/common/objc/semantics.bzl

Lines 19 to 34 in 5806691

	def _check_toolchain_supports_objc_compile(ctx, cc_toolchain):
	feature_configuration = cc_common.configure_features(
	ctx = ctx,
	cc_toolchain = cc_toolchain,
	language = "objc",
	requested_features = ctx.features,
	unsupported_features = ctx.disabled_features,
	)
	if not cc_common.action_is_enabled(
	feature_configuration = feature_configuration,
	action_name = "objc-compile",
	):
	fail("Compiling objc_library targets requires the Apple CC toolchain " +
	"which can be found here: https://github.com/bazelbuild/apple_support#toolchain-setup")

[erikmchut]

Thanks for working on this change. I use a custom toolchain for objc compilation, and the built-in apple transition causes unexpected configuration changes that are difficult to work around.