-
Notifications
You must be signed in to change notification settings - Fork 236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stop using deprecated GdkPixbuf.from_xpm_data
#1035
Conversation
This function can fail at runtime with certain versions of gdk-pixbuf. Replace it with an XPM parsing function created specifically for the icon data in the Pixmaps module.
@@ -1,7 +1,7 @@ | |||
(library | |||
(name unison_lib) | |||
(wrapped false) | |||
(modules :standard \ linktext linkgtk3 uigtk3 uimacbridge test) | |||
(modules :standard \ linktext linkgtk3 uigtk3 pixmaps uimacbridge test) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commit message says "fix", but I think this is "add module that the previous commit started using". Always watching out for ways future self might misread....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered it a fix as the list of modules was incorrect even before the pixbuf change. It's just that it didn't produce an actual error before the pixbuf change, but it wasn't correct. The pixbuf change made the Pixmaps module depend on lablgtk.
I would really like to see someone review and test. We just had a release, and I am going to try not to merge anything for a bit until the release and the gdk-pixbuf-is-unstable kerfluffle settle out a bit. |
On openSUSE Tumbleweed I get the following error, i.e. the UI cannot be shown any more: From https://build.opensuse.org/project/show/Archiving:unison |
Are you testing the branch in this PR, or an artifact built by CI from this branch? It is known that a release build of unison with a buggy gdk-pixbuf (as contained in openSUSE) does not work. |
@ithod I'm pretty certain that https://build.opensuse.org/project/show/Archiving:unison does not contain the patch from this PR. This makes your comment a duplicate of #1033. |
Sorry for my unclear message, indeed the error occurs if the patch is not applied. I wanted to make clear that this patch solves the error given above. When I apply the patch (the 1st, not sure what |
Thank you @ithod.
The 1st is the real patch, yes. The other one is only needed when building with |
We've had one test report, and enough time that anybody that wanted to test has had a chance. |
Re #1033 #1034
Since the XPM format is so simple, the function to parse (subset of) XPM used for the icons is trivial. This approach does not require external resources (like image files or loader plugins) nor embedding binary-format icons in the source.
Working but not heavily tested.