Resolve crash running toga-demo on Fedora #3564
Conversation
|
@rmartin16 Tagging you for a review here because you were able to reproduce the problem. |
| default. The name of the system package that provides the ICO loader is distribution | ||
| dependent: | ||
|
|
||
| - Fedora: ``gdk-pixbuf2-modules-extra`` |
There was a problem hiding this comment.
Hmm... is there anywhere else that lets you specify system deps when packaging in Briefcase? If so, might be worth documenting that this needs to be manually added when packaging your app if you're using ICO icons.
Or just make it default.
Sorry if this is already existent.
There was a problem hiding this comment.
In Briefcase - yes. That's what system-runtime-requires is for.
Given we know this package is a dependency, it wouldn't be the worst idea to add this to the Toga bootstrap, similar to how webkit2gtk3 is listed, but commented out. That way, it's visible that the requirement might be needed, but isn't a forced requirement unless you know you need it.
There's a couple of similar requirements for status icon apps, and geolocation IIRC.
There was a problem hiding this comment.
This package isn't even 1 megabyte; I think it'd be better to just add by default. However, maybe worth a note that versions of Fedora before 41 do not support the package (although, <41 is EOL).
There was a problem hiding this comment.
This package isn't even 1 megabyte; I think it'd be better to just add by default.
True - but by that logic - why has Fedora split it out as a standalone package at all?
Maybe it's worth adding it into the Briefcase default, but we should still annotate why it's being included (implicitly noting that it could be removed to save the precious megabyte if you wanted to)
However, maybe worth a note that versions of Fedora before 41 do not support the package (although, <41 is EOL).
Fedora 41 is the oldest release that Fedora supports, so I'm not sure that's too much of a concern.
There was a problem hiding this comment.
True - but by that logic - why has Fedora split it out as a standalone package at all?
I agree; this feels more about ideology than pragmatism. From conversing with chatgpt o3, this seems to be part of Fedora's desire to be highly modular and efficient...
Fedora 41 is the oldest release that Fedora supports, so I'm not sure that's too much of a concern.
True; i was thinking about users trying to build for earlier versions of Fedora. It seems that Fedora is quickly deprecating older versions; so, I could imagine users building for older versions and getting surprised when the install of the distributable fails.
There was a problem hiding this comment.
I'm still getting caught on brutus.ico.
(venv) user@fedora:~/tmp/toga/demo$ python -m toga_demo
Traceback (most recent call last):
File "/home/user/tmp/toga/gtk/src/toga_gtk/icons.py", line 36, in __init__
native = GdkPixbuf.Pixbuf.new_from_file(str(path)).scale_simple(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
gi.repository.GLib.GError: gdk-pixbuf-error-quark: Couldn’t recognize the image file format for file “/home/user/tmp/toga/demo/toga_demo/resources/brutus.ico” (3)
Ok - I've finally got a working Fedora 42 box, and I can reproduce this. It looks like the icon lookup order isn't what I thought it was... so it's trying "no suffix .ico" before it tries "png with suffix"... Back to the drawing board, I guess... |
Fedora doesn't install the GDK image loader for ICO format by default. This causes the toga demo to crash, as it currently only provides ICO and ICNS format icons.
This can be fixed by users installing the
gdk-pixbuf2-modules-extrapackage on Fedora; or we can add a PNG icon (as PNG icons will be loaded before ICO is attempted. This PR adds the PNG to resolve the immediate problem, and adds a documentation advisory for the general case.Fixes #3422.
PR Checklist: