Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: setup basic check format #3

Merged
merged 3 commits into from
Apr 18, 2019
Merged

ci: setup basic check format #3

merged 3 commits into from
Apr 18, 2019

Conversation

mattklein123
Copy link
Member

We can extend this for other languages, etc. in the future.

We can extend this for other languages, etc. in the future.

Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123
Copy link
Member Author

cc @junr03

Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123 mattklein123 merged commit 56b5252 into master Apr 18, 2019
@mattklein123 mattklein123 deleted the format_ci branch April 18, 2019 17:34
@junr03 junr03 mentioned this pull request Jun 21, 2019
crockeo added a commit to crockeo/envoy-mobile that referenced this pull request Dec 3, 2021
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>
@Augustyniak Augustyniak mentioned this pull request Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants