Update pushed through by... somebody? partially breaking application

I’m maintaining a Flatpak for PhotoQt for quite a while by now. Earlier this week a pull request was opened with some changes that would reduce the package size quite a bit. That was great, but before I was able to get to reviewing the pull request just over two days later, it was already merged and the app was updated on Flathub. The problem: The update removed about 1/3 of the supported image formats from the app - kinda a big deal for an image viewer.

Why was this pull request merged without waiting for any feedback? I verified my app on Flathub a while ago - but I was not able to “verify” this update, it happened behind my back. This is not a big deal for an image viewer, but tbh it doesn’t sit right that this is possible in general. I assume that the person who did that is part of the Flathub team (as I don’t think just anybody can merge randomly), but what is the point of the process of updating apps and verification if an update can be pushed through by others that alters the feature set of an application? Isn’t this what pull requests and reviews are supposed to prevent?

I’m really a little annoyed (to put it mildly) that this happened. In an already busy week I’m now forced to debug these changes to find what caused this regression, or to simply revert the changes altogether. Given the recent talk around security of Flathub I would hope that things were handled a little more cautiously.

Thank you for listening to my rant.

1 Like

I agree that it shouldn’t have been merged without your review considering the app is verified and is being maintained (previous commit was 2 weeks ago).

We usually do “non-maintainer” merges on unmaintained applications (or using EOL runtimes) when the maintainer is not reachable but generally after waiting for some months for a response.

it has been reverted now. There is 340MB of unecesary bloat (hello *.a never cleaned up), and a bunch of duplicate libraries (two different version of jxl are loaded, and also 2 manifest errors (one flatpak-builder tells you, one that we should add to the linter)

That makes sense, and I agree that is a good/necessary thing. I guess it just caught me a little off-guard that it happened here, and my response was a little grumpy and maybe a bit harsh (sorry).

Oh, the pull request was definitely much appreciated, and I will step through the changes probably later today to see what exactly caused that loss of functionality. I expect most of your changes to have been the right move.

If I run flatpak run --command=flatpak-builder-lint org.flatpak.Builder manifest org.photoqt.PhotoQt.yml on the original manifest file though, I don’t get any output back. I assume one of the two errors you mention is the versions entry for the ffmpeg plugin, what is the other one?

ffmpeg-full is becoming a runtime extension next runtime version 25.08 so you don’t need to specify it in the app manifest again. Multiple versions for it doesn’t make sense and anything that is shipping libraries because major versions correspond to wildly different ABI but for some extensions that ship extra game files, help, docs etc. it does eg. if you switch between beta stable and test.

1 Like

Turns out the only issue with the pull request was in the cleanup rules, removing all *.la files is too aggressive, likely since some image libraries like ImageMagick search for their available plugins at runtime only.

That’s incorrect. a .a is NEVER used at runtime. It’s purely at compile time. And the cleanup rule happen at the end of everything.

Oh, I was referring to .la files. That specific cleanup rule was the source of the problems:

“Some packages, such as [ImageMagick], use a libtool function, lt_dlopen, to load libraries as needed during execution and resolve their dependencies at run time. In this case, the .la files should remain.”
(Source: About Libtool Archive (.la) files)

1 Like