Open molletts opened 2 years ago
rehashedsalt
commented
2 years ago +1, though usage of a construct like command -v ${CMD} would be nicer than which ${CMD}, since it's a shell builtin. It's functionally identical, but doesn't spawn a subprocess.
molletts
commented
2 years ago +1 to command -v ${CMD} - I didn't even think of that option.
kb-1000
commented
2 years ago The script uses zenity to display a progress bar. If zenity is unavailable, the pipe on the output of wget fails.
That's to be expected. If you ignore the dependencies of the packages you install (the DEB, RPM and AUR packages all declare a dependency on Zenity), expect running the software contained in them to fail. That's like reporting a bug to Qt that when QtCore is missing, QtGui fails to load.
Unfortunately, although it appears to be intended as a drop-in replacement (the command line args are identical), its behaviour is sufficiently different that simply dropping it in (e.g. by symlinking it) doesn't work
If it's not a drop-in replacement, don't use it as one.
That being said, supporting a Qt-based progress bar window doesn't sound like a bad idea to me, but qarma does not really look suitable (for example, being unreleased)
molletts
commented
2 years ago That's to be expected. If you ignore the dependencies of the packages you install (the DEB, RPM and AUR packages all declare a dependency on Zenity), expect running the software contained in them to fail.
It came to light as a result of a missing dependency. I've filed a bug against the package to rectify that. That said, I'd have still been a little surprised to see an app I know to be Qt-based pulling in something I know to be from Gnome-land.
If it's not a drop-in replacement, don't use it as one.
One could argue the same about Linux support - it's not a drop-in replacement for Windows, after all (nor does it even look like that may be the ultimate aim, as is the case with qarma/zenity).
I just had a look at my experimental LXQt-based system; zenity wants to pull in a bunch of stuff that is not needed by anything else. Why should I have to install GTK in order to run a Qt application in a native Qt desktop environment, especially when it is so easy to make qarma work as a drop-in replacement in environments that lack zenity? It's not as if I'm completely reworking the script to create a progress bar by piping a shell-generated video stream into VLC. (That might be fun to try, though... Maybe after a few [too many] beers.)
I'm not suggesting that you should drop the zenity dependency from the official packages in favour of a qarma one, merely that, with minimal work, it could be accommodated for the benefit of those using pure Qt environments.
If accommodating the use of an alternative piece of software is too alien to the Linux philosophy, why not just have the dirty fallback to appease the die-hard heretics?
rehashedsalt
commented
2 years ago That being said, supporting a Qt-based progress bar window doesn't sound like a bad idea to me, but qarma does not really look suitable (for example, being unreleased)
kdialog then? It's already used elsewhere in MultiMC as a fallback option when Zenity isn't around: https://github.com/MultiMC/Launcher/blob/develop/launcher/Launcher.in#L7.
Its implementation is a bit messier in that it requires callouts over dbus to update the progressbar and just discards STDIN. Here's a snippet from KDE's docs:
dbusRef=`kdialog --progressbar "Initializing" 4`
qdbus $dbusRef Set "" value 1
qdbus $dbusRef setLabelText "Thinking really hard"
sleep 2
qdbus $dbusRef Set "" value 2
sleep 2
qdbus $dbusRef setLabelText "Thinking some more"
qdbus $dbusRef Set "" value 3
sleep 2
qdbus $dbusRef Set "" value 4
sleep 2
qdbus $dbusRef closeWe could probably just pipe wget into a while read loop to make the updates without too much trouble.
Worst case, MultiMC could just roll its own. kdialog is more or less a shim around QProgressDialog* with some dbus plumbing to my understanding.
Operating System
Linux
Description of bug
When launching the wrapper script on a system that does not have zenity installed (for example, a KDE or LXQt-based system), it fails. If launched from the applications menu rather than from a terminal, the failure is silent.
Steps to reproduce
The launcher will fail with the following output:
Suspected cause
The script uses zenity to display a progress bar. If zenity is unavailable, the pipe on the output of wget fails.
As MultiMC is a Qt program and will therefore pull in Qt dependencies, it would make sense to use qarma in preference to zenity if it is available to avoid pulling in additional GTK dependencies unnecessarily in Qt-based environments. Unfortunately, although it appears to be intended as a drop-in replacement (the command line args are identical), its behaviour is sufficiently different that simply dropping it in (e.g. by symlinking it) doesn't work - it doesn't understand numbers suffixed with a '%' character (!) and it doesn't auto-close when its input pipe closes. Because the final line of wget's output differs slightly from previous lines, the sed regexp never outputs 100%, the --auto-close option is never triggered and, while zenity closes when its standard input is closed, qarma waits until it is cancelled by the user. (Both of these behavioural differences probably warrant a qarma bug report, really.)
I propose the following patch for the launcher to address both these issues. Firstly, it checks for the existence of qarma and zenity and chooses one. It also adds a minimal fallback if neither exists (as was the case on my Xfce system). It also amends the sed regexp to output the progress numbers without the percentage sign and match the final line of wget output so that "100" gets printed on completion.
(It's worth mentioning that my wget, GNU Wget 1.21.2, doesn't accept 'force' as a suboption of '--progress=dot', but this only triggers a warning and, assuming that some other version does require it in order to behave as expected, it may as well stay because it's harmless.)
This issue is unique