After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770121 - Fix gd-tagged-entry CSS being missing
Fix gd-tagged-entry CSS being missing
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on: 770555
Blocks:
 
 
Reported: 2016-08-18 22:24 UTC by Ernestas Kulik
Modified: 2016-11-22 10:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
configure.ac: link against libgd dynamically (805 bytes, patch)
2016-08-18 22:24 UTC, Ernestas Kulik
committed Details | Review
Revert "configure.ac: link against libgd dynamically" (724 bytes, patch)
2016-08-29 13:57 UTC, Bastien Nocera
committed Details | Review
query-editor: Fix unavailable styling for tagged entry (1.74 KB, patch)
2016-08-29 13:57 UTC, Bastien Nocera
none Details | Review

Description Ernestas Kulik 2016-08-18 22:24:20 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.
Comment 1 Ernestas Kulik 2016-08-18 22:24:24 UTC
Created attachment 333596 [details] [review]
configure.ac: link against libgd dynamically
Comment 2 Carlos Soriano 2016-08-18 22:39:42 UTC
Review of attachment 333596 [details] [review]:

+1 thanks!!
Feel free to backport
Comment 3 Ernestas Kulik 2016-08-19 07:04:52 UTC
Attachment 333596 [details] pushed as 43f2065 - configure.ac: link against libgd dynamically
Comment 4 Michael Catanzaro 2016-08-27 22:47:42 UTC
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/
Comment 5 Ernestas Kulik 2016-08-28 07:01:24 UTC
(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.
Comment 6 Carlos Soriano 2016-08-28 07:44:44 UTC
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.
Comment 7 Michael Catanzaro 2016-08-28 12:16:50 UTC
(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.
Comment 8 Carlos Soriano 2016-08-28 13:17:13 UTC
(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)
Comment 9 Michael Catanzaro 2016-08-28 16:03:08 UTC
Yes
Comment 10 Bastien Nocera 2016-08-29 09:28:19 UTC
(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.
Comment 11 Carlos Soriano 2016-08-29 09:36:09 UTC
> You should also know better Carlos.

Excuse me? What do you mean by that?
Comment 12 Bastien Nocera 2016-08-29 09:40:47 UTC
(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.
Comment 13 Carlos Soriano 2016-08-29 09:52:04 UTC
(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.
Comment 14 Bastien Nocera 2016-08-29 13:57:02 UTC
Created attachment 334370 [details] [review]
Revert "configure.ac: link against libgd dynamically"

This reverts commit 43f2065add00c029584ced89b4c4ea391f9b52b3. This will
be fixed another way.
Comment 15 Bastien Nocera 2016-08-29 13:57:08 UTC
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.
Comment 16 Michael Catanzaro 2016-08-29 16:21:23 UTC
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?
Comment 17 Ting-Wei Lan 2016-08-30 08:24:45 UTC
(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)
Comment 18 Bastien Nocera 2016-08-30 09:23:00 UTC
(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.
Comment 19 Carlos Soriano 2016-09-02 08:19:49 UTC
Review of attachment 334370 [details] [review]:

Whatever we do, this need to be reverted. I reverted already in 3.20.
Comment 20 Michael Biebl 2016-09-04 23:41:51 UTC
(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.
Comment 21 Bastien Nocera 2016-09-05 11:32:48 UTC
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.
Comment 22 Debarshi Ray 2016-11-22 10:18:28 UTC
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.