Created attachment 55151 [details] [review]
use newer pkg-config private fields

Commited. Thanks.

Hi,
since 0.11.16 the following fails:
echo "#include <vte/vte.h>" > test.c && gcc -o /dev/null test.c `pkg-config
--cflags --libs vte`

with 0.11.15 it worked fine, it's most probably related to this change

This is with pkg-config 0.20

Hey,
The situation is a bit borken actually, here's the story:
- Steve Langasaek is a skilled release manager for Debian and sent a patch
improving the number of ELF deps of libvte
- this worked with a new pkg-config feature which is meant to do that, but has a
nasty side-effect with respect to include PATHs
- we noticed the build failures (as Sebastian explained) of other packages, and
engaged discussion with the pkg-config maintainer, Tollef Fog Heen, but we
couldn't reach a strong consensus[1]: we're still waiting for a decision to be
taken one way or the other

For Debian, I opened a bug[2] to request proposed pkg-config changes but it's
only a proposal fix, and can be rejected.

For now, it might be best for vte to revert that patch, and mark this bug as
waiting for a newer pkg-config.

[1] one of the camp milits for linking high level libs with lower level libs of
which some headers are used in high level libs headers, the other camp milits
for only linking when functions from low level are directly called

[2] http://bugs.debian.org/340904

Reopening as patch breaks stuff

Patch backed out.

*** Bug 325030 has been marked as a duplicate of this bug. ***

Created attachment 58328 [details] [review]
use newer pkg-config private field for freetype link flags only

Hi,

While reviewing the 0.11.16 -> 17 diff, I've seen you've backed out the move to Libs.private entirely:
-Requires.private: @NEEDEDPACKAGES@
-Libs: -L${libdir} -lvte
-Libs.private: @FT2_LIBS@ @OTHERLIBS@
+Requires: @NEEDEDPACKAGES@
+Libs: -L${libdir} -lvte @FT2_LIBS@ @OTHERLIBS@

Please move at least @FT2_LIBS@ to Libs.private, that should be safe since libvte doesn't require other apps to link to libfreetype, nor do vte's headers #include freetype's headers.

For the other libs, I agree the move was dangerous since this discussion is still ongoing with pkg-config's upstream.

Loïc, what's the status on the pkg-config changes for the Requires.private? Or have we reached a conclusion with this bug?

r1605: 2007-02-02  Chris Wilson  <chris@chris-wilson.co.uk>

	Bug 322240 – Usage of pkg-config privates header

	* vte.pc.in:
		Move @FT2_LIBS@ to Libs.private.

FYI, pkg-config's upstream added support for pulling Cflags of Requires.private, so the initial patch could work if you require pkg-config >= 0.21 (see http://bugs.debian.org/340904).

The most important part if freetype though, thanks for fixing it.

Requires now only contains glib, gobject, pango, gtk all of which are public since they're exposed in vte.h. So there's nothing left to do here.

Hmm not sure what you mean here; if applications use pango / gtk etc. and vte, they should check for both, include both, and link to both.

If you agree, then Requires.private is enough and will avoid passing duplicate cflags and link flags and might avoid deps in some rare cases.

Vte trunk has:

Requires: @VTE_PKGS@

where

VTE_PKGS="glib-2.0 >= $GLIB_REQUIRED gobject-2.0 pango >= $PANGO_REQUIRED gtk+-2.0 >= $GTK_REQUIRED"

All of these are exposed in the vte.h file and therefore *must* be in Requires.

I don't think using the vte ABI implies using e.g. the pango ABI; for instance in the case of aptitude:
% nm -D =aptitude | grep vte
         U vte_reaper_add_child
         U vte_reaper_get
         U vte_terminal_forkpty
         U vte_terminal_get_type
         U vte_terminal_new
but no pango symbol is used.

I agree the pango API is exposed and some pango types are used in some vte API; does that mean we should automatically make all vte apps link to pango?

Say someone wants to set font and creates a PangoFontDescription: this would require a var of this type in the program's source, so a pango include and link should be in order -- no?

(Frankly, I don't think this makes a whole lot of difference for Vte -> glib, gtk, pango; I just want to get this right).


Also: why GObject?

Loïc, what you makes some sense for libraries, but what about the headers?  pkg-config does not support something like Requires.cflags vs Requires.libs.

I agree that gobject can be moved to Requires.private, but it doesn't make any difference at all.

The headers aren't a problem with Requires.private: pkg-config includes Cflags from requires.privates modules.

(In reply to comment #16)
> The headers aren't a problem with Requires.private: pkg-config includes Cflags
> from requires.privates modules.

What about vte users that include vte.h?  They should also add include path for pango, glib, and gtk+ to their compiler.  That's the purpose of the Requires line.

No, as I was saying, Requires.private modules are included in Cflags.  Try with some .pc with Requires.private, on my system gucharmap.pc Requires.private gconf, so:
pkg-config --cflags gucharmap | grep gconf => includes gconf
pkg-config --libs gucharmap | grep gconf => doesn't

Ah I see.  That's undocumented at best.  The common idea of what Requires.private does is that it includes Libs.private.  But hey, Requires.private is undocumented.

Maybe you get can get it documented upstream?  I'd be more than happy to fix all my modules then.

So I've spent most of this morning fixing the handling of Requires and Requires.private in misc cases and documenting it in the pkg-config man page; the result is in git.debian.org:/git/users/lool/pkg-config.

I started from the Debian package as I couldn't find the pkg-config repo (CVS seemed obsolete and couldn't find the upstream git one).  Sadly, there *is* a pkg-config repo, using bzr at http://pkg-config.freedesktop.org/bzr/...

FYI, Requires.private were documented in bzr rev tfheen@err.no-20080323194608-xulilhv46i5au8t1 which should be in pkg-config 0.23.

Any non-ssh way to access your git tree?

git://git.debian.org/git/users/lool/pkg-config

Thanks.  Donno why git defaulted to ssh.

So where is this documented?  The pkg-config.1 from your git repo still doesn't mention Requires.private.

Created attachment 123404 [details]
pkg-config man page from git.debian.org/git/users/lool/pkg-config tree; 

(It selected ssh because of the hostname + colon + pathname syntax I used)

This is my take on Requires.private:
       Requires.private:
              This is a comma-separated list  of  private  packages  that  are
              required  by  your package.  Private packages are packages which
              are needed for static linking or compile  flags.   Requires.pri‐
              vate: follows the same syntax as Requires:.

also Libs.private was updated:
       Libs.private:
              This  line  should  list  any private libraries in use.  Private
              libraries are libraries  which  are  not  exposed  through  your
              library,  but  are  needed  in the case of static linking.  This
              differs from Requires.private: in that it  references  libraries
              that do not have package files installed.

I'm attaching the pkg-config.1 man page I added to the proposed git tree.

This file is in the misc-requires-private-fixes branch, which is not the default; I used the usual layout for Debian packages, but created a new branch because I wanted upstream to review the merging; perhaps I should have merged to master myself; sorry for not mentioning the branch name;

Created attachment 123405 [details]
pkg-config man page from http://pkg-config.freedesktop.org/bzr/ tree

More importantly, this is the version from pkg-config's bzr tip which I'm attaching; it's slightly garbled and says:
              Requires.private:  A  list of packages required by this package.
              The difference from Requires is that the packages  listed  under
              Requires.private  are not taken into account when a flag list is
              computed for dynamically linked executable (i.e., when  --static
              was not specified).  In the situation where each .pc file corre‐
              sponds to a library, Requires.private shall be used  exclusively
              to specify the dependencies between the libraries.

However, what IMO really is important is the decision to support pulling Cflags from Requires.private.

Debian bug http://bugs.debian.org/340904 has the actual discussion; there was a serious bug in the initial implementation in 0.21 though.

pkg-config (0.22-1) unstable; urgency=low

  * New upstream release
    - Make Requires.private a whole lot more useful by traversing the
      whole tree, not just the top-level, for Cflags.  Closes: #390132
    - Add support for using the system glib.
...
 -- Tollef Fog Heen <tfheen@debian.org>  Mon, 18 Jun 2007 23:38:59 +0200

pkg-config (0.21-1) unstable; urgency=low

  * New upstream release
...
    - Always adds all cflags.  (closes: #340904)
...
 -- Tollef Fog Heen <tfheen@debian.org>  Wed, 16 Aug 2006 21:11:21 +0200

I see.  I wish a new token was added for that purpose.  It's really messy.  Say, eg, package X uses glib internally because that makes writing C code easier, but it doesn't expose it in its API.  Now everyone using X ends up adding include path for glib unnecessarily.

Anyway, that's how it is.  Do you think it's worth moving them in vte now?

Also makes one wonder what is Requires: supposed to be used for these days.

I don't think adding a new token was considered; perhaps this idea could have some sympathy upstream?  I know the change in behavior of Requires.private was a long debate.  Adding a new token would have been more granular and less invasive I guess.  I don't know whether or not this contradicts some design decision in pkg-config.

Requires are still useful for link flags and cases like gtk -> gdk-${backend} at least, but probably less so I guess.

In Debian we'd like to see more use of Requires.private with their new behavior to reduce the number of ELF deps because it will reduce package dependencies, help startup time (you know it's a hot topic ;-), and cause less warning about unused libraries during package builds.

(--as-needed works for this, but it's a workaround, and breaks some rare use cases; I usually recommending it after -z defs, but python stuff doesn't like this.)

At the end of the day, any use of Requires is debatable.  For example, why should an application only using gtk_* have to link to gdk?

I'm willing to bring this up upstream, but where is upstream? :)

heh; I think one clear use of define is when you do something like:
#define gtk_foo g_bar

I can't come up with many instances of this, but this could be one:
#define gtk_marshal_VOID__STRING  g_cclosure_marshal_VOID__STRING

Upstream is at bugs.freedesktop.org apparently.

Ok, reported upstream:
http://bugs.freedesktop.org/show_bug.cgi?id=18815

Christian, is it that the patch doesn't apply to tip anymore?

Given the discussion, I'm not sure if it's productive pushing this change in all packages.  It's a step in the wrong direction IMO.

Let's WONTFIX this then.