Open matthijskooijman opened 4 years ago
matthijskooijman
commented
4 years ago No, I hadn't changed the version of the extension. But I just tried now (set it to 3.36), and it makes no difference, the extension is still auto-blacklisted.
Extra weird, if the version is not known by e.g.o, it should also skip it entirely.
However, it seems that the "Version" sent to e.g.o is actually a single integer, not a 3.36-style version number. Is there such a number you could also change?
mwilck
commented
4 years ago AFAICS all these are strings. Btw I had a "mental typo", I'd set the version in metadata.json to "0.36.0". Looking at the code, this would throw an exception and set version = 1.
mwilck
commented
4 years ago Hooray that was it!! With the settings below, it works! :+1:
"uuid": "contact@projecthamster.org",
"version": "36"Apparently the version needs to be set to at least "3". With "2" I get the same failure.
mwilck
commented
4 years ago If I read the Django code correctly, the call to grab_proper_extension_version() (which may return None and cause the "blacklist" status) is skipped if we provide a extension version (integer !) which isn't in the data base yet. The extension version is apparently not parsed from the input metadata.json, but simply incremented each time the extension is saved.
As you already figured out, grab_proper_extension_version() would return None if there were no "visible" versions of the extension in question. "visible" means status=STATUS_ACTIVE, which would imply an approval (positive review) from an admin on extensions.gnome.org.
IOW, it seems that an extension with the contact@projecthamster.org UUID has been uploaded twice to E.G.O, but none of these version have ever been approved.
mwilck
commented
4 years ago I haven't found any documentation specifying that the "version" field needs to be an integer. But reading the code, it's of course obvious.
matthijskooijman
commented
4 years ago Hm, what does that mean for our branches and versioning plan in #319? The update check does seem to assume a linear history of these version numbers, so a higher build number means a higher version, so if we just number releases incrementally, releasing an update for an older shell version would get a higher version. If the compatible version list is truncated on the top, this could still work (since the update has a higher version, but will not be valid for shell versions that have better (even though older) versions in place.
Alternatively, we could do something like 0.36.1 => 000.036.001 => 36001 to ensure proper comparison of versions while keeping a single int?
benjaoming
commented
4 years ago Alternatively, we could do something like 0.36.1 => 000.036.001 => 36001 to ensure proper comparison of versions while keeping a single int?
I like that proposal, but is the version maintained elsewhere? Maybe best to maintain a version string that's derived from some kind of single source of truth? I don't mind maintaining version strings in code, I think that's great, but if this conversion rule has to be maintained as well, it would be a kind trip wire for a stressed release maintainer.
mwilck
commented
4 years ago If I read the code of the GNOME web service correctly, the version number we set doesn't matter for extensions.gnome.org at all. self.version on ExtensionVersion objects is never set from the meta data. The only place where it's set is in the save() method, which sets it to self.extension.versions.latest().version + 1 (code). The only thing that matters, if we want to be able to use a locally installed extension, is that version is parsed to an integer which is greater than the largest version on E.G.O. This also means that if people download the extension from E.G.O, it will be different from what we've set in our code.
IOW, we can set version to what we want, as long as it's correctly parsed by the python int() function, and is large enough (currently, 3, but this will change as soon as we start uploading again). So 36001 (or, better even, 3036001) might be a good choice for local install.
matthijskooijman
commented
4 years ago I'm not sure this is limited to local installs. If we release an update for an older shell version, we also want to upload that to e.g.o and we don't want that update to be used in favor of higher versions.
E.g., if we have:
Then someone running 0.34.0 on shell 3.34 might be "upgraded" to 0.30.2. This is the scenario I referred to before. We could fix that by using the large version number, but I guess you're saying that the version number we set is ignored anyway, not that I re-read your code.
In that case, the scenario above should be fixed by careful tracking of gnome-shell versions, e.g. release 0.30.2 for shell versions 3.30-3.32, so 3.34 will not use it (this should give each shell version its own proper incrementing range).
Then I wonder what this means for the nice "0.30.2" version numbers we planned. They are probably still good to use, since they help use track branches, but the linear version numbers generated by e.g.o will then not be related to them (nor will they be recorded in git, which is a bit weird). Alternatively, we upload to e.g.o. before tagging and then put the version number in the tag (e.g. shell-3.32-123, or 0.32.123 where 32 refers to the minimum gnome-shell version and 123 is the e.g.o-assigned version number). Not sure I like that much.
It seems that some other extensions do put these nice versions in their metadata, which probably always results in an "upgrade" command (because they'll have a version=1 on e.g.o that is valid).
mwilck
commented
4 years ago @matthijskooijman:
Then someone running 0.36.0 might be "upgraded" to 0.30.2. This is the scenario I referred to before.
Firstly, unlike the "blacklist" case (*), this wouldn't happen silently. Rather, the user would receive a "Extension Updates Available" notification, which would open the "Extensions" tool when clicked. I haven't tried, but I think in this case the tool would tell the user that she was about to "update" to an incompatible version of the extension.
Secondly, this can only happen the user has disable_version_validation set to TRUE. If disable_version_validation is FALSE (default, I've no idea why it was TRUE on my system), the service would return the newest extension version supporting the shell version that the user has, which would be 0.36.0 in your scenario.
IMO this is how versioning on E.G.O is designed to work, and it's actually quite powerful. It allows all shell-version branches of an extension to be updated, and still delivers the latest matching extensions to users.
(*) Wrt the "blacklist", it's really a bug IMO, and I think we should report it.
mwilck
commented
4 years ago Then I wonder what this means for the nice "0.30.2" version numbers we planned. They are probably still good to use, since they help use track branches,
The question is where to use them. We can't use them in metadata.json. We'd need to track an extra version field. Not sure if that's worth it.
but the linear version numbers generated by e.g.o will then not be related to them (nor will they be recorded in git, which is a bit weird).
Well, we could manually download the E.G.O. packages once they're public, extract the version number, and create a special tag for our own reference (e.g. ego:17).
It seems that some other extensions do put these nice versions in their metadata
As we've learned now, that's pretty obviously wrong. The others, like us, just don't know that, because it isn't documented anywhere.
matthijskooijman
commented
4 years ago Firstly, unlike the "blacklist" case (*), this wouldn't happen silently. Rather, the user would receive a "Extension Updates Available" notification, which would open the "Extensions" tool when clicked. I haven't tried, but I think in this case the tool would tell the user that she was about to "update" to an incompatible version of the extension.
If I read the code correctly, if the extension version is not compatible with the shell version, it won't be considered for a possible upgrade. I just noticed that I mislabeled a version in my previous post (wrote 0.36.0 where I meant 0.34.0, since that version was intended to still support 3.34) and I forgot to explicitly mention this problem would happen for someone running shell 3.34. So what I meant was that someone runing 0.36.0 on shell 3.34 (which is a valid combination) would be "upgraded" to 0.30.2 (because it is still compatible with 3.34 and version 12 > version 10). This happens unless we would mark 0.30.2 as not compatible with 3.34 (which I think is the solution here).
The question is where to use them. We can't use them in metadata.json. We'd need to track an extra version field. Not sure if that's worth it.
Well, putting them in tags or branches might work. OTOH, we might as well name branches after the "minimum" gnome shell version (for-shell-3.32) or something and use your ego:17 tag format (OTOH, then it is not quite clear, except when looking at metadata, which tags belong to which branches, but we could maybe also tag with the shell version (but then we would need minor version numbers again), or maybe just tag with branch and ego version (for-shell:3.32,ego:17)?
mwilck
commented
4 years ago If I read the code correctly, if the extension version is not compatible with the shell version, it won't be considered for a possible upgrade.
You're right. (Update:) I thought so first, but not quite. That code is difficult to read :nerd_face: . IIUC the algorithm does the following:
1) If the user's shell version is unknown to the web API, goto 4.
2) If no visible version of the extension supports the user's shell version, goto 4.
3) Return the highest version of the extension that supports the user's shell version.
4) if disable_version_validatation is false, return None.
5) If the set of shell versions supported by visible extension versions is empty, return None.
6) If the user's shell version is below all shell versions supported by the extension, select the lowest supported shell version, and return the latest extension version supporting it.
7) Otherwise, there's at least one supported shell version lower than the user's. Select the latest of these shell versions, and return the highest extension version supporting it.
3.) is the default case, and 7.) would be the usual case for users upgrading to a new GNOME version. The case I ran into is the pathological 5.) (Update): In case 6. and 7., the user may get update notifications for extension versions that don't support his shell version.
Wrt branch names, it just occured to me that it might be reasonable to name branches after the latest shell version they support, rather than the oldest. After all, the current shell version will normally be supported by develop, and we only branch if develop can't support older versions any more because of compatibility issues. At the time a branch is created, we'll therefore know what maximum shell version it supports (which would be the version below the lowest one supported by develop at that point of the commit history). The ego:$N tags would correspond to code snapshots, and thus would be tags of commits on some branch. We could track (and tag) extension versions like 0.32.3 besides that, but their usefulness would be limited.
Does this make sense?
mwilck
commented
4 years ago @matthijskooijman,
It seems that some other extensions do put these nice versions in their metadata
Have you found examples? That might be useful for a GNOME bug report.
Meanwhile, this comment makes me think that we probably shouldn't have set the version field at all.
mwilck
commented
4 years ago mwilck
commented
4 years ago @matthijskooijman and fellow @projecthamster/gnome-shell-extension maintainers, if you find time, please look at the GNOME issue. The GNOME devs say it "works as designed". Not that I expected much else, but some support in the discussion might still be helpful.
Sp far I've learned two things:
version field is reserved for e.g.o. versioning only, and shouldn't be used by local extensions at all. I guess we should remove it completely.The special case we've run in (some version uploaded, but none reviewed/approved) seems to be real pathological.
matthijskooijman
commented
4 years ago I posted to the gnome issue before, but seeing they already had an issue about this predating ours, I expect this will be fixed on their end some way or another. The take away for us is to leave the version field empty, so maybe we can put some meaningful version number in the extension description or something like that.
This relates to #304 but concerns a specific issue: When gnome-shell asks for update info on the extension, e.g.o seems to return "blacklist", which causes gnome-shell to uninstall the extension.
It is not entirely clear why this happens, or what this mechanism is intended for, but investigation of the e.g.o source code suggests that there is no explicit blacklist that triggers this, but instead some weird internal state (though reproducing it using a manual request has not succeeded yet, and with the values we think gnome-shell sends to this service, there should not really be a way to trigger this).
This issue also seems to happen only on gnome-shell 3.36, so maybe something changed in the request it sends to trigger this?
For previous discussion, detailed analysis and links to relevant source code, see https://github.com/projecthamster/hamster-shell-extension/issues/321#issuecomment-598126449 and onwards.