incompatible_objc_linking_info_migration

[googlewalt]

This flag is part of efforts to migrate ObjcProvider linking info to CcLinkingContext. It controls whether native Objective-C/C++ rules will use linking info from ObjcProvider or CcInfo (which contains the CcLinkingContext). If the flag is false, bazel will get its linking info from ObjcProvider (pre-migration behavior). If the flag is true, bazel will get its linking info from CcInfo (post-migration behavior).

See #16939 for more details on the migration, and general migration guide.

The flag was implemented on 12/15/2022, and enabled by default on 1/25/2023. It is not in Bazel 6.0. The reason for flipping the flag relatively soon is to because there have already been cleanups to internal versions of rules_apple/rules_swift that require the flag flip, so we wanted to try to keep the repos relatively consistent.

Below is more information on what the bazel community may need to do for the migration:

• The change should mostly impact rules maintainers, and is expected to be mostly transparent to end users.

• For rules maintainers:

  • All internal versions of the language rules were migrated by 12/8. Affected rules were rules_apple and rules_kotlin. Note that no changes were required for rules_swift. There was one change required for rules_kotlin but I'm not clear on how it is maintained; for now I'll provide info on rules_apple.

  OSS maintainers of rules_apple need to update those rules. Here is a list of relevant changes if cherrypicking is required:

  10/10: https://github.com/bazelbuild/rules_apple/commit/254a4bba620054e633bc7a82c01b02a8661bf07d 10/13: https://github.com/bazelbuild/rules_apple/commit/608ae263acfe1b517bc15a0bd8c1cea27cbd49d7 11/9: https://github.com/bazelbuild/rules_apple/commit/c13da60c1d896348b6432fa9edf4f5edbb94c19c 11/10: https://github.com/bazelbuild/rules_apple/commit/8e68a3ce1975b5a822562464b7f73bfab0e241c1 11/10: https://github.com/bazelbuild/rules_apple/commit/d8ab65d2efd9afe04687ad5df565a2305b39e96e
  11/11: https://github.com/bazelbuild/rules_apple/commit/17f4ac463d98f118f1b492b1f9fd70f7c69a1b28 12/8: https://github.com/bazelbuild/rules_apple/commit/ed4e8c61efe9e120977f0a184f143c2554fb84e7

• Migrating other rules:

  • Rules that generate linking info need to generate them in CcInfo. These are typically calls to apple_common.new_objc_provider(). Only calls that produce linking info require migration.

  • Rules that use linking info need use them from CcInfo. These are typically references to fields in ObjcProviderApi.

  • Rules that produce AppleDynamicFrameworkInfo and AppleExecutableBinaryInfo need to add CcInfos that provide the linking info to them.

  • See #16939 for details. For examples for how to migrate, see (1) above links, (2) rules_swift which has lots of examples of how the same linking info is represented in ObjcProvider and CcInfo.

• End users need to make sure they are using compatible versions of Starlark rules. They may need to fix some build errors that were previously not reported, such as duplicate definition link errors.

[googlewalt]

@keith @brentleyjones FYI.

[BalestraPatrick]

@googlewalt Is it expected that this is causing failures to link due to missing symbols from frameworks in an iOS application with Objective-C and Swift? I was testing something unrelated on latest Bazel's master and run into this failure. Setting --incompatible_objc_linking_info_migration=false fixed it.

[keith]

I think it is since we don't have all the patches above in the rules, so some providers are missing the required info.

[googlewalt]

Sounds likely. Please disable the migration flag until rules_apple is caught up with the patches.

[googlewalt]

@keith @brentleyjones Do you have any ETAs for when rules_apple can get caught up with the required patches, so I can continue with the cleanup?

[keith]

The issue we have right now is that the cc_toolchain_forwarder is depended on by many upstream changes, but that change requires all users to have a platform_mappings file, which we're not sure we want to force on everyone.

[googlewalt]

Thanks @keith for importing all the required changes. AFAICT rule_apple and rules_swift should now work with bazel at head which has --incompatible_objc_linking_info_migration=true. Can I go ahead and proceed with the cleanup? The next steps are: 1. stop generating linking info in ObjcProvider in native rules and rules_apple (rules_swift did this already). 2. delete --incompatible_objc_linking_info_migration flag and native support for linking info in ObjcProvider. 3. implement --incompatible_objc_provider_remove_linking_info flag as prelude to removing the linking info APIs.

[keith]

I'm tracking the current updates needed for rules_apple here https://github.com/bazelbuild/rules_apple/pull/1848, that list you gave was helpful but there are a few other fixes that I'm still debugging.

As for your next few steps from the outside of google perspective:

1. I don't think we'll be able to take these changes externally until we want to drop support for bazel 6.x, which we naturally want to delay as long as possible since users are mostly on LTS releases unfortunately
2. I imagine this change to entirely delete this flag shouldn't be done until after bazel 7.x releases? That seems like a long time but otherwise the addition, flipping, and deletion of this would all have happened only on rolling releases?
3. Same comment from 1 for our side, I don't think we'd be able to make the external rules work with that until we drop bazel 6.x

[googlewalt]

It is no longer a requirement to wait an entire release to flip a flag, then an entire release to remove a flag. That requirement was back before we switched to the LTS model, when releases would happen ever few months. Part of the motivation for switching to the LTS model is to improve development velocity at head, while still providing stability with LTS releases. Taking 1-2 years to implement/flip/retire an incompatible flag would slow down development even more than it was before.

Can we make a 6.x fork of rules_apple? I think in another thread @brentleyjones mentioned that this was already done for 5.x and seems like a good model to continue.

[brentleyjones]

@googlewalt It's correct that we forked for 6.x, but we don't want to do that again. Instead, we want to support both 6.x and 7.x in the same ruleset version. The forking makes it really hard for users, especially with Bzlmod version resolution, and I agree with @meteorcloudy that we shouldn't fork unless absolutely necessary: https://github.com/bazelbuild/bazel/issues/17372#issuecomment-1438703881

[googlewalt]

Ok IIUC these constraints don't necessarily block my plan, nor is a fork required? Once https://github.com/bazelbuild/rules_apple/pull/1848 is done, rule_apple should be able to support both bazel 6.x and 7.x.

1. I can clean up the internal rules_apple with the understanding that the patches will not land land in OSS. To make things easier to track, I can try to minimize the number of patches to 1 (similar to what rules_swift already did).

2. Deleting --incompatible_objc_linking_info_migration does not affect behavior of 7.x, as that flag is enabled in 7.x.

3. --incompatible_objc_provider_remove_linking_info can be implemented, but it would need to be kept off to avoid breaking OSS rules_apple. Flipping the flag and final cleanup might have to wait till 7.x, but that's much better than being blocked earlier.

How does that sound?

[keith]

1. yep sounds fine. we're pretty behind already because of the other blockers i mentioned
2. I think this is a bit quick, realistically i imagine next to no one has tested this externally, it working for google means a lot for sure, but i imagine there's something we might have missed that folks will be blocked on updating on if they can't turn off the flag anymore. hopefully it's fine but just mentioning that concern
3. sounds good, there will be some point closer to 7.x where dropping 6.x will probably be fine, but it's a large maintenance burden for us to drop it so soon after the 6.x release, especially when most users will stay on 6.x until 7.x is fully released anyways.

[keith]

ok rules_apple now passes tests with the flag enabled as well

[googlewalt]

That's great to hear. Thanks for unblocking me!

[meteorcloudy]

If you want this flag to be tested by https://buildkite.com/bazel/bazelisk-plus-incompatible-flags, please add migration-ready label.