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

Bump minSdk to 21 - Android 5 / Lollipop #7613

Merged
merged 7 commits into from
Jul 13, 2022

Conversation

litetex
Copy link
Member

@litetex litetex commented Jan 3, 2022

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Bump minSdk to 21 - Android 5 / Lollipop - Dropped support for Android 4.4 and removed all corresponding code snippets

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@Fs00
Copy link
Contributor

Fs00 commented Jan 3, 2022

There also resources in values-v21 and values-night-v21 folders that can be put into their corresponding files in values folder.

@litetex
Copy link
Member Author

litetex commented Jan 4, 2022

@Fs00

Thank you for the hint.
I consolidated the styles 😄

@Fs00
Copy link
Contributor

Fs00 commented Jan 4, 2022

Great! After a quick look at the code I've found other things that are unnecessary on API 21+:

  • AndroidX Multidex library and its relative multiDexEnabled config parameter are not needed anymore, since Android 5+ natively supports multiple DEX files
  • Vector drawables backwards-compatibility (enabled here) can be disabled (vector are natively supported on Android 5+) and usages of attribute app:srcCompat need to be replaced with android:src

@TobiGr
Copy link
Member

TobiGr commented Jan 4, 2022

Please also check whether we can disable coreLibraryDesugraing. That is crushing reporoducible / deterministic builds (see #6486).

Are you going to upgrade okHttp in this PR?

@litetex
Copy link
Member Author

litetex commented Jan 4, 2022

Thank you guys for the hints, I will have a look.

Are you going to upgrade okHttp in this PR?

No that should be a new one. I'm just dropping 4.4 here 😄

@litetex
Copy link
Member Author

litetex commented Jan 4, 2022

Please also check whether we can disable coreLibraryDesugraing. That is crushing reproducible / deterministic builds (see #6486).

This is not possible.
Otherwise the app crashes on devices < Android 8 / API 26 because Java 8 features are missing.

Fun Fact: Compiling works execution not
01/04 18:26:38: Launching 'app' on Pixel 3a API 22.
Install successfully finished in 28 s 40 ms.
$ adb shell am start -n "org.schabi.newpipe.debug.increaseminsdk/org.schabi.newpipe.MainActivity" -a android.intent.action.MAIN -c android.intent.category.LAUNCHER
Connected to process 5554 on device 'Pixel_3a_API_22 [emulator-5556]'.
Connected to process 5624 on device 'Pixel_3a_API_22 [emulator-5556]'.
Capturing and displaying logcat messages from application. This behavior can be disabled in the "Logcat output" section of the "Debugger" settings page.
I/ACRA: ACRA is enabled for org.schabi.newpipe.debug.increaseminsdk, initializing...
D/InfoCache: clearCache() called
I/art: Rejecting re-init on previously-failed class java.lang.Class<org.ocpsoft.prettytime.PrettyTime$$ExternalSyntheticLambda0>
    Rejecting re-init on previously-failed class java.lang.Class<org.ocpsoft.prettytime.PrettyTime$$ExternalSyntheticLambda0>
D/LeakCanary: Updated AppWatcher.config: Config(watchDurationMillis=10000)
I/stetho: Listening on @stetho_org.schabi.newpipe.debug.increaseminsdk_devtools_remote
D/LeakCanary: Updated LeakCanary.config: Config(dumpHeap=false)
D/AndroidRuntime: Shutting down VM
E/ACRA: ACRA caught a NoClassDefFoundError for org.schabi.newpipe.debug.increaseminsdk
    java.lang.NoClassDefFoundError: Failed resolution of: Ljava/time/format/DateTimeFormatter;
        at org.schabi.newpipe.error.ErrorActivity.<clinit>(ErrorActivity.java:72)
        at java.lang.reflect.Constructor.newInstance(Native Method)
        at java.lang.Class.newInstance(Class.java:1606)
        at android.app.Instrumentation.newActivity(Instrumentation.java:1066)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2226)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2387)
        at android.app.ActivityThread.access$800(ActivityThread.java:151)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1303)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:135)
        at android.app.ActivityThread.main(ActivityThread.java:5254)
        at java.lang.reflect.Method.invoke(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:372)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)
     Caused by: java.lang.ClassNotFoundException: Didn't find class "java.time.format.DateTimeFormatter" on path: DexPathList[[zip file "/data/app/org.schabi.newpipe.debug.increaseminsdk-1/base.apk"],nativeLibraryDirectories=[/vendor/lib64, /system/lib64]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:56)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:511)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:469)
        at org.schabi.newpipe.error.ErrorActivity.<clinit>(ErrorActivity.java:72) 
        at java.lang.reflect.Constructor.newInstance(Native Method) 
        at java.lang.Class.newInstance(Class.java:1606) 
        at android.app.Instrumentation.newActivity(Instrumentation.java:1066) 
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2226) 
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2387) 
        at android.app.ActivityThread.access$800(ActivityThread.java:151) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1303) 
        at android.os.Handler.dispatchMessage(Handler.java:102) 
        at android.os.Looper.loop(Looper.java:135) 
        at android.app.ActivityThread.main(ActivityThread.java:5254) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:372) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698) 
    	Suppressed: java.lang.ClassNotFoundException: java.time.format.DateTimeFormatter
        at java.lang.Class.classForName(Native Method)
        at java.lang.BootClassLoader.findClass(ClassLoader.java:781)
        at java.lang.BootClassLoader.loadClass(ClassLoader.java:841)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:504)
        		... 16 more
     Caused by: java.lang.NoClassDefFoundError: Class not found using the boot class loader; no stack available
Connected to process 5760 on device 'Pixel_3a_API_22 [emulator-5556]'.
Capturing and displaying logcat messages from application. This behavior can be disabled in the "Logcat output" section of the "Debugger" settings page.
I/ACRA: ACRA is enabled for org.schabi.newpipe.debug.increaseminsdk, initializing...
D/InfoCache: clearCache() called
I/art: Rejecting re-init on previously-failed class java.lang.Class<org.ocpsoft.prettytime.PrettyTime$$ExternalSyntheticLambda0>
    Rejecting re-init on previously-failed class java.lang.Class<org.ocpsoft.prettytime.PrettyTime$$ExternalSyntheticLambda0>
D/LeakCanary: Updated AppWatcher.config: Config(watchDurationMillis=10000)
I/stetho: Listening on @stetho_org.schabi.newpipe.debug.increaseminsdk_devtools_remote
D/LeakCanary: Updated LeakCanary.config: Config(dumpHeap=false)
D/AndroidRuntime: Shutting down VM
E/ACRA: ACRA caught a NoClassDefFoundError for org.schabi.newpipe.debug.increaseminsdk
    java.lang.NoClassDefFoundError: Failed resolution of: Ljava/time/format/DateTimeFormatter;
        at org.schabi.newpipe.error.ErrorActivity.<clinit>(ErrorActivity.java:72)
        at java.lang.reflect.Constructor.newInstance(Native Method)
        at java.lang.Class.newInstance(Class.java:1606)
        at android.app.Instrumentation.newActivity(Instrumentation.java:1066)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2226)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2387)
        at android.app.ActivityThread.access$800(ActivityThread.java:151)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1303)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:135)
        at android.app.ActivityThread.main(ActivityThread.java:5254)
        at java.lang.reflect.Method.invoke(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:372)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)
     Caused by: java.lang.ClassNotFoundException: Didn't find class "java.time.format.DateTimeFormatter" on path: DexPathList[[zip file "/data/app/org.schabi.newpipe.debug.increaseminsdk-1/base.apk"],nativeLibraryDirectories=[/vendor/lib64, /system/lib64]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:56)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:511)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:469)
        at org.schabi.newpipe.error.ErrorActivity.<clinit>(ErrorActivity.java:72) 
        at java.lang.reflect.Constructor.newInstance(Native Method) 
        at java.lang.Class.newInstance(Class.java:1606) 
        at android.app.Instrumentation.newActivity(Instrumentation.java:1066) 
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2226) 
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2387) 
        at android.app.ActivityThread.access$800(ActivityThread.java:151) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1303) 
        at android.os.Handler.dispatchMessage(Handler.java:102) 
        at android.os.Looper.loop(Looper.java:135) 
        at android.app.ActivityThread.main(ActivityThread.java:5254) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:372) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698) 
    	Suppressed: java.lang.ClassNotFoundException: java.time.format.DateTimeFormatter
        at java.lang.Class.classForName(Native Method)
        at java.lang.BootClassLoader.findClass(ClassLoader.java:781)
        at java.lang.BootClassLoader.loadClass(ClassLoader.java:841)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:504)
        		... 16 more
     Caused by: java.lang.NoClassDefFoundError: Class not found using the boot class loader; no stack available

@Fs00
Copy link
Contributor

Fs00 commented Jan 4, 2022

@litetex It just came to my mind that if you disable vector drawables backwards compatibility, you need also to replace app:drawableXXXCompat attributes with their android:drawableXXX counterpart (I've seen that there are some usages of those in NewPipe).

@litetex
Copy link
Member Author

litetex commented Jan 5, 2022

@litetex It just came to my mind that if you disable vector drawables backwards compatibility, you need also to replace app:drawableXXXCompat attributes with their android:drawableXXX counterpart (I've seen that there are some usages of those in NewPipe).

When I don't replace the app:drawableXXXCompat the app works as expected and there is also no warning inside Android studio.

Could you add a reference where it is stated that you should rather use android:drawableXXX instead of app:drawableXXXCompat?
https://stackoverflow.com/questions/63479754/getting-appdrawableendcompat-instead-of-androiddrawableend-warning-for-lollipo

Anyway, I replaced the usages now.

@sonarcloud
Copy link

sonarcloud bot commented Jan 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Plugdemholes-Imeanpatchdembugs

Will there be a new seperate downloadable Legacy release for the last version that supported Android 4.4?
Would be a shame if this new Legacy release would just replace the old one, considering the last Legacy release for Android 4.1 still seems to work.

Asking because when I'm checking the changelog and the version of the current Legacy release, there doesn't seem to be any indication in the changelog that there was a change in minSdk in version 0.20.9 ?.?

@opusforlife2
Copy link
Collaborator

This will only be required once a security issue is found in OkHttp 3.x, right? What I mean is, could we keep releasing new versions with Kitkat support until we are forced to upgrade OkHttp to 4.x?

@Stypox
Copy link
Member

Stypox commented Jan 7, 2022

@opusforlife2 since OkHttp 3.x is now not supported anymore, it's just a matter of time before a vulnerability is found, and it would be difficult to know that a new vulnerability was found. So it is a good idea imo to just drop support for 4.4, upgrade to OkHttp 4.x and stay on the safe side

@TacoTheDank
Copy link
Member

TacoTheDank commented Jan 7, 2022

This is indeed a cause for celebration!

I can think of a few other compatibility things that can be removed, but they aren't that important. I can just create a PR for those if we do decide to go forward with dropping all legacy support (everything < 21, including NewPipe Legacy).

@opusforlife2
Copy link
Collaborator

So it is a good idea imo to just drop support for 4.4, upgrade to OkHttp 4.x and stay on the safe side

In that case, considering that 19 PRs are already merged for the next version, maybe we should only merge this PR and release ASAP.

@SameenAhnaf

This comment was marked as off-topic.

@litetex litetex force-pushed the increase-minsdk branch 2 times, most recently from 47fea6e to 4278844 Compare February 28, 2022 18:47
@litetex litetex marked this pull request as draft February 28, 2022 19:11
@litetex litetex marked this pull request as ready for review February 28, 2022 19:16
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me

@Stypox Stypox mentioned this pull request Mar 3, 2022
5 tasks
@TacoTheDank TacoTheDank added the codequality Improvements to the codebase to improve the code quality label Mar 6, 2022
@TacoTheDank
Copy link
Member

@litetex Could I rebase this PR to fix the conflicts?

@litetex
Copy link
Member Author

litetex commented Jul 6, 2022

@TacoTheDank Feel free to do so but note that this PR was create some months ago.
There are also certainly other things that need to be adjusted.

@TacoTheDank TacoTheDank force-pushed the increase-minsdk branch 3 times, most recently from 9600dee to fc28e96 Compare July 6, 2022 21:49
Copy link
Member Author

@litetex litetex left a comment

Choose a reason for hiding this comment

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

@TacoTheDank
Are you finished with your changes?

If there is nothing additional to do I think we can merge the branch.

@opusforlife2
Copy link
Collaborator

Are we done with all the stuff that was proposed to be merged before removing Kitkat support? I can't exactly remember what those things were, though I think DASH and new stream notifications were two of them.

@Stypox
Copy link
Member

Stypox commented Jul 11, 2022

@opusforlife2 yes, we are done; there has been a recent discussion about whether to also wait for tex's DASH-2, but we decided not to wait any longer.

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Took a look at the code, lgtm, thank you to everyone involved!

@leetfin

This comment was marked as spam.

@TacoTheDank
Copy link
Member

@TacoTheDank Are you finished with your changes?

If there is nothing additional to do I think we can merge the branch.

Yep, all done!

@Stypox
Copy link
Member

Stypox commented Jul 13, 2022

I tested some misc things and could not find issues

@Stypox Stypox merged commit bc37312 into TeamNewPipe:dev Jul 13, 2022
@sonarcloud
Copy link

sonarcloud bot commented Jul 13, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TacoTheDank TacoTheDank mentioned this pull request Jul 14, 2022
5 tasks
@triallax triallax mentioned this pull request Jul 14, 2022
5 tasks
@litetex litetex deleted the increase-minsdk branch July 15, 2022 17:24
@opusforlife2
Copy link
Collaborator

about whether to also wait for tex's DASH-2, but we decided not to wait any longer.

Isn't that purely the extractor? Extractor changes shouldn't have any influence on Legacy, no? I was under the impression that the front-end is what differs between NP and Legacy, which is why we wanted to add some critical front-end PRs before bumping.

Is that not the case?

@Stypox
Copy link
Member

Stypox commented Jul 18, 2022

@opusforlife2 no, DASH-2 will also require big front-end changes because the extractor API will be changing again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codequality Improvements to the codebase to improve the code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop support for Android 4.4 / Require Android 5 / API 21
9 participants