In-memory / patch.json fix for compatibility update

[b100dian]

This should fix #10

However this only updates the in-memory model at check-for-updates. So it will 'work' only as long as you have once checked for updates successfully during a daemon instance run.

The patch.json file is not re-written with the new contents.
What would the side effects of that be?
Actually I've updated just the compatibility part of the patch.json

[Olf0]

First of all: Thank you for taking care of this issue #10.
(Side note: When I manage to finalise addressing issue #17, we have addressed all my old issue reports but one: issue #11)

While I do see that this approach is nicely non-intrusive from a coding perspective, I believe that the resulting behaviour is too confusing for the average user. Obsoleted comment by take 2.

Sorry, I have not taken the time to understand the original code, but it seems that the compatibility information is separate from the JSON file of a Patch (from the Web Catalog), so it can be and is separately downloaded and evaluated?
But it is also part of the JSON file proper, isn't it?
IMHO this design is confusing, the JSON files are small enough (a few hundred bytes to 100 kbytes) to simply handle them as an atomic unit, and re-download a Patch from the Web Catalog as a whole, when something changed (e.g., the compatibility information).

But it looks like you ultimately implemented this nicely WRT usability while retaining the original, slightly complicated design. 😀

[b100dian]

I'm not very happy with how things are:)
The initial non-intrusive patch was made under the observation that the check-for-updates only went for this response
curl https://coderus.openrepos.net/pm2/api/project?name=patch-five-cameras-tucana | jq .
(if you have jq installed to format it)

As you can see, patch.json is not entirely present there.

The workflow was: if files section indicated a newer version, notifiy that updates are available. Then the user would go and install the updated as would normally the first time.

The 2nd take was like: ok, I don't have the patch.json immediately available because we're just checking for updates. But, if the current version has a newer/different compatibility field, let's store just that silently.

It is still up for testing.. you need a patch to edit the versions back and forth and somehow trigger the check-for-updates (I think closing the Settings ui and re-opening it then going to Patchmanager)

[b100dian]

Regarding downloading a fresh patch.json: maybe the whole patch could be re-downloaded if the (same) version changes. But then, maybe the author broke something, I wouldn't do that automatically.
Also, other fields in patch.json (or the files section in the above request) could be problematic to update automatically: what if the name changes or stuff like this? Or the section? What are the consequences?

This little hack should probably be more guarded around "only if the compatibility list added some new versions" too.

I'm open to ideas how to make this better (considering the code around is already a bit convoluted. Maybe I should bite the bullet and simplify some, but not today:)

[Olf0]

Regarding downloading a fresh patch.json: maybe the whole patch could be re-downloaded if the (same) version changes. But then, maybe the author broke something, I wouldn't do that automatically. Also, other fields in patch.json (or the files section in the above request) could be problematic to update automatically: what if the name changes or stuff like this? Or the section? What are the consequences?

Absolutely true (i.e., things are often not as simple as they seem)!
Thank you for clearly pointing this out.

[Olf0]

This little hack should probably be more guarded around "only if the compatibility list added some new versions" too.

Yes, if nothing was changed, except for some additional SailfishOS release(s) on the compatibility list, this approach is safe AFAICS.

[nephros]

I'm still not clear on what the ideal workflow or algorithm should look like. AFAICS we have these cases:

1. File version field bump. That's easy, higher version wins, -> notify user
2. Except it's not so easy, higher compatible version (according to web catalog JSON response) wins. Or do we simply always notify, even for incompatible versions?
3. Patch description last_updated field changed. not sure, probably ignore. Or trigger compatability check.
4. File uploaded field: do we have something locally we can compare this to? Yes, the last_updated field of patch.json. But, when installing or updating a patch tar, is this field set to web catalog last_updated (patch) or uploaded (file) field?
5. Compatible changed. When to check? Surely when file uploaded changed. Maybe when patch description last_updated. Other times? Always for installed patches just to make sure?

(Oh and maybe the whole checking thing could just be replaced with a nuclear option: "redownload all patches", and "redownload/update all compatible patches", user triggered, and let them keep the pieces.
That's basically how Storeman/pkcon/zypper works from a UI perspective.)

[Olf0]

@nephros, issue #10 is only about your case 5: Solely something in the "compatible" field for the installed version of a Patch was altered (usually new SailfishOS release(s) have been added), everything else is unchanged.
AFAIU (i.e., I have not checked that) only updating the "compatible" field for a specific version of a Web Catalog Patch does not update the "uploaded" field for that version or the "last_updated" field of the Patch.
IMO for all other cases the extant update detection mechanism is / does O.K.

When to check?

Honestly, I have not explicitly thought about this before, but simply assumed, "when the Web Catalog is checked for updates".
AFAICS (not by looking at the code, but by using Patchmanager), the "Web Catalog is checked for updates" is triggered by (or happens when) entering the Patchmanager GUI in the SailfishOS Settings app. But even if it happens earlier (and can only be seen then), I think the right place in the code is where this check is already implemeneted.

[b100dian]

Sorry for the delay answering here. This PR only detects "same-version, but become-compatible" changes (or that's the goal). It runs at web catalog update check indeed. And it then only changes the compatibility of the stored patch json.
Now, I don't remember if last_updated or uploaded changed when doing tests for this, so I will need more time to check this.