Add different release channels for autoupdater #2362

ctrlaltca · 2017-01-15T14:24:46Z

This is an initial implementation of #2321.
Currently two release channels are implemented: "stable releases" and "development snapshot", respectively pointing to our stable releases and git builds on bintray.

Users can choose their preferred release channel from the settings:

Their choice is reflected in the update dialog:

Also, remove the wrong commit hash from update dialog and show version name instead (fix #2247);
Still to be fixed / reworked, but maybe in another PR: don't use release date to determine if a newer version is available (aka avoid #2338)

ZeldaZach · 2017-01-15T15:01:36Z

Just a thought to go with; git rev-list --count HEAD will give us a count of how many commits were made to the HEAD, which we can possibly use a confirmation. When a client starts up, it can run that commit and compare the counts. If they're not equal (aka HEAD > version string) then we show them update message.

$ git rev-list --count 784a75db
3539
$ git rev-list --count HEAD
3545

tooomm · 2017-01-15T15:59:16Z

cockatrice/src/dlg_settings.cpp

-    updateNotificationCheckBox.setText(tr("Notify when new client features are available"));
+    clearDownloadedPicsButton.setText(tr("Reset/clear downloaded pictures"));
+    updateReleaseChannelLabel.setText(tr("Update channel"));
+    updateNotificationCheckBox.setText(tr("Notify when a new version is available"));


Do we have any logic for automatic update checks yet?! First time I hear about that. :)

Not really; it still works that when you connect to a server and it tells you that you are missing some feature, we suppose that probably a new version supporting that feature is out.
I't just that imho the old text, even if formally correct, made no sense at all to an average user.

I think the wording is still misleading...

Maybe add a "on connect" and rephrase a bit?
"On connect, notify everytime a feature supported by the server is missing in my client"
or "repeatedly" instead everytime

I've tried to change the sentence, but it's so long that it forces the settings window to be wider. So the candidates are:

# original wording Notify when new client features are available # short, misleading wording Notify when a new version is available # Precise but too long wording On connect, notify everytime a feature supported by the server is missing in my client # Other attempts Notify if a feature supported by the server is missing in my client Notify if the server reports that a feature is missing in my client

Others?

EDIT: i found out a way to make the sentence can take up more space in the window without affecting its size, so even the longer version is viable.

Notify if a feature supported by the server is missing in my client

I really like this one but I don't have strong feelings - use your judgement

tooomm · 2017-01-15T16:11:51Z

@ctrlaltca

Also, remove the wrong commit hash from update dialog and show version name instead (fix #2247);

Can you show a picture, please?

I want to throw in #2359 again.
Now we should introduce the propper wording for the os on mac computers and scrap legacy terms.
We need to make sure that the app itself checks the new/right location though.

ctrlaltca · 2017-01-16T20:20:35Z

Just a thought to go with; git rev-list --count HEAD will give us a count of how many commits were made to the HEAD, which we can possibly use a confirmation. When a client starts up, it can run that commit and compare the counts. If they're not equal (aka HEAD > version string) then we show them update message.

That could work, unless if the user want to switch back from a development version to a stable version; just checking for HEAD != version string could be a simple workaround.
Anyway, the main problem is being able to push a value inside is the file that we load from bintray when checking for new versions (https://api.bintray.com/packages/cockatrice/Cockatrice/Cockatrice/files), so that Cockatrice is able to get this information. I'll dig into bintray's documentation, hopefully we can customize that file.
As an alternative, we could host a json file on cockatrice's repo.

Can you show a picture, please?

Development snapshot update

Stable update:

I want to throw in #2359 again.

Please, let me fix this once before we break it again ;)

ZeldaZach · 2017-01-16T21:04:40Z

I like the option with the dev/release channels; Looks good. If you're on a dev channel, will it allow you to go back to release and download the latest release with a notification or will you have to hit "download anyway?"

Having a JSON file tell which is the releases could be done, and we can host it on the /latest/release portion of the client as an uploaded file (github.com/cockatrice/cockatrice/releases/latest)

Daenyth · 2017-01-18T15:32:26Z

I don't think we need to do a manual releases file, we should be able to get the information from the releases page on github, or via the releases api.

…

On Mon, Jan 16, 2017 at 4:04 PM Zach H ***@***.***> wrote: I like the option with the dev/release channels; Looks good. If you're on a dev channel, will it allow you to go back to release and download the latest release with a notification or will you have to hit "download anyway?" Having a JSON file tell which is the releases could be done, and we can host it on the /latest/release portion of the client as an uploaded file ( github.com/cockatrice/cockatrice/releases/latest) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2362 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA5NH3A1Rbse3N9q-T1T7i-pJM63wXxks5rS9togaJpZM4Lj7ek> .

ctrlaltca · 2017-01-18T15:33:41Z

I don't think we need to do a manual releases file, we should be able to
get the information from the releases page on github, or via the releases
api.

How can we link the release information from github to the downloadable files from bintray?

Daenyth · 2017-01-18T15:43:11Z

We could use a git tag with a specific structure. We talked about using semver in another ticket, if we put that info into the git tags it would be enough to determine new releases.

…

On Wed, Jan 18, 2017 at 10:33 AM ctrlaltca ***@***.***> wrote: I don't think we need to do a manual releases file, we should be able to get the information from the releases page on github, or via the releases api. How can we link the release information from github to the downloadable files from bintray? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2362 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA5NGYrhW5Z3BRUAivhtwaTZ-o_i0Iwks5rTjDVgaJpZM4Lj7ek> .

tooomm · 2017-01-18T20:08:47Z

Regarding the release notes there is a Release notes tab on bintray, I'm pretty sure it used to display information from github releases the same as it's displaying information from the README.md file in the corresponding tab once. Not working anymore though?!

woogerboy21 · 2017-01-19T00:43:46Z

Compiled and tinkered a bit. Any chance you could throw some info in the debug log when the update gets checked with some type of usable info? Like maybe current version, versions found and results? Something like that.

ctrlaltca · 2017-01-19T08:08:24Z

We could use a git tag with a specific structure. We talked about using
semver in another ticket, if we put that info into the git tags it would be
enough to determine new releases.

So, for stable releases:

Cockatrice grabs https://api.github.com/repos/Cockatrice/Cockatrice/releases/latest
We show the user the update popup with

"name": "Cockatrice: All Hallow's Eve, Revision 1",
"published_at": "2016-12-31T23:55:38Z",
link to "html_url": "https://github.com/Cockatrice/Cockatrice/releases/tag/2016-12-31-Release",

Cockatrice will note the tag name: "tag_name": "2016-12-31-Release" and look for the same tag name on bintray: https://api.bintray.com/packages/cockatrice/Cockatrice/Cockatrice/files (this will be easier if we tag using semver instead of plain names)
cockatrice will download a matching package from bintray

For development releases:

Grab https://api.github.com/repos/Cockatrice/Cockatrice/commits , only consider the most recent one
We show the user the update popup with

commit->message: "Fix use of uninitalized value (Fix use of uninitalized value #2364)",
commit->author->date: "2017-01-16T20:06:35Z"
link to "html_url": "ae5fc1f",

Cockatrice will note the commit "sha": "ae5fc1fe2c7d89af7e9899f0af3f8728971eb073", trucate it to the short hash "ae5fc1f" and look for the same tag name on bintray: https://api.bintray.com/packages/cockatrice/Cockatrice/Cockatrice-git/files
cockatrice will download a matching package from bintray

Is this what you mean? If we want to take this route i'll start working on it

Regarding the release notes there is a Release notes tab on bintray, I'm pretty sure it used to display information from github releases the same as it's displaying information from the README.md file in the corresponding tab once. Not working anymore though?!

Probably someone copypasted them manually. Bintray can be configured to fetch data from github, but imho we'd better redirect people on github, since all the informations are already here.

Compiled and tinkered a bit. Any chance you could throw some info in the debug log when the update gets checked with some type of usable info? Like maybe current version, versions found and results? Something like that.

Sure, i will add some debug!

Daenyth · 2017-01-19T15:28:00Z

Yes, that's exactly what I mean

tooomm · 2017-01-20T16:23:12Z

Probably someone copypasted them manually. Bintray can be configured to fetch (release notes) data from github, but imho we'd better redirect people on github, since all the informations are already here.

Link within the release notes tab of stable releases to https://github.com/Cockatrice/Cockatrice/releases/latest ?

You asked in gitter: Regarding release names we had a discussion here: #2341

Edit:
Can we explain the different channels somewhere? Put an information "(i)" next to it in settings which gives some information on moues over for example?
Especially since the development branch is most likely out of date all the time, people should know about it for sure, and that they are running "on the edge".

It would be better to have (additionally) a beta and/or release candidate channel, which both make way more sense to the users and the project in general.
People will get bored and annoyed by updates all the time in the development branch, so they most likely won't stay there. But we want some parts of the userbase to use beta/rc builds to give feedback and actually test+report things. Once people realize that bugs are fixed faster there, and that they don't have to wait weeks/month for fixes in the next stable it's also a good benefit for them.
We could put folders in the normal cockatrice project on bintray or use the extendend file names as talked about in #2341 to destinguish them for example.

ZeldaZach · 2017-02-08T18:27:14Z

Any word on what we should do with this @Daenyth ?

ctrlaltca · 2017-02-20T13:39:53Z

Any word on what we should do with this @Daenyth ?

I'm still (slowly) working on implementing the changes

ctrlaltca · 2017-02-21T23:44:53Z

Still work in progress, but updates for the development version are now implemented as discussed in #2362 (comment)

ctrlaltca · 2017-02-22T10:13:40Z

Both updates for the stable channel and the development channel have been implemented, and debug messages added as requested. This can be tested by reviewers.

On the stable channel it always download the last stable release available on bintray, since there's no way to actually link these files to the release on github, since both the release and the files are created manually; you may think of naming it the same and use this as a link, but this is fragile and a big con is the fact that on bintray the release can't be modified after the creation. We would risk to end up with the same problem of the last 2 releases where a not-matching date broke the update procedure.

woogerboy21 · 2017-02-25T21:32:28Z

Tinkered around some more. Curious about something. When running the most recent commit level with this PR rolled in and using the stable release option I have a popup that shows an update is available and lists the current stable release version that has been published. Is this by design? What I mean by this is since I am running a newer client than what is available in the stable version are we expecting the client to say "hey there is a more recent stable version than what your running (since your not running a stable release)" or are we wanting it to say "hey this client is newer than the stable release available, do you want to install the stable release?"

Just want to make sure 1, the logic isnt actually failing to identify the correct versions and not informing people of updates when there is none and 2, expected behavior.

ctrlaltca · 2017-02-26T18:47:48Z

This PR is not using the "release date" to compare the current running version with the last version available for download, since we can't really control that information on bintray. This was the problem that caused the "update loop" in the two last releases.
The code from this PR is using the git commit hash embedded in cockatrice, comparing it to the "last version" commit hash obtained from github.
If you run cockatrice from a release channel (stable or dev) and you decide to change it to the other release channel you'll always find a different "last" version available, since it will surely have a different git commit hash.
There is no concept of a "newer" release, since dates are not involved in comparing the currently running version and the one available online.

Daenyth · 2017-02-27T13:50:19Z

@ctrlaltca What's the status of this at the moment? Are you waiting for code review, waiting for manual testing, or still making changes? I'd love to help push this through so if it's code review let me know and I'll get to it as I have time

ctrlaltca · 2017-02-27T14:07:04Z

@Daenyth Waiting for review/test

Daenyth

I only have minor feedback points - once they are addressed and this is tested manually please go ahead and merge unless you think you need more specific feedback from me.

Daenyth · 2017-02-28T01:04:06Z

cockatrice/src/dlg_update.cpp

+        publishDate = release->getPublishDate().toString(Qt::DefaultLocaleLongDate);
+
+        QMessageBox::StandardButton reply;
+        reply = QMessageBox::question(this, "Update Available",


It seems like this is mixing UI and business logic together. An alternate approach might be to have the release updater not use any UI components but instead provide a function of UpdateCheckResult checkForUpdates() where UpdateCheckResult can be a tagged union with success+download_url, success-but-no-url-for-your-OS, you-are-up-to-date, or error-updating+the_error.

Does that make sense?

That would allow us to unit test in isolation from the UI.

Anyway I'm not going to block the PR on that change, but I'd like to hear your thoughts on it.

Also it seems like tagged unions are a goddamn PITA in cpp: http://en.cppreference.com/w/cpp/language/union

Thoughts on rust? I do have some docs on incorporating rust into a cpp app. I'd be perfectly willing to start extracting all the core bits to safer rust libraries and leave the Qt parts for just UI calling functions exposed from rust FFI.

The business logic is mostly implemented in releasechannel.cpp, but these last checks ust be made in the UI since it's good to have the QMessageBox instanciated with a valid parent widget.
basically we are calling settingsCache->getUpdateReleaseChannel()->checkForUpdates(); to start the check for updates and then waiting on the DlgUpdate::finishedUpdateCheck() slot for the result. It must be an async call anyway since the network is involved, so we cant just call a method and get the result directly.

Daenyth · 2017-02-28T01:07:53Z

cockatrice/src/releasechannel.cpp

+#define DEVMANUALDOWNLOAD_URL "https://bintray.com/cockatrice/Cockatrice/Cockatrice-git/_latestVersion#files"
+#define GIT_SHORT_HASH_LEN 7
+
+ReleaseChannel::ReleaseChannel(int _index, QString _name)


Seems like the last 2 fields may want to be an UpdateResponse type object instead of the checker class itself.

I moved them inside the Release channels in the last update.

Daenyth · 2017-02-28T01:09:52Z

cockatrice/src/releasechannel.cpp

+
+    if(!(resultMap.contains("object") && 
+        resultMap["object"].toMap().contains("sha"))) {
+        qDebug() << "Invalid received from the tag update server";


Is it easy to include the contents of resultMap here? Might help. Also probably should be qWarning at least

Daenyth · 2017-02-28T01:10:58Z

cockatrice/src/releasechannel.cpp

+void DevReleaseChannel::checkForUpdates()
+{
+    qDebug() << "Searching for updates on the dev channel: " << DEVRELEASE_URL;
+    response = netMan->get(QNetworkRequest(QString(DEVRELEASE_URL)));


This code is almost identical to the other class - it seems like url and log message are the only difference. Might be good to make it a method of the base interface they both extend and take the urls as parameters

Daenyth · 2017-02-28T01:11:08Z

cockatrice/src/releasechannel.cpp

+    QString tmp = QString(reply->readAll());
+    reply->deleteLater();
+
+    // Errors are incapsulated in an object, check for them first


encapsulated

Removed the comment altogether, it was a leftover from a copypaste

Daenyth · 2017-02-28T01:11:46Z

cockatrice/src/settingscache.cpp

@@ -162,7 +164,16 @@ SettingsCache::SettingsCache()
    if(!QFile(settingsPath+"global.ini").exists())
        translateLegacySettings();

+    // updates
+    dummy = QT_TRANSLATE_NOOP("i18n", "Stable releases");
+    dummy = QT_TRANSLATE_NOOP("i18n", "Development snapshots");


What's that do?

It was used to force qt's l'update to find and extract these translatable strings.
I moved the strings inside the *ReleaseChannel classes and removed this trick.

Daenyth · 2017-02-28T01:12:44Z

oracle/CMakeLists.txt

@@ -23,7 +23,9 @@ SET(oracle_SOURCES
    ../cockatrice/src/settings/layoutssettings.cpp
    ../cockatrice/src/thememanager.cpp 
    ../cockatrice/src/qt-json/json.cpp
- )
+    ../cockatrice/src/releasechannel.cpp
+     ${VERSION_STRING_CPP}


version cpp wasn't here already? How silly. We should probably make cockatrice's about menu reusable and call the same code from cockatrice and oracle. Not in this pr though

It was not included in oracle yet; now it's a dependency since oracle uses cockatrice's settingscache.

ZeldaZach · 2017-02-28T17:37:04Z

cockatrice/src/dlg_update.cpp

-                tr("Cockatrice was not built with SSL support, so cannot download updates! "
-                           "Please visit the download page and update manually."));
+        QMessageBox::critical(this, tr("Error"),
+            tr("Cockatrice was not built with SSL support, so cannot download updates! "


"Cockatrice was not build with SSL support, so you cannot download updates automatically! Please visit the download page to update manually."

ZeldaZach · 2017-02-28T17:43:31Z

cockatrice/src/releasechannel.cpp

+    foreach(QVariant file, resultList)
+    {
+        QVariantMap map = file.toMap();
+        // TODO: map github version to bintray version


any word on this?

Explained in the 2nd paragraph of #2362 (comment)
Unfortunately i've not found a "perfect solution" yet, even though it's working anyway.

Sounds good for now, if we have to change uploads what will we do though? Have to publish a new revision?

Publish a new revision to make cockatrice "find" the update.
Then, it will always download the "latest" stable release available on bintray.

ZeldaZach · 2017-02-28T17:44:40Z

cockatrice/src/releasechannel.h

+    ~ReleaseChannel();
+protected:
+	int index;
+	QString name;


Fixed, thank you

ZeldaZach

After my review, as long as the small nitty gritty stuff is fixed up, I'm good to see this go in. Glad to finally have something like this after all the years :)

ZeldaZach · 2017-02-28T19:02:48Z

Builds seem to be failing @ctrlaltca

Daenyth · 2017-02-28T19:57:51Z

Seems likely to be aws-related given that it's a connection error

Nightfirecat · 2017-02-28T21:58:00Z

AWS has been facing a lot of issues today, most notably S3 being nearly fully out of operation for a few hours. That said, the error seems to be with a failing Invoke-WebRequest to download compiled protobuf 2.6.1 code: https://ci.appveyor.com/project/Daenyth/cockatrice-8k7y5/build/0.0.1-branch-master-build-565/job/2jfjveibhmpp2xof#L282, https://ci.appveyor.com/project/Daenyth/cockatrice-8k7y5/build/0.0.1-branch-master-build-565/job/df442gc7xk7rgavd#L282

Fetch releases from github and find corresponding installers on bintray

ctrlaltca · 2017-03-01T07:53:42Z

I've squashed down commits and pushed to the PR to force the builds to run again.
If everything's ok then i'll merge.

ZeldaZach · 2017-03-01T08:32:58Z

I'll be looking into publishing a release on ~ March 14th (Pi Day :D)

ctrlaltca · 2017-03-01T08:42:58Z

Looks like the travis-ci build didn't trigger.. anyway it worked on the very same commit for my branch: https://travis-ci.org/ctrlaltca/Cockatrice/builds/206512853

tooomm reviewed Jan 15, 2017

View reviewed changes

tooomm mentioned this pull request Jan 18, 2017

updated mac wording for bintray deployment #2359

Merged

tooomm mentioned this pull request Feb 19, 2017

Check for updates to mtgjson automatically #2416

Closed

5 tasks

ctrlaltca force-pushed the issue_2321 branch from 757863e to fc5a028 Compare February 22, 2017 10:08

Daenyth approved these changes Feb 28, 2017

View reviewed changes

ZeldaZach reviewed Feb 28, 2017

View reviewed changes

ZeldaZach approved these changes Feb 28, 2017

View reviewed changes

Add release channels for autoupdater

c568685

Fetch releases from github and find corresponding installers on bintray

ctrlaltca force-pushed the issue_2321 branch from f4d436a to c568685 Compare March 1, 2017 07:52

ctrlaltca merged commit 7373819 into Cockatrice:master Mar 1, 2017

ctrlaltca deleted the issue_2321 branch March 1, 2017 08:43

This was referenced Mar 1, 2017

Infinite update loop #2338

Closed

Add release channels to auto updater #2321

Closed

tooomm mentioned this pull request Mar 2, 2017

Add link to release notes to "update dialog" #2248

Closed

This was referenced Mar 12, 2017

Missing information in the client updater #1784

Closed

rephrase missing feature setting #2462

Merged

several issues with new release channels #2465

Closed

GuillaumeSeren mentioned this pull request Nov 29, 2023

games-board/cockatrice: Bump to 20230914 gentoo/gentoo#34008

Closed

Add different release channels for autoupdater #2362

Add different release channels for autoupdater #2362

Conversation

ctrlaltca commented Jan 15, 2017 • edited by ZeldaZach Loading

ZeldaZach commented Jan 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tooomm Jan 15, 2017 • edited Loading

Choose a reason for hiding this comment

ctrlaltca Jan 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tooomm commented Jan 15, 2017

ctrlaltca commented Jan 16, 2017

ZeldaZach commented Jan 16, 2017

Daenyth commented Jan 18, 2017 via email

ctrlaltca commented Jan 18, 2017

Daenyth commented Jan 18, 2017 via email

tooomm commented Jan 18, 2017

woogerboy21 commented Jan 19, 2017

ctrlaltca commented Jan 19, 2017

Daenyth commented Jan 19, 2017

tooomm commented Jan 20, 2017 • edited Loading

ZeldaZach commented Feb 8, 2017

ctrlaltca commented Feb 20, 2017

ctrlaltca commented Feb 21, 2017

ctrlaltca commented Feb 22, 2017

woogerboy21 commented Feb 25, 2017 • edited Loading

ctrlaltca commented Feb 26, 2017 • edited Loading

Daenyth commented Feb 27, 2017

ctrlaltca commented Feb 27, 2017

Daenyth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZeldaZach left a comment

Choose a reason for hiding this comment

ZeldaZach commented Feb 28, 2017

Daenyth commented Feb 28, 2017

Nightfirecat commented Feb 28, 2017

ctrlaltca commented Mar 1, 2017 • edited Loading

ZeldaZach commented Mar 1, 2017

ctrlaltca commented Mar 1, 2017 • edited Loading

ctrlaltca commented Jan 15, 2017 •

edited by ZeldaZach

Loading

tooomm Jan 15, 2017 •

edited

Loading

ctrlaltca Jan 16, 2017 •

edited

Loading

tooomm commented Jan 20, 2017 •

edited

Loading

woogerboy21 commented Feb 25, 2017 •

edited

Loading

ctrlaltca commented Feb 26, 2017 •

edited

Loading

ctrlaltca commented Mar 1, 2017 •

edited

Loading

ctrlaltca commented Mar 1, 2017 •

edited

Loading