Bug 15199

Summary: Broken pkg-config file
Product: Telepathy Reporter: Brian Pepple <bpepple>
Component: tp-glibAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED NOTOURBUG QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: bugzilla, mclasen
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: pkconfig patcg

Description Brian Pepple 2008-03-25 07:25:52 UTC
Created attachment 15453 [details] [review]
pkconfig patcg

Requires.private are for requirements that aren't exposed in the headers. Glib
and dbus-glib are clearly used in the telepathy-glib-devel headers, so they
shouldn't be listed in the private section.
Comment 1 Simon McVittie 2008-03-25 09:08:06 UTC
You're requesting that we revert a change I was asked to make a while ago. Here's an edited transcript of the IRC discussions in which I was persuaded to start using Requires.private in the first place - "Mithrandir" here is Tollef Fog Heen, the pkg-config maintainer, who confirms that as long as we depend on pkg-config >= 0.21, the change I made is legitimate and indeed to be recommended.

If there is a concrete bug caused by this change (e.g. a package that uses telepathy-glib fails to build or run in a particular environment) then please let us know. Otherwise, I'm inclined to mark this NOTABUG.

11:38 < sjoerd> see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=456335 for 
some interesting discussion

11:23 < sjoerd> It's a nasty issue
11:23 < sjoerd> Up untill now i haven't seen good guidelines to properly cleanup
 .pc files
11:24 < sjoerd> Their don't see any good rules to when something does and when s
omething doesn't go itno Libs vs Libs.private
11:24 < Mithrandir> in general, dependencies should go to Libs.private.
11:24 < Mithrandir> or Requires.private.
11:25 < sjoerd> So what 's left for Requires: ?
11:25 < smcv> we have GLib things in our API
11:26 < smcv> so we need at least GLib and GObject in Requires
11:26 < Mithrandir> smcv: that's fine, since Requires still means cflags are pas
sed along.
11:26 < Zdra> that makes me think that .pc could be updated wit tp-glib
11:26 < Mithrandir> sjoerd: non-C languages, at least.
11:26 < smcv> Mithrandir: but Requires.private wouldn't be sufficient, right?
11:28 < Mithrandir> smcv: sorry, I misspoke.  Requires.private still means -I ar
e passed along.
11:28 < Mithrandir> (iirc, etc)
11:30 < smcv> Mithrandir: pkg-config doesn't mention Requires.private - is it bl
eeding-edge?
11:30 < smcv> pkg-config(1) I mean
11:30 < Mithrandir> smcv: the docs are lagging a bit, and well, bleeding-edge; i
t's a year old or so.
11:30 < Mithrandir> iirc
11:33 < smcv> Mithrandir: judging by fd.o #3097, Requires.private does not do wh
at we want
11:34 < smcv> Mithrandir: code using telepathy-glib will not compile without -I/
usr/include/glib-2.0, which Requires.private does not give us
11:38 < bigon> patch about Require.private http://www.mail-archive.com/util-linu
x-ng@vger.kernel.org/msg00962.html
11:46 < smcv> ok, so it appears http://bugs.debian.org/cgi-bin/bugreport.cgi?bug
=340904 gives us the semantics Mithrandir wants
11:46 < smcv> until they change it again :-/

12:32 < Mithrandir> smcv: 3097 is ages old, and the semantics have changed once 
after that, introducing -I even for items in Requires.private.
12:36 < smcv> Mithrandir: yeah, having read some mailing list threads, it seems 
we can just require pkg-config 0.21
12:37 < smcv> Mithrandir: so that's what I merged
12:38 < Mithrandir> smcv: if the semantics for some reason doesn't fit how you u
se it, please prod me so I can either get pkg-config adjusted or show you how to
 use it correctly.
12:39 < Mithrandir> since people who use pkg-config tend not to tell me what the
y want changed that often..
12:39 < smcv> Mithrandir: I'm using Requires.private for dbus, dbus-glib, glib, 
gobject now
12:39 < Mithrandir> you might want Requires: pkg-config >= 0.21 as well, as you 
noted.
12:39 < smcv> Mithrandir: libtelepathy still has a Requires on telepathy-glib be
cause various symbols that used to be part of libtelepathy have migrated (!)
12:39 < smcv> Mithrandir: and yes, I'm checking for 0.21 in the configure.ac
Comment 2 Brian Pepple 2008-03-25 09:25:34 UTC
(In reply to comment #1)
> You're requesting that we revert a change I was asked to make a while ago.
> Here's an edited transcript of the IRC discussions in which I was persuaded to
> start using Requires.private in the first place - "Mithrandir" here is Tollef
> Fog Heen, the pkg-config maintainer, who confirms that as long as we depend on
> pkg-config >= 0.21, the change I made is legitimate and indeed to be
> recommended.
> 
> If there is a concrete bug caused by this change (e.g. a package that uses
> telepathy-glib fails to build or run in a particular environment) then please
> let us know. Otherwise, I'm inclined to mark this NOTABUG.

I believe Bastien was having a problem build the gnome-phone-manager (1) with t-g, and if I remember correctly we are using pkconfig-0.23 in Rawhide right now.

(1) https://bugzilla.redhat.com/show_bug.cgi?id=436773#c0
Comment 3 Bastien Nocera 2008-03-25 09:34:11 UTC
This would probably fail currenly (not actually tested):

$ cat test.c
#include <telepathy-glib/run.h>

int main(int argc, char **argv)
{
  return 0;
}

$ gcc -o test test.c `pkg-config --libs --cflags telepathy-glib`

dbus-glib and glib are exposed in the telepathy-glib headers, so they shouldn't be in the private requires. Say you added a dependency on libssl inside telepathy-glib, but didn't expose it in the headers. Then you would add it to Requires.private.
Comment 4 Simon McVittie 2008-03-25 09:55:52 UTC
I tried your example, but for all the headers (individually) instead of just run.h:

smcv@carbon% cat > ~/tmp/true.c
int main(int argc, char **argv) { return 0; } 
smcv@carbon% ( cd /usr/include/telepathy-1.0 && echo telepathy-glib/*.h ) | while read -d' ' h; do gcc -o ~/tmp/true ~/tmp/true.c `pkg-config --libs --cflags telepathy-glib` -include $h && ~/tmp/true || echo FAIL: $h; done 
In file included from <command-line>:0:
/usr/include/telepathy-1.0/telepathy-glib/defs.h:60: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘G_END_DECLS’
FAIL: telepathy-glib/defs.h

(There is indeed a minor bug here - telepathy-glib/defs.h should either #include an appropriate header from GLib to define G_BEGIN_DECLS and G_END_DECLS, or become entirely self-contained - but our pkg-config use appears to be sufficient.)

I deliberately excluded the telepathy-glib/_gen/*.h hierarchy, since these headers are not public API, and indeed are unlikely to be self-contained.

Further, examining the result from an arbitrary one of the successful runs, I get:

smcv@carbon% ldd ~/tmp/true
        linux-gate.so.1 =>  (0xffffe000)
        libtelepathy-glib.so.0 => /usr/lib/libtelepathy-glib.so.0 (0xb7f31000)
        libc.so.6 => /lib/i686/cmov/libc.so.6 (0xb7dd6000)
        libdbus-glib-1.so.2 => /usr/lib/libdbus-glib-1.so.2 (0xb7dba000)
	libdbus-1.so.3 => /usr/lib/libdbus-1.so.3 (0xb7d83000)
	libgobject-2.0.so.0 => /usr/lib/libgobject-2.0.so.0 (0xb7d47000)
	libglib-2.0.so.0 => /usr/lib/libglib-2.0.so.0 (0xb7c99000)
	/lib/ld-linux.so.2 (0xb7fca000)
	libnsl.so.1 => /lib/i686/cmov/libnsl.so.1 (0xb7c80000)
	libselinux.so.1 => /lib/libselinux.so.1 (0xb7c66000)
	libpcre.so.3 => /usr/lib/libpcre.so.3 (0xb7c3f000)
	libdl.so.2 => /lib/i686/cmov/libdl.so.2 (0xb7c3b000)
smcv@carbon% objdump -x ~/tmp/true | grep NEEDED
  NEEDED      libtelepathy-glib.so.0
  NEEDED      libc.so.6

... which is the result we're after. Reverting the change to the .pc file as you suggest would result in ~/tmp/true being linked against dbus-glib, libdbus, etc. whether it actually uses them or not, which is precisely what distros want to avoid.
Comment 5 Bastien Nocera 2008-03-25 10:30:36 UTC
Right, let's fix that one warning (which would require adding glib includes, thus moving it outside).

My problem was for this particular error:
/usr/include/telepathy-1.0/telepathy-glib/channel-iface.h:25:25: error:
glib-object.h: No such file or directory

So I guess that glib should definitely be non-private.
Comment 6 Simon McVittie 2008-03-25 10:42:12 UTC
I agree that that error is a bug, but it's nothing to do with pkg-config.

Like I said, all the other headers work for me.

smcv@carbon% pkg-config --version
0.22

smcv@carbon% pkg-config --cflags telepathy-glib
-I/usr/include/telepathy-1.0 -I/usr/include/dbus-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/lib/dbus-1.0/include  

smcv@carbon% cat /usr/lib/pkgconfig/telepathy-glib.pc    
prefix=/usr
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: Telepathy-GLib
Description: GLib utility library for the Telepathy framework
Version: 0.7.5
Requires.private: dbus-glib-1 >= 0.73, glib-2.0 >= 2.10, gobject-2.0 >= 2.10
Libs: -L${libdir} -ltelepathy-glib
Cflags: -I${includedir}/telepathy-1.0

What versions of pkg-config and telepathy-glib are you using, what's in your .pc file, and what --cflags does pkg-config give you?
Comment 7 Simon McVittie 2008-03-25 10:50:17 UTC
For the record, the above success report was with Debian's pkg-config 0.22-1; a copy of pkg-config 0.23 built from the upstream tarball and run with PKG_CONFIG_PATH=/usr/lib/pkgconfig has the same behaviour.

pkg-config's NEWS says

pkg-config 0.22
===
 - Make Requires.private a whole lot more useful by traversing the
   whole tree, not just the top-level, for Cflags.

so we may need to require 0.22 rather than just 0.21.
Comment 8 Bastien Nocera 2008-03-25 11:07:38 UTC
On rawhide:

$ rpm -q pkgconfig
pkgconfig-0.23-2.fc9.i386
$ pkg-config --libs --cflags telepathy-glib
-I/usr/include/telepathy-1.0  -ltelepathy-glib  
$ grep private /usr/lib/pkgconfig/telepathy-glib.pc
Requires.private: dbus-glib-1 >= 0.73, glib-2.0 >= 2.10, gobject-2.0 >= 2.10

Debian is using a stock 0.23. Matthias, what's the problem here?
Comment 9 Simon McVittie 2008-03-25 11:17:43 UTC
http://err.no/personal/blog/tech/2008-03-25-18-07_pkg-config,_sonames_and_Requires.private (a blog posting from Tollef) explains how he thinks pkg-config ought to work.

The Fedora maintainer for pkg-config appears to have included an early unreviewed patch from bug #4738, which breaks the intended semantics of Requires.private on Fedora. Updating to a later form of that patch might solve this, and so would reverting it.

In any case, NOTOURBUG.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.