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

pull updates from gh #408

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

zomfg
Copy link

@zomfg zomfg commented Nov 15, 2020

This switches update check to here directly
convenient api endpoint and builtin QVersionNumber simplify the code a bit
should work the same otherwise


On a related note (where this started)...
I wanted to check out gh workflows and long story short, I managed to make one that builds Prismatik for all platforms
and I think it would be nice to have it here:

  • everybody will be able to use it super easily in their forks and without waiting for a "ok to test"
  • we, mere mortals, won't have to depend as much on your setup, and bother you with versions of stuff

and it can be applied for the releases, I tested the following:

  • when a PR is merged
    • everything is built
    • a pre-release is published (this would not show up in the end point for this PR, but it's an easy change if desired) with
      • tag: last actual release +1: 5.11.2.25 -> 5.11.2.26-preXXX (where XXX could be a datetime(YmdHMS) or timestamp..)
      • body: "PR title (#xx)"
  • then, when you think there's enough stuff for a full release
    • do the usual housekeeping (changelog, version bumps in files)
    • tag 5.11.2.26 and push
    • everything is built and a release is drafted with all the binaries (the tag is the trigger, not just commits to master)
    • you redact your notes and click publish
    • done

Now, making a pre-release for every merge would be too much, so it'll need some kind of rule, which could be:

This would avoid people rolling back too far and/or waiting months for something new/fixed

And last but not least:
I streamlined the linux build process a bit, and combined with the above it's now easy to add a build + packaging step for whatever distribution / package manager: so debian 10.x will get an actual debian 10.x build with whatever versions it currently has, the freshest ubuntu will have it's own, Arch won't get an old deb repackage (I use Arch btw), a flatpak for the cool kids etc
This works through docker on github, and locally you can do both docker or a native build if you happen to run the target system (which is what the docker container runs internally).
So others can setup a package for their OS without you having to mess with your build server and it'll just pop in the next release.

On windows side, I skipped the whole UpdateElevate step for now, but it should not be too hard to add (unless you think otherwise), you can add the private key to your gh secrets and we'll pull from there
their windows env 2019 2016
so if you have some particular instructions for this part...

and mac does pretty much the travis routine...

Thoughts?

@zomfg zomfg mentioned this pull request Nov 26, 2020
@psieg
Copy link
Owner

psieg commented Dec 2, 2020

The workflows thing sounds like a good idea, when I set up that Jenkins free cloud CI wasn't really a thing yet. I was looking into Travis-Windows at some point but it was still very experimental at that point. Pre-releases for everything might be too much, I'd rather provide downloads only for real ones. People who want to try pre-releases are usually able to build themselves as well ;)

What I don't like about switching the update check is that

  • Firmware updates are no longer a thing (admittedly, it's unlikely there will be another one)
  • Manual control. With the AV issues I like being able to publish a new version and enable the auto-udpater/notification to pull it only a week or so later if nothing came up from people who have manually updated. I think that's not possible using the GH API, right?

@zomfg
Copy link
Author

zomfg commented Dec 2, 2020

pre-releases not for everything, just the ones that are significant enough, so no spam, they don't show up in that api call (but that can be added)
the cycle is kinda slow as is, so maybe this will allow you to do small review/merge here and there instead of waiting to have enough time to go over everything, maybe...
Nah I don't agree about people easily making their own builds, the ones who can will also solve their issues, yet people ask for trivial stuff like bump device LED limits, I bet they'd be glad to have a test build (speaking of which, why not unlock that?)

and

  • when you'll have a new firmware, you just add it to the release assets, and then we'll have to add a check for it in the code (just like it's looking specifically for the .exe here). The only issue I see is that if you don't add the binary to the release after that one it won't show up for those that did not update. To solve this either attach previous firmware in every release (could be part of the workflow I guess), or setup a secondary dummy FW repo with it's own release endpoint to pull
  • We can just add "the release must be X days old" condition
    but also pre-releases will help detect some of these issues before the final one ;)

update.url = releaseObject.value(QStringLiteral("html_url")).toString();
update.text = releaseObject.value(QStringLiteral("body")).toString();
update.title = releaseObject.value(QStringLiteral("name")).toString();
update.softwareVersion = QVersionNumber::fromString(releaseObject.value(QStringLiteral("tag_name")).toString());
Copy link
Owner

Choose a reason for hiding this comment

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

What if this fails, e.g. "FW_version_6+1"?

Copy link
Author

Choose a reason for hiding this comment

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

softwareVersion will be isNull() internally and compare() will treat it as v0, so no update

@psieg
Copy link
Owner

psieg commented Dec 2, 2020

If desired one can always retrieve the master build from the CI status. I'd rather release more often than having people run pre-versions a lot. If the rest of the process is even more automated the overhead is lower...

  • Including a new firmware in all releases doesn't seem like a good solution.
  • X days old still isn't the same thing as "explicitly considered ok". Maybe a description regex, but that's messy

@zomfg
Copy link
Author

zomfg commented Dec 2, 2020

Normal people don't know what a CI is, if it's enabled, where to look for it and if there are assets to download, and how to dig through jenkins or whatever (and you don't have a page for mac builds?). GH CI is slightly more practical in that respect, but the build artifacts are not "public", I think you can share direct links, but people can't go to the CI and download on their own, hence the pre-release route.
What's wrong with making these builds more accessible?

  • ok
  • sure, but then when is it? after an arbitrary number of days? without regex it can work like this:
    • do a real pre-release, wait X days, update status to release, or keep as pre-release til it's ready
    • or do a release, wait X days, leave as is if nothing happens, update it to pre-release and it'll be pulled from that API endpoint (could be combined with "must be X days old" as extra safety I guess)

speaking of AV, VirusTotal has a public API apparently, could be part of CI...

@psieg
Copy link
Owner

psieg commented Dec 3, 2020

One thing is that conceptually there shouldn't be two binaries with the same version and different code/behavior in the wild. Ideally the continuously built versions would have different version numbers (but because of the fork we're already using the last version number field, at least on Windows).
I don't see the value of automatic pre-releases at this point, they spam the release history (is the changelog vs the last full or pre-release?) and if we'd be considering the version "good for end-user/widespread use" they'd be full release versions. A "beta channel" doesn't make sense for a project as small as this IMO. The kind of user that needs a hotfixed version will be on that bug and get a link there, though a release should follow quickly.

Back to the API endpoint discussion, there's also a difference in the body texts between GH releases and the updater ones. The updater body will be shown in desktop notifications on clients, those have fewer and shorter lines than GH (e.g. I leave out the issue references and less important changes). Plus the fluff text that's on each release on the download page is not relevant for that notification.
Using the pre-relase status is a possibility, but the point is that such versions should trigger release notifications to watchers etc, which pre-releases don't I guess. So flagging releases later in case of issues with age check is the better approach to me.
Still, I don't see the benefit in coupling these two things and the tech-debt cost of the XML isn't that high.

Re AV: pushing things to VirusTotal doesn't fix the issue. I've had one version "turn bad" only after a few days (after I put it on the release page, not sure if related). But if you think it would help, feel free to give it a try.

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