App update notification #1608

krtkush · 2018-08-16T20:20:36Z

I carefully read the contribution guidelines and agree to them.

Added feature described in #1520 and #1531.

Currently it is a very basic version. Will further build on this feature in the future.

…nd fdroid.

…p_update_notif

TobiGr

Hey @krtkush!
Thanks for working on this.
Unfortunately, I think there are some details, which we need to discuss first. Apart from that, I've pointed some things out in the code which should be improved. But I recommend you to wait with fixes until we have finished our discussion.

My biggest concern about these changes are the additional build flavors. When someone installs NewPipe from F-Droid, he/she will always be forced to use this source to update the application.
Additionally, with this new config every user needs to uninstall the current version of the app.
Not only for this reason, but also as update notifications can be disturbing from time to time, I suggest to add a setting which handles the update options:

Enable / Disable search for new updates
Preferred download mirror (applies when F-Droid and GitHub versions are the same, see below).

To identify whether the app is from F-Droid or GitHub we need to check the app's signature. A guide how this can be achieved can be found here.

IMHO we should come back to one release apk using F-Droid's reproducible builds feature.

TobiGr · 2018-08-16T21:26:45Z

app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java

@@ -663,6 +664,9 @@ public void onPlaying() {
            lockManager.acquireWifiAndCpu();

            hideControls(DEFAULT_CONTROLS_DURATION, DEFAULT_CONTROLS_HIDE_TIME);
+
+            // Check for new version
+            new CheckForNewAppVersionTask().execute();


Why do you run the check here, too?

From what I remember the check was not being performed in the pop-up mode. Hence, I had to put it here. I'll check once again and make changes if needed.

TobiGr · 2018-08-16T21:26:56Z

app/src/main/java/org/schabi/newpipe/CheckForNewAppVersionTask.java

+    @Override
+    protected void onPostExecute(String response) {
+
+        Log.i("Response--", response);


This entry does not help us to find issues in the future. Please add some additional information or remove this log entry.

This was not supposed to go into production. I'll remove it.

app/src/main/java/org/schabi/newpipe/CheckForNewAppVersionTask.java

theScrabi · 2018-08-17T14:55:01Z

My biggest concern about these changes are the additional build flavors. When someone installs NewPipe from F-Droid, he/she will always be forced to use this source to update the application.

Due to different signatures you already have to reisntall the app if you want to use the fdroid version.

Additionally, with this new config every user needs to uninstall the current version of the app.

How comes? As long as the package name and signing key does not change nothing needs to be re installed.

Enable / Disable search for new updates

I agree. Although it should be enabled by default.

TobiGr · 2018-08-17T15:56:38Z

Due to different signatures you already have to reisntall the app if you want to use the fdroid version.

That's true, but I'd like to get back to one version.

How comes? As long as the package name and signing key does not change nothing needs to be re installed.

flavorDimensions "apkSource"
    productFlavors {
        github {
            dimension "apkSource"
            applicationIdSuffix ".github"

        }

        fdroid {
            dimension "apkSource"
            applicationIdSuffix ".fdroid"
        }
}

This means, we now have four different builds:
githubRelease
githubDebug
fdroidRelease
fdroidDebug

None of them matches our release build because these new have suffixes. This means an upgrade is not possible.

theScrabi · 2018-08-17T17:47:24Z

But do we need the suffix?

I am with you though, I think trying to make the reproducible build work might not be a bad idea.

TobiGr · 2018-08-17T20:08:18Z

The current implementation uses the new build flavors to decide whether to check for an update. As mentioned above, there are other ways to do this (e.g. the signature). So if this is implemented in a different way, we won't need a suffix.

krtkush · 2018-08-20T02:58:38Z

Hey @TobiGr, @theScrabi

I agree - A single version will be a better approach; I'll look into replacing the flavor with signature check.

theScrabi · 2018-09-11T14:48:14Z

@krtkush @TobiGr so here are some updates. After the youtube amageddon that made us release v0.14.1 a few days ago, I had a conversation with the fdroid people about update speed.

It seems to be a complicated topic which seem to not get a widespread acceptance within their team. Therefore we should try and even deliver updates ourself even if its a version deploid by fdroid.

This on one hand makes things easier for us since we don't have to deploy two separate versions, and on the other hand we need to make NewPipe a reproducable build within fdroid.

@krtkush could you please try to fix the conflicts, and @TobiGr would you please review it after the changes are done.

I am willing to put this feature into the next version if it's possible :)

krtkush · 2018-09-11T15:32:38Z

@theScrabi
Sure! I'll work on it this weekend.

Just to be clear, we'll be going with the signature check method as suggested by @TobiGr, right? And not with different builds.

theScrabi · 2018-09-11T17:47:48Z

Yes the signature check method.

…p_update_notif

krtkush · 2018-09-15T09:28:24Z

@TobiGr @theScrabi Does NewPipe use different KeyStores for the github and f-droid versions respectively?

I'll need the developer certificate signature of the KeyStore(s), as instructed in the linked article. Could either of you provide that?

TobiGr · 2018-10-04T17:45:25Z

@krtkush Sorry for the silence. We are all quite busy. If I am correct, you can find the fingerprint on @theScrabi's website: https://schabi.org If you need any further information about the signing key, please ask him :)

krtkush · 2018-10-06T13:20:24Z

Thanks @TobiGr!

@theScrabi Can you confirm both the keys? On the website I only see one - SHA1: B0:2E:90:7C:1C:D6:FC:57:C3:35:F0:88:D0:8F:50:5F:94:E4:D2:15

TobiGr · 2018-11-23T20:25:11Z

@krtkush Thanks, I was just about to test it.

theScrabi

Will block until issue with vectors is done.

krtkush · 2018-11-25T19:50:27Z

Done. I really hope it is actually done this time 😛

TobiGr · 2018-12-03T20:06:24Z

@krtkush Looks good to merge. Thanks for changing the code that often!
@theScrabi Can you please remove your block?

theScrabi · 2018-12-15T07:30:10Z

Done.

…wPipe into 1520_app_update_notif

krtkush · 2019-01-19T06:27:37Z

Hey @theScrabi, I was wondering why the latest release not have this feature included?

theScrabi · 2019-01-20T01:33:08Z

I wanted to take care about pending features once my semester is over. However the sudden release came because of the full shutdown on Thursday.

krtkush · 2019-01-20T06:24:38Z

full shutdown on Thursday

Ohkay. I wasn't aware of this.

Good luck with your semester.

snaik20 · 2023-09-04T18:35:07Z

app/src/main/java/org/schabi/newpipe/CheckForNewAppVersionTask.java

+        if (!mPrefs.getBoolean(app.getString(R.string.update_app_key), true)
+                || !isGithubApk()) {
+            this.cancel(true);
+        }


Checking this before starting the task would be more efficient, at App#L108

This is 4 years old code, is this still the case?

Hi @Stypox,

I apologize for my previous comment. I didn't realize that the code was 4 years old, and I see that it has now been deleted. I was going through the code to explore and understand the codebase, but I should have been more careful about commenting on old code.

I'll be more mindful of the age of code in the future and avoid commenting on old code unless I'm sure it's still relevant.

Thanks for your patience and understanding.

krtkush added 12 commits August 11, 2018 19:13

Added HTTPS request to get version data. Added APK flaor for github a…

75a44fb

…nd fdroid.

Moved the asynctask to its own class.

5e2aa51

Added notification channel and code to show notification.

f85e19c

Merge branch 'dev' of https://github.com/krtkush/NewPipe into 1520_ap…

7c875a8

…p_update_notif

Moved the new version check to the application class.

06f20c6

Added version check in the pop-up player

930c971

Added new icon for update notification.

12b93d6

Code refactored and added comments.

af42e32

More refactoring.

2a18eac

Added version code check.

e7abeb5

Bug fix.

04e974b

Removed debug code

910c10f

TobiGr requested changes Aug 16, 2018

View reviewed changes

TobiGr mentioned this pull request Aug 16, 2018

Auto upgrade Newpipe #1531

Closed

krtkush added 4 commits September 15, 2018 12:51

Pull request changes begins here.

17197ad

Conflict resolution.

395c958

Merge branch 'dev' of https://github.com/krtkush/NewPipe into 1520_ap…

cde5f7d

…p_update_notif

Pull request changes v1.

6417bd9

Removed flvor checks. Added update settings under main settings.

7124d9b

theScrabi requested changes Nov 25, 2018

View reviewed changes

vector -> png

069654c

krtkush dismissed TobiGr’s stale review via 069654c November 25, 2018 19:48

Test code revert.

c864b15

krtkush and others added 2 commits December 2, 2018 18:07

Merge branch 'dev' into 1520_app_update_notif

7ed460c

Merge branch 'dev' into 1520_app_update_notif

f62bfea

TobiGr previously approved these changes Dec 3, 2018

View reviewed changes

krtkush added 2 commits December 5, 2018 13:04

Merge branch 'dev' into 1520_app_update_notif

bfda8dc

Merge branch 'dev' into 1520_app_update_notif

67499bd

theScrabi previously approved these changes Dec 15, 2018

View reviewed changes

gkeegan mentioned this pull request Dec 21, 2018

[feature request] update notification #1957

Closed

krtkush added 2 commits December 28, 2018 18:07

Conflict resolution.

b674006

Merge branch '1520_app_update_notif' of https://github.com/krtkush/Ne…

794c370

…wPipe into 1520_app_update_notif

krtkush dismissed stale reviews from theScrabi and TobiGr via 794c370 December 28, 2018 12:39

theScrabi mentioned this pull request Jan 23, 2019

Question / Enhancement: Independently upgradable extractor / parser #2045

Closed

3 tasks

theScrabi merged commit 794c370 into TeamNewPipe:dev Jan 23, 2019

This was referenced Jan 24, 2019

Set up own F-Droid repo to push urgent updates to users #1981

Closed

release v0.15.1 #2067

Merged

Release v0.15.1 #2068

Closed

Pentaphon mentioned this pull request Jun 29, 2019

Option to notify user on startup when there's a new version on Github. Adamantcheese/Kuroba#149

Closed

krtkush mentioned this pull request Jul 16, 2019

Automatic updates #2361

Closed

snaik20 reviewed Sep 4, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

App update notification #1608

App update notification #1608

krtkush commented Aug 16, 2018

TobiGr left a comment

TobiGr Aug 16, 2018

krtkush Aug 20, 2018

TobiGr Aug 16, 2018

krtkush Aug 20, 2018

theScrabi commented Aug 17, 2018

TobiGr commented Aug 17, 2018

theScrabi commented Aug 17, 2018

TobiGr commented Aug 17, 2018

krtkush commented Aug 20, 2018

theScrabi commented Sep 11, 2018

krtkush commented Sep 11, 2018

theScrabi commented Sep 11, 2018

krtkush commented Sep 15, 2018

TobiGr commented Oct 4, 2018

krtkush commented Oct 6, 2018 •

edited

Loading

TobiGr commented Nov 23, 2018

theScrabi left a comment

krtkush commented Nov 25, 2018

TobiGr commented Dec 3, 2018 •

edited

Loading

theScrabi commented Dec 15, 2018

krtkush commented Jan 19, 2019

theScrabi commented Jan 20, 2019

krtkush commented Jan 20, 2019

snaik20 Sep 4, 2023

Stypox Sep 4, 2023

snaik20 Sep 16, 2023

App update notification #1608

App update notification #1608

Conversation

krtkush commented Aug 16, 2018

TobiGr left a comment

Choose a reason for hiding this comment

TobiGr Aug 16, 2018

Choose a reason for hiding this comment

krtkush Aug 20, 2018

Choose a reason for hiding this comment

TobiGr Aug 16, 2018

Choose a reason for hiding this comment

krtkush Aug 20, 2018

Choose a reason for hiding this comment

theScrabi commented Aug 17, 2018

TobiGr commented Aug 17, 2018

theScrabi commented Aug 17, 2018

TobiGr commented Aug 17, 2018

krtkush commented Aug 20, 2018

theScrabi commented Sep 11, 2018

krtkush commented Sep 11, 2018

theScrabi commented Sep 11, 2018

krtkush commented Sep 15, 2018

TobiGr commented Oct 4, 2018

krtkush commented Oct 6, 2018 • edited Loading

TobiGr commented Nov 23, 2018

theScrabi left a comment

Choose a reason for hiding this comment

krtkush commented Nov 25, 2018

TobiGr commented Dec 3, 2018 • edited Loading

theScrabi commented Dec 15, 2018

krtkush commented Jan 19, 2019

theScrabi commented Jan 20, 2019

krtkush commented Jan 20, 2019

snaik20 Sep 4, 2023

Choose a reason for hiding this comment

Stypox Sep 4, 2023

Choose a reason for hiding this comment

snaik20 Sep 16, 2023

Choose a reason for hiding this comment

krtkush commented Oct 6, 2018 •

edited

Loading

TobiGr commented Dec 3, 2018 •

edited

Loading