Regression: Unexpected clang argument for extension targets

[rahul-malik]

@c-parsons :
We have a share and siri ios_extension rule in our application and they have a few dependencies like the Facebook SDK.

They've been building correctly but as of this PR we are getting failures because -fapplication-extension is passed and fails on third-party code.

Additionally we are unable to specify extension_safe=False in our rule definition without encountering an error since it seems to already be hard-coded to True.

[allevato]

Just to make sure I'm understanding your issue:

• You've been compiling code that isn't extension-safe into an extension, and that's failing now because we're more strict about forcing that flag to be set?
• So, are you asking for a way to explicitly disable -fapplication-extension on extensions to get that code to compile, even though it opens you up to API safety issues?

And the PR you linked is definitely the one that causes it? I wouldn't expect that one to affect it.

[rahul-malik]

• Yes, there are Third-Party SDKs (Facebook in this case) that are used in our extension but we do not call any unsafe extension APIs. Dependencies linked to our extension are causing the issues and we may not be in a position to make them compliant.
• A way to disable this (at least for dependencies) would allow us to upgrade for other fixes (resource processing logic for example) while we work through seeing if we can get rid of these dependencies or resolve the issue another way. This is a regression IMO and should be called out through warnings for some time before failing builds.

The PR I linked is the first one that starts to show the error for me. I wonder if it's because _create_linked_binary_target is reading the value for extension_safe from it's arguments rather than kwargs.

-      extension_safe = kwargs.get("extension_safe"),
--
+      extension_safe = extension_safe,

[allevato]

Ok, having extension_safe default to True for extension targets but letting the user override it if they know what they're doing seems like a reasonable thing to do (since this is possible in Xcode by just modifying the build setting there).

I'll take a stab at it soon. The code you cited wasn't intended to change the behavior, but it's possible there was a subtle change there.

[rahul-malik]

@allevato - Thanks!

[rahul-malik]

@allevato - I was actually able to remove the dependency to unblock the issue. I still think you should have a way to disable this but just wanted to give you a heads up.

[allevato]

I realized this morning that there's actually nothing to be done here.

The extension_safe attribute on apple_binary was rolled back to be a no-op in bazelbuild/bazel@8effaa7, but the baseline for the 0.5.2 release was cut before that commit. It was rolled back because we found that it often caused the same dependency to be compiled twice if it was used in, say, an application and an extension (once with -fapplication-extension and once without), which was detrimental to the build times for a number of our teams.

Even though you've already unblocked yourself, if you build Bazel from HEAD, the problem should go away. Since we're moving fast with the Apple Skylark rules and they often require changes to Bazel's core, we're kind of avoiding keeping features in sync with official releases right now.

In the future if we

[rahul-malik]

If we....? : 🍿

[allevato]

Oops 😄

In the future if we fix the handling of the flag at the binary level, then we can revisit the "escape hatch" and have an attribute on the bundling rules that does the "escape hatch" behavior described above.

[rahul-malik]

@allevato - Would it be possible to use GH releases to have notes for when rules_apple depends on something that isn't in the latest official version? We're trying to minimize the amount of bazel version updates for our engineering team and CI infra but we also want to stay up-to-date with rules_apple fixes / updates.