probonopd / linuxdeployqt

Makes Linux applications self-contained by copying in the libraries and plugins that the application uses, and optionally generates an AppImage. Can be used for Qt and other applications
Other
2.22k stars 414 forks source link

Inconsistent algorithm for main icon detection #496

Closed kefir500 closed 1 year ago

kefir500 commented 3 years ago

While migrating from Ubuntu 16.04 to 18.04 in my CI/CD environment, I suddenly discovered that my AppImage main icon is now a 48x48 image instead of a larger one. I started digging and found a couple of inconsistencies.

Correct me if I'm wrong, but as I see it, the initial idea of the main icon detection is to prioritize some images over the others (256x256 over 48x48, SVG over XPM, etc.). However, the current algorithm is using the foreach/continue construction which reassigns the iconToBeUsed variable until the last available (not the best) candidate is met.

Furthermore, it seems that QDirIterator is using readdir() under the hood, which doesn't guarantee a specific order, so it fully depends on a filesystem implementation. I don't think linuxdeployqt should rely on a random order of this iteration.

If you confirm my assumptions on the icon priorities, I'm ready to submit a pull request fixing this issue.

probonopd commented 3 years ago

You are right, and I appreciate your change. Thanks!