bazel: Remove custom framework rule by keith · Pull Request #797 · envoyproxy/envoy-mobile

keith · 2020-04-20T20:39:02Z

With this change we can remove our custom swift_static_framework rule
and replace it with the rules_apple ios_static_framework rule. There are
a few behavioral differences here that required other changes.

ios_static_framework uses -enable-library-evolution, which means we
get a swiftinterface file instead of a swiftmodule. This opts us
in to having to guarantee some level of binary compatibility (once we
hit 1.0 at least) but has the benefit of not requiring consumers to
be on the exact same Xcode version as what we build with
To Support Initial setup #1 we had to remove the bridging header use, this was
previously required because if we imported the Objective-C dependency
directly it also had to be shipped to consumers. Now we use
@_implementationOnly imports with this, and consume it with
private_deps so it's not exposed to consumers at all. This is a
new-ish private feature in the Swift compiler to support this type of
use case.
To propagate the correct header with the framework so that it can be
imported by Objective-C (which isn't automatically done by
rules_swift / rules_apple
Add generated_header to SwiftInfo? bazelbuild/rules_swift#291) we need to
access it from the swift_library target, which I wrote a tiny rule to
do.

Signed-off-by: Keith Smiley keithbsmiley@gmail.com

keith · 2020-04-20T20:41:12Z

bazel/swift_header_collector.bzl

+    dep = ctx.attr.deps[0]
+    return [
+        DefaultInfo(
+            files = dep[CcInfo].compilation_context.headers,


AFAIK since this is a direct dep there cannot be more than 1 in this list, but I could filter by ones ending with -Swift.h if that wasn't the case.

From what I saw in the past we only had one here IIRC, so this is prob fine

examples/objective-c/hello_world/ViewController.m

library/objective-c/BUILD

keith · 2020-04-20T20:42:43Z

Depends on #796

rebello95

So much better!

rebello95 · 2020-04-20T20:47:12Z

bazel/swift_header_collector.bzl

+    dep = ctx.attr.deps[0]
+    return [
+        DefaultInfo(
+            files = dep[CcInfo].compilation_context.headers,


From what I saw in the past we only had one here IIRC, so this is prob fine

examples/objective-c/hello_world/ViewController.m

rebello95 · 2020-04-20T20:49:19Z

library/swift/src/BUILD

+
+swift_header_collector(
+    name = "collector",
+    deps = ["lib"],


Does this change make it possible for us to split out the above code into different libraries and still collect them into one framework here?

not particularly right here, but overall it depends on how you want to split things up. if you have public API in multiple modules I don't think that will end up working in a single framework, but if you have other internal modules like the EnvoyEngine objc is acting as here now, you could include that in private_deps and have as many as you want. But I think no matter what you'll have to be able to @_implementationOnly import them

bazel/swift_header_collector.bzl

library/swift/src/BUILD

With this change we can remove our custom swift_static_framework rule and replace it with the rules_apple ios_static_framework rule. There are a few behavioral differences here that required other changes. 1. ios_static_framework uses `-enable-library-evolution`, which means we get a `swiftinterface` file instead of a `swiftmodule`. This opts us in to having to guarantee some level of binary compatibility (once we hit 1.0 at least) but has the benefit of not requiring consumers to be on the exact same Xcode version as what we build with 2. To Support #1 we had to remove the bridging header use, this was previously required because if we imported the Objective-C dependency directly it also had to be shipped to consumers. Now we use `@_implementationOnly` imports with this, and consume it with `private_deps` so it's not exposed to consumers at all. This is a new-ish private feature in the Swift compiler to support this type of use case. 3. To propagate the correct header with the framework so that it can be imported by Objective-C (which isn't automatically done by rules_swift / rules_apple bazelbuild/rules_swift#291) we need to access it from the swift_library target, which I wrote a tiny rule to do. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

keith · 2020-04-20T21:46:38Z

Might actually depend on bazelbuild/rules_apple#753

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

keith · 2020-04-22T23:59:35Z

Potential fix for the duplicate symbol issue I'm seeing currently bazelbuild/rules_swift#420

keith · 2020-04-22T23:59:59Z

Potential fix for the duplicate symbol issue I'm seeing currently bazelbuild/rules_swift#420

keith · 2020-04-24T23:29:18Z

Filed an issue for the blocker here bazelbuild/bazel#11223

This is an in between to #797 which starts building the archive with swiftinterface files, but does it with our existing swift_static_framework rule because of issues linked on that PR. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

stale · 2020-05-05T02:24:23Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

This is an in between to #797 which starts building the archive with swiftinterface files, but does it with our existing swift_static_framework rule because of issues linked on that PR. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

stale · 2020-05-12T05:51:33Z

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

This is an in between to #797 which starts building the archive with swiftinterface files, but does it with our existing swift_static_framework rule because of issues linked on that PR. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

keith mentioned this pull request Apr 20, 2020

Add generated_header to SwiftInfo? bazelbuild/rules_swift#291

Closed

keith requested review from kastiglione and rebello95 April 20, 2020 20:39

keith commented Apr 20, 2020

View reviewed changes

examples/objective-c/hello_world/ViewController.m Show resolved Hide resolved

keith commented Apr 20, 2020

View reviewed changes

library/objective-c/BUILD Show resolved Hide resolved

rebello95 previously approved these changes Apr 20, 2020

View reviewed changes

kastiglione reviewed Apr 20, 2020

View reviewed changes

bazel/swift_header_collector.bzl Outdated Show resolved Hide resolved

bazel/swift_header_collector.bzl Outdated Show resolved Hide resolved

bazel/swift_header_collector.bzl Outdated Show resolved Hide resolved

keith dismissed rebello95’s stale review via fdc8683 April 20, 2020 21:01

kastiglione previously approved these changes Apr 20, 2020

View reviewed changes

library/swift/src/BUILD Outdated Show resolved Hide resolved

keith dismissed kastiglione’s stale review via ba42f26 April 20, 2020 21:06

keith added 3 commits April 20, 2020 14:30

Change collector API

720bc2d

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

Rename target

ce6b7b8

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

keith force-pushed the ks/bazel-remove-custom-framework-rule branch from ba42f26 to ce6b7b8 Compare April 20, 2020 21:31

Update other references

6707956

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>

keith changed the title ~~[bazel] Remove custom framework rule~~ bazel: Remove custom framework rule Apr 27, 2020

keith mentioned this pull request Apr 27, 2020

bazel: Update swift_static_framework to use swiftinterfaces #815

Merged

stale bot added the stale label May 5, 2020

stale bot closed this May 12, 2020

junr03 deleted the ks/bazel-remove-custom-framework-rule branch July 8, 2020 21:25

junr03 restored the ks/bazel-remove-custom-framework-rule branch July 15, 2021 23:42

junr03 mentioned this pull request Jul 21, 2021

bazel: update to 4.1.0 #1616

Merged

junr03 deleted the ks/bazel-remove-custom-framework-rule branch December 23, 2021 22:28

Conversation

keith commented Apr 20, 2020

Uh oh!

keith Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

rebello95 Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

keith commented Apr 20, 2020

Uh oh!

rebello95 left a comment

Choose a reason for hiding this comment

Uh oh!

rebello95 Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rebello95 Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

keith Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keith commented Apr 20, 2020

Uh oh!

keith commented Apr 22, 2020

Uh oh!

keith commented Apr 22, 2020

Uh oh!

keith commented Apr 24, 2020

Uh oh!

stale bot commented May 5, 2020

Uh oh!

stale bot commented May 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants