Initial setup #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattklein123
force-pushed
the
initial_repo_setup
branch
6 times, most recently
from
f293ae5
to
73edfe8
Compare
Signed-off-by: Matt Klein <mklein@lyft.com>
mattklein123
force-pushed
the
initial_repo_setup
branch
from
73edfe8
to
a0548c3
Compare
|
@junr03 PTAL |
junr03
reviewed
Apr 15, 2019
|
@junr03 updated |
Signed-off-by: Matt Klein <mklein@lyft.com>
keith
added a commit
that referenced
this pull request
Apr 20, 2020
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>
keith
added a commit
that referenced
this pull request
Apr 20, 2020
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>
keith
added a commit
that referenced
this pull request
Feb 9, 2022
When building the jni dylib for android, we previously stripped all debug info to decrease the artifact size. With this change we now produce the same stripped binary as before, but before stripping it we create a dump of the debug info suitable for crash reporting. This is made overly difficult for a few reasons: 1. Bazel doesn't support fission for Android bazelbuild/bazel#14765 2. Extra outputs from rules are not propagated up the dependency tree, so just building `android_dist` at the top level, isn't enough to get the extra outputs built as well 3. Building the library manually alongside the android artifact on the command line results in 2 separate builds, one for android as a transitive dependency of `android_dist` and one for the host platform This change avoids #1 fission for now, but the same approach could be used once that change makes its way to a bazel release. This change ignores #2 but fixes #3 so it requires you to explicitly build the library as part of the command line invocation if you want debug info, like: ``` ./bazelw build --fat_apk_cpu=... android_dist //library/common/jni:libenvoy_jni.so.debug_info ``` Theoretically we could probably shove this artifact into the aar to make sure it was correctly produced, but that risks us accidentally shipping that in the aar. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
keith
added a commit
that referenced
this pull request
Feb 9, 2022
When building the jni dylib for android, we previously stripped all debug info to decrease the artifact size. With this change we now produce the same stripped binary as before, but before stripping it we create a dump of the debug info suitable for crash reporting. This is made overly difficult for a few reasons: 1. Bazel doesn't support fission for Android bazelbuild/bazel#14765 2. Extra outputs from rules are not propagated up the dependency tree, so just building `android_dist` at the top level, isn't enough to get the extra outputs built as well 3. Building the library manually alongside the android artifact on the command line results in 2 separate builds, one for android as a transitive dependency of `android_dist` and one for the host platform This change avoids #1 fission for now, but the same approach could be used once that change makes its way to a bazel release. This change fixes #2 by using a separate output group that can be depended on by the genrule that writes to dist while avoiding #3 because the custom rule producing these uses the android transition. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
jpsim
added a commit
that referenced
this pull request
Apr 22, 2022
This was causing hangs/crashes in our Lyft apps so I'm reverting this while I continue to investigate. Here's the crash backtrace I'm seeing on iOS (not very helpful unfortunately): ``` * thread #11, stop reason = signal SIGABRT * frame #0: 0x000000013a02300e libsystem_kernel.dylib`__pthread_kill + 10 frame #1: 0x00000001399a61ff libsystem_pthread.dylib`pthread_kill + 263 frame #2: 0x0000000138905684 libsystem_c.dylib`abort + 123 frame #3: 0x00000001024793af Lyft`___lldb_unnamed_symbol4253$$Lyft + 3231 frame #4: 0x000000010247e0c7 Lyft`___lldb_unnamed_symbol4339$$Lyft + 130 frame #5: 0x00000001399a64e1 libsystem_pthread.dylib`_pthread_start + 125 frame #6: 0x00000001399a1f6b libsystem_pthread.dylib`thread_start + 15 ``` This breaks h3 functionality, which merged yesterday and is as of yet unused. Signed-off-by: JP Simard <jp@jpsim.com>
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.