Skip to content
New issue

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

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

Already on GitHub? # to your account

[internalsdk:replica] Remove root route handler #720

Merged
merged 3 commits into from
Feb 23, 2022
Merged

Conversation

soltzen
Copy link

@soltzen soltzen commented Feb 18, 2022

Related to https://github.com/getlantern/grants/issues/468

Fixes the recursion panic in the issue. I've also amended the README a bit with unrelated comments.

Bug repro steps for reference:

  • Check out main branch: 80f75db
  • ./scripts/run_avd.rb --level=30 --abi=x86_64 --use_google_apis --window to spin a new emu
  • make -B android-lib-prod ANDROID_ARCH=all && flutter run
  • Search for "Automated CobbleGen" in Discover tab
  • App crashes with the same recursion panic in the attached issue

It was there for serp_web searches, but we don't use that.
Furthermore, it caused a major recursion issue. See here: https://github.com/getlantern/android-lantern/issues/719
@soltzen soltzen requested a review from oxtoacart February 18, 2022 16:09
@soltzen
Copy link
Author

soltzen commented Feb 23, 2022

I'm gonna go ahead and merge this.

@oxtoacart Still would like the review, please

@soltzen soltzen merged commit 17e8233 into main Feb 23, 2022
@soltzen soltzen deleted the soltzen/li5283 branch February 23, 2022 21:18
@@ -28,7 +28,7 @@ Note - you might see an error like `Can't load Kernel binary: Invalid SDK hash.`

All those dependencies must be in your PATH

* Java 11 or greater
* Java 8 or greater (for Android to work)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change this back to 8? I intentionally changed it to 11 in this PR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I build android-release with Java 8, it fails with the below error, so we definitely can't use Java 8.

* What went wrong:
Execution failed for task ':app:kaptProdReleaseKotlin'.
> A failure occurred while executing org.jetbrains.kotlin.gradle.internal.KaptWithoutKotlincTask$KaptExecutionWorkAction
   > java.lang.reflect.InvocationTargetException (no error message)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this was just merged, I'll fix in main.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change this back to 8? I intentionally changed it to 11 in this PR.

Android's sdkmanager and avdmanager require Java 8. I just reproduced your error when running it with 8, and reproduced mine when running either of those binaries with 11... That's not fun.

Apologies for the revertion. Should've got your okay first

Copy link

@oxtoacart oxtoacart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the crash, thanks @soltzen .

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants