GNOME Bugzilla – Bug 770121
Fix gd-tagged-entry CSS being missing
Last modified: 2016-11-22 10:18:28 UTC
Automatic resource registration appears to not work properly with the static libgd variant, which causes warnings. That can be worked around by linking against libgd dynamically.
Created attachment 333596 [details] [review] configure.ac: link against libgd dynamically
Review of attachment 333596 [details] [review]: +1 thanks!! Feel free to backport
Attachment 333596 [details] pushed as 43f2065 - configure.ac: link against libgd dynamically
Are you able to start nautilus with [1] installed on your system? I suspect not. You'll probably need to revert this unfortunately, see bug #760442 [1] https://libgd.github.io/
(In reply to Michael Catanzaro from comment #4) > Are you able to start nautilus with [1] installed on your system? I suspect > not. You'll probably need to revert this unfortunately, see bug #760442 > > [1] https://libgd.github.io/ Seems to work fine for me and I have had libgd installed for ages. Not even playing around with LD_LIBRARY_PATH has any effect.
fwiw if we have any more problems with libgd I would drop the dep and copy the small GtkEntry class we are using into nautilus tree until gtk+ adopts it.
(In reply to Carlos Soriano from comment #6) > fwiw if we have any more problems with libgd I would drop the dep and copy > the small GtkEntry class we are using into nautilus tree until gtk+ adopts > it. You're building and distributing gtk-hacks, notification, _view-common (private, but epiphany does it too), and tagged-entry. If you're only using one file you should only be building one thing.
(In reply to Michael Catanzaro from comment #7) > (In reply to Carlos Soriano from comment #6) > > fwiw if we have any more problems with libgd I would drop the dep and copy > > the small GtkEntry class we are using into nautilus tree until gtk+ adopts > > it. > > You're building and distributing gtk-hacks, notification, _view-common > (private, but epiphany does it too), and tagged-entry. If you're only using > one file you should only be building one thing. Right. However that's irrelevant to this bug or discussion, isn't it? (honest question)
Yes
(In reply to Ernestas Kulik from comment #0) > Automatic resource registration appears to not work properly with the > static libgd variant, which causes warnings. That can be worked around > by linking against libgd dynamically. I'd rather this had been fixed, instead of worked around, and every other user of the library fending for themselves. You should also know better Carlos.
> You should also know better Carlos. Excuse me? What do you mean by that?
(In reply to Carlos Soriano from comment #11) > > You should also know better Carlos. > > Excuse me? What do you mean by that? That you shouldn't take work-arounds in nautilus, instead pushing your contributors to fix the original problem.
(In reply to Bastien Nocera from comment #12) > (In reply to Carlos Soriano from comment #11) > > > You should also know better Carlos. > > > > Excuse me? What do you mean by that? > > That you shouldn't take work-arounds in nautilus, instead pushing your > contributors to fix the original problem. I didn't know it was a workaround, since gnome-photos and gnome-documents are using the same approach, so I rather though it was my problem doing it wrong. Sometimes it's just ignorance, which I think is fair to have. Or you think it isn't? In any case, I would like you to clarify to me what you mean that they should know better me, because I have the feeling you are commenting about my personality, which I don't personally like neither I find it appropriate for a bug report.
Created attachment 334370 [details] [review] Revert "configure.ac: link against libgd dynamically" This reverts commit 43f2065add00c029584ced89b4c4ea391f9b52b3. This will be fixed another way.
Created attachment 334371 [details] [review] query-editor: Fix unavailable styling for tagged entry When linking directly the libgd static library, gcc will "helpfully" strip out any unused functions, and it considers the tagged entry constructor to be unused. Force it to be used, and mark the variable as unused to avoid warnings.
Review of attachment 334371 [details] [review]: ::: src/nautilus-query-editor.c @@ +249,3 @@ GObjectClass *gobject_class; GtkWidgetClass *widget_class; + GResource *junk __attribute__ ((unused));; You've got an extra semicolon here. @@ +251,3 @@ + GResource *junk __attribute__ ((unused));; + + junk = gd_tagged_entry_get_resource (); Can't you do it in a library constructor instead, so we don't need this magic in each application?
(In reply to Michael Catanzaro from comment #4) > Are you able to start nautilus with [1] installed on your system? I suspect > not. You'll probably need to revert this unfortunately, see bug #760442 > > [1] https://libgd.github.io/ I can see this problem on FreeBSD. $ nautilus /home/lantw44/gnome/devinstall/bin/nautilus: Undefined symbol "gd_tagged_entry_new" $ ldd `which nautilus` | grep libgd.so libgd.so => /usr/local/lib/libgd.so (0x800973000)
(In reply to Michael Catanzaro from comment #16) > Review of attachment 334371 [details] [review] [review]: > > ::: src/nautilus-query-editor.c > @@ +249,3 @@ > GObjectClass *gobject_class; > GtkWidgetClass *widget_class; > + GResource *junk __attribute__ ((unused));; > > You've got an extra semicolon here. > > @@ +251,3 @@ > + GResource *junk __attribute__ ((unused));; > + > + junk = gd_tagged_entry_get_resource (); > > Can't you do it in a library constructor instead, so we don't need this > magic in each application? Indeed. I did this in the patch for bug 770555. We'll just need to revert the original patch in this bug, and update libgd to take advantage of the fix.
Review of attachment 334370 [details] [review]: Whatever we do, this need to be reverted. I reverted already in 3.20.
(In reply to Carlos Soriano from comment #13) > (In reply to Bastien Nocera from comment #12) > > (In reply to Carlos Soriano from comment #11) > > > > You should also know better Carlos. > > > > > > Excuse me? What do you mean by that? > > > > That you shouldn't take work-arounds in nautilus, instead pushing your > > contributors to fix the original problem. > > I didn't know it was a workaround, since gnome-photos and gnome-documents > are using the same approach IIRC, gnome-documents needs libgd as shared library due to the usage of JS and introspection.
Attachment 334370 [details] pushed as 02be448 - Revert "configure.ac: link against libgd dynamically" and bumped the libgd version in the submodule to the latest git.
Arrived here while updating the copy of libgd in gnome-photos ... (In reply to Carlos Soriano from comment #13) > (In reply to Bastien Nocera from comment #12) > > (In reply to Carlos Soriano from comment #11) > > > > You should also know better Carlos. > > > > > > Excuse me? What do you mean by that? > > > > That you shouldn't take work-arounds in nautilus, instead pushing your > > contributors to fix the original problem. > > I didn't know it was a workaround, since gnome-photos and gnome-documents > are using the same approach, For clarification, only gnome-documents links dynamically to libgd because of the reasons in comment 20 - JavaScript and introspection, but gnome-photos has always linked it statically. gnome-photos is different from nautilus in that it is set up to use the dark theme. Back when libgd, didn't ship its own CSS, libgd/gd-tagged-entry.c would hard code the style class to "documents-entry-tag" and the actual CSS would be shipped by the application. Now, libgd ships the CSS for the light variant, so gnome-documents and nautilus doesn't need to ship their own. However, gnome-photos continues to ship its own CSS because it needs the dark variant. That is why it didn't suffer from the same problem as nautilus. I hope this clears it up. :) The next step would be to move the dark variant inside libgd.