Generates invalid appdata when change is a downgrade

[wjt]

On https://github.com/flathub/us.zoom.Zoom/pull/35 the checker correctly submitted an update to a newer version of Zoom. The rotating URL was then rolled back to the older version, and the checker dutifully opened https://github.com/flathub/us.zoom.Zoom/pull/36. However, the latter PR prepended the new version to the appdata <releases>:

   <releases>
+    <release version="3.0.287250.0828" date="2019-08-29"/>
     <release version="3.0.291715.0908" date="2019-09-09"/>
     <release version="3.0.287250.0828" date="2019-08-29"/>

This is invalid:

Processing application us.zoom.Zoom
us.zoom.Zoom.desktop:              AppData problem: tag-invalid : <release> version was duplicated
us.zoom.Zoom.desktop:              AppData problem: tag-invalid : <release> versions are not in order [3.0.287250.0828 before 3.0.291715.0908]

Two changes suggest themselves:

• Validate the generated appdata with appstream-util before sending the PR
• Avoid generating this invalid change in the first place

The second could be done in a couple of ways:

• The cheat's way to do it is to only include one <release> in <releases> and just update it every time the version changes. However, the Zoom appdata has been dutifully preserving the full release history since before this checker entered the scene.
• The better approach would be to drop all <release> tags that do not compare older than the one we are about to prepend.

[wjt]

On https://github.com/flathub/com.adobe.Flash-Player-Projector/pull/28 we hit an issue where the tarball changed, but the web page from which the version number is scraped had not been updated. As a result, the generated PR included this bogus patch:

   <releases>
+    <release version="32.0.0.270" date="2019-11-13"/>
     <release date="2019-09-24" version="32.0.0.270"/>

Here's the checker output: https://gist.github.com/wjt/a3d952671d79bb2e390c067c9dcdd872

Quoth @hadess:

Should we have a check in the HTML checker that it throws an error in case the URL's content changed but not the version number? Is it possible that the HTML had the old version number?

I'm not sure that throwing an error is the right thing to do here – at present that would mean that I get an email; if we move this to Flathub infra then it probably means that Bart would get paged. Another option might be to open the PR anyway, but include a warning that the version number has gone down (in the Zoom case where this ticket started) or stayed the same (in the Flash projector case), which would allow the package maintainer to take action as needed. (Action is needed because the URL's old checksum is invalid.)

[hadess]

On flathub/com.adobe.Flash-Player-Projector#28 we hit an issue where the tarball changed, but the web page from which the version number is scraped had not been updated. As a result, the generated PR included this bogus patch:

I'm absolutely not certain that this is the case though. I don't have sufficient data to ascertain that the HTML would have been outdated. I don't have the environment to run the full tool, and I've only run the checker itself, as part of the tests.

[wjt]

What data would you need to ascertain that – the full HTML contents? That's not currently included in the log, though I suppose it could be added. I can at least add some logging about what matched in the htmlchecker?

[hadess]

What data would you need to ascertain that – the full HTML contents? That's not currently included in the log, though I suppose it could be added. I can at least add some logging about what matched in the htmlchecker?

The output of the matches is already a good start. Whether we need any more will depend on whether the output of that is interesting ;)

[hadess]

This is the log from a recent check:

==> checking com.adobe.Flash-Player-Projector
+ 2019-12-10 05:04:18,415    INFO src.checker: Referenced file not found: shared-modules/gtk2/gtk2.json
+ 2019-12-10 05:04:18,416   DEBUG src.checker: Checking module flashplayer (path: /github/workspace/flathub/com.adobe.Flash-Player-Projector/com.adobe.Flash-Player-Projector.json)
+ 2019-12-10 05:04:18,416   DEBUG src.checker: Checking individual sources in /github/workspace/flathub/com.adobe.Flash-Player-Projector/com.adobe.Flash-Player-Projector.json
+ 2019-12-10 05:04:18,416   DEBUG src.checker: [1/2] checking flash_player_sa_linux.x86_64.tar.gz
+ 2019-12-10 05:04:18,416   DEBUG src.checkers.debianrepochecker: flash_player_sa_linux.x86_64.tar.gz is not a debian-repo type ext data
+ 2019-12-10 05:04:18,416   DEBUG src.checkers.flashchecker: flash_player_sa_linux.x86_64.tar.gz is not a flash type ext data
+ 2019-12-10 05:04:18,416   DEBUG src.checkers.htmlchecker: Getting extra data info from https://www.adobe.com/support/flashplayer/debug_downloads.html; may take a while
+ 2019-12-10 05:04:18,832   DEBUG src.checkers.htmlchecker: version-pattern re.compile('The latest versions are <span>([\\d\\.-]*)<\\/span>') matched: 32.0.0.293
+ 2019-12-10 05:04:18,833   DEBUG src.checkers.htmlchecker: url-pattern re.compile('(https:\\/\\/fpdownload\\.macromedia\\.com\\/pub\\/flashplayer\\/updaters\\/\\d+\\/flash_player_sa_linux\\.x86_64\\.tar\\.gz)') matched: https://fpdownload.macromedia.com/pub/flashplayer/updaters/32/flash_player_sa_linux.x86_64.tar.gz
+ 2019-12-10 05:04:21,754   DEBUG src.checker: [2/2] checking com.adobe.Flash-Player-Projector.svg
+ 2019-12-10 05:04:21,754   DEBUG src.checkers.debianrepochecker: com.adobe.Flash-Player-Projector.svg is not a debian-repo type ext data
+ 2019-12-10 05:04:21,754   DEBUG src.checkers.flashchecker: com.adobe.Flash-Player-Projector.svg is not a flash type ext data
+ 2019-12-10 05:04:21,754   DEBUG src.checkers.htmlchecker: com.adobe.Flash-Player-Projector.svg is not a html type ext data
+ 2019-12-10 05:04:21,754   DEBUG src.checkers.urlchecker: Getting extra data info from https://helpx.adobe.com/content/dam/help/mnemonics/flashplayer_app_RGB.svg; may take a while
+ 2019-12-10 05:04:22,009   DEBUG src.checkers.urlchecker: URL https://helpx.adobe.com/content/dam/help/mnemonics/flashplayer_app_RGB.svg still valid
+ 2019-12-10 05:04:22,009    INFO src.checker: Updating /github/workspace/flathub/com.adobe.Flash-Player-Projector/com.adobe.Flash-Player-Projector.json
+ 2019-12-10 05:04:22,013    INFO src.main: Committing updates
+ 2019-12-10 05:04:22,014   DEBUG src.main: $ git checkout HEAD@{0}

I think it shows that there's a new tarball, but the version number didn't change in the HTML page. And indeed, the web page hasn't changed yet.

We could always get that info from the tarball itself, but that'd be another pile of code on top:

$ strings flashplayer | grep "Adobe Flash Player [0-9][0-9],[0-9]" | uniq
Adobe Flash Player 32,0,0,303

[hadess]

We could always get that info from the tarball itself, but that'd be another pile of code on top:

$ strings flashplayer | grep "Adobe Flash Player [0-9][0-9],[0-9]" | uniq
Adobe Flash Player 32,0,0,303

And this version is the same that's available in the web service used to check on the Flash plugin itself. Can you think of a good way to combine the two checkers?

[hadess]

Happened again: https://github.com/flathub/com.adobe.Flash-Player-Projector/pull/32

[wjt]

I can't think of a good way short of hardcoding a special case for the projector into the web service for the flash plugin – but if we expect that the web service is updated more promptly than the page that's currently being scraped, that doesn't seem such a bad option. The presence of a Flash-specific checker is already a special case, after all.

[hadess]

I think a better/easier option might be for the code calling into the html checker to also pass the old version information, so that the html checker can fail in a "please retry in a little while" way if the version number hasn't changed yet.

The problem for me to work on this is that I don't have an easy way to test any of the changes I make outside the checkers themselves.