Bug 4037 - X libraries don't make proper use of "Requires" header in pkg-config files.
Summary: X libraries don't make proper use of "Requires" header in pkg-config files.
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/Xlib (show other bugs)
Version: git
Hardware: All All
: high normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
: 5065 5166 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-08-10 13:51 UTC by James Henstridge
Modified: 2011-10-02 03:34 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Proposal patch (25.08 KB, patch)
2005-09-30 02:40 UTC, Dawid Gajownik
no flags Details | Splinter Review
Proposal patch II (25.08 KB, patch)
2005-09-30 02:50 UTC, Dawid Gajownik
no flags Details | Splinter Review

Description James Henstridge 2005-08-10 13:51:45 UTC
The libX11 pkg-config file directly includes the cflags and libs of the
libraries it depends on, rather than referencing them in "Requires" line.

This means that if a problem is found in one of the .pc files (such as the
-D_XOPEN_SOURCE problem that has since been fixed), all the libraries that
depend on the broken .pc file need to be rebuilt to fix their .pc files.

If you use the "Requires" line, you only need to fix the package in question
(which I gather is one of the benefits people want from modularisation ...).

The last few lines of the x11.pc.in file should read something like:
  Requires: xproto
  Libs: -L${libdir} -lX11 @LIBS@
  Cflags: -I${includedir}

Other .pc files need similar fixes.
Comment 1 James Henstridge 2005-08-10 17:04:12 UTC
adding as a dep for 1690.

While this doesn't affect the ability to release source packages in a modular
fashion it'd cause unnecessary binary package updates for .pc file bugs if this
is not fixed, which reduces the benefits of the modularisation to distributors.
Comment 2 Dawid Gajownik 2005-09-30 02:40:54 UTC
Created attachment 3438 [details] [review]
Proposal patch

Here's a proposal patch. May someone review it? I hope I haven't screw
something up ;-)

Some comments:
* in libX11/x11.pc.in there is `@LIBS@' but I don't see where it's defined in
configure.ac :/ Maybe there should be `@X11_LIBS'?
* some inconsistency with using AC_SUBST(FOO_LIBS) and AC_SUBST(FOO_CFLAGS).
Some packages use it after PKG_CHECK_MODULES and some not. I'm newbie in
pkg-config world but it seems that it's not needed (at least if you use only
@FOO_LIBS@ and @FOO_CFLAGS@)
* libXau (stupid question): should XTHREAD_CFLAGS be also included in xau.pc.in
like in libX11/x11.pc.in file?
* libXdamage/configure.ac - I don't understand why do you need support for X11
libs installed without x11.pc file? Is that really necessary?
* libXevie/xevie.pc.in is broken. Some variables are not substituted after
runing ./configure script (X_LIBS and X_CFLAGS are not defined in
configure.ac). Right now this file is a symlink to the xc/lib/Xevie/xevie.pc.in
file so I would suggesting creating new file with this content:

--8<-------------------------------------------
prefix=@prefix@
exec_prefix=@exec_prefix@
libdir=@libdir@
includedir=@includedir@

Name: Xevie
Description: X Event Interceptor Library
Version: @VERSION@
Requires: xproto x11 xextproto xext evieproto
Cflags: -I${includedir}
Libs: -L${libdir} -lXcursor
--8<-------------------------------------------
(it's a bit modified
http://cvs.freedesktop.org/*checkout*/xorg/xc/lib/Xevie/xevie.pc.in file). I'm
not shure about this `-lXcursor' flag. Isn't this some kind of mistake?
Shouldn't be there `-lXevie'?
* I don't know how to handle conditional compilation so I haven't touched
libXfont, lbxutil and something else ;-) This should be investigated further.
* libXpm/xpm.pc.in uses @LIBS@ which is not defined in configure.ac
* libXft - support for `non-pkg-configed` Xrender. Is that necessary? (BTW do
you need to provide xft-config script? xft.pc seems to be prefered way of
configuring sources)
* libXrender/configure.ac - I don't understand what is this for:

--8<------------------------------
X_REQUIRES="x11"
X_NON_PKG_CFLAGS=""
X_NON_PKG_LIBS=""

AC_SUBST(X_REQUIRES)
AC_SUBST(X_NON_PKG_CFLAGS)
AC_SUBST(X_NON_PKG_LIBS)
--8<------------------------------

In xrender.pc.in there is something like this:

--8<------------------------------
Requires: @X_REQUIRES@
Cflags: -I${includedir} @RENDER_CFLAGS@ @X_NON_PKG_CFLAGS@
Libs: -L${libdir} -lXrender @X_NON_PKG_LIBS@
--8<------------------------------

Adding empty variables does not make any sense to me :/

That would be all. As I sad before, I don't have experience with these things
so please forgive me stupid questions ;-)
Comment 3 Dawid Gajownik 2005-09-30 02:50:23 UTC
Created attachment 3439 [details] [review]
Proposal patch II

Ooops. There was a small typo in previous patch. Sorry.
Comment 4 James Henstridge 2005-09-30 03:39:32 UTC
Here are answers to some of your questions.  You should probably also get one of
the developers to look over your patch and queries.

(In reply to comment #2)
> Some comments:
> * in libX11/x11.pc.in there is `@LIBS@' but I don't see where it's defined in
> configure.ac :/ Maybe there should be `@X11_LIBS'?

Autoconf automatically adds libraries found with AC_CHECK_LIB() to the LIBS
environment variable.  Whether it should be in the .pc file depends on what the
configure script is checking for.

> * some inconsistency with using AC_SUBST(FOO_LIBS) and AC_SUBST(FOO_CFLAGS).
> Some packages use it after PKG_CHECK_MODULES and some not. I'm newbie in
> pkg-config world but it seems that it's not needed (at least if you use only
> @FOO_LIBS@ and @FOO_CFLAGS@)

The extra AC_SUBST() calls are not necessary with Automake-1.7 and higher.  They
don't hurt, but they don't help either.

> * libXdamage/configure.ac - I don't understand why do you need support for X11
> libs installed without x11.pc file? Is that really necessary?

Probably to support installing the package on systems with X installations other
than X11R7.0.

> * I don't know how to handle conditional compilation so I haven't touched
> libXfont, lbxutil and something else ;-) This should be investigated further.

The usual way would be to AC_SUBST() a variable containing the list of libraries
/ package requrements from the configure script, and use @varname@ in the .pc
file.  Cairo might be a good place to look for examples.

> * libXpm/xpm.pc.in uses @LIBS@ which is not defined in configure.ac

See answer above.

> * libXft - support for `non-pkg-configed` Xrender. Is that necessary? (BTW do
> you need to provide xft-config script? xft.pc seems to be prefered way of
> configuring sources)
> * libXrender/configure.ac - I don't understand what is this for:
> 
> --8<------------------------------
> X_REQUIRES="x11"
> X_NON_PKG_CFLAGS=""
> X_NON_PKG_LIBS=""
> 
> AC_SUBST(X_REQUIRES)
> AC_SUBST(X_NON_PKG_CFLAGS)
> AC_SUBST(X_NON_PKG_LIBS)
> --8<------------------------------
> 
> In xrender.pc.in there is something like this:
> 
> --8<------------------------------
> Requires: @X_REQUIRES@
> Cflags: -I${includedir} @RENDER_CFLAGS@ @X_NON_PKG_CFLAGS@
> Libs: -L${libdir} -lXrender @X_NON_PKG_LIBS@
> --8<------------------------------
> 
> Adding empty variables does not make any sense to me :/

To support the case when libX11 and friends do not provide a .pc file.  Maybe
support for that has been removed from the configure script without cleaning it
up properly.
Comment 5 Alan Coopersmith 2005-09-30 09:31:02 UTC
> * libXau (stupid question): should XTHREAD_CFLAGS be also included in xau.pc.in
> like in libX11/x11.pc.in file?

No - for libX11, the XTHREAD settings affect the interfaces other libraries use
to interact with libX11 and they need to match.    libXau just needs to use the
flags to ensure they call the right functions in the system libraries, and other
software that calls libXau doesn't need to be built any differently.  Ideally
Xau would also depend on x11.pc.in to set them, but then we'd have a dependency
loop.
Comment 6 Adam Jackson 2005-11-19 04:48:40 UTC
*** Bug 5065 has been marked as a duplicate of this bug. ***
Comment 7 Alan Coopersmith 2005-11-19 04:55:55 UTC
Assigning to kem, since he's almost done fixing this.
Comment 8 Kevin E. Martin 2005-11-19 18:31:06 UTC
Fixed in CVS.
Comment 9 Alan Coopersmith 2005-11-26 05:47:39 UTC
*** Bug 5166 has been marked as a duplicate of this bug. ***
Comment 10 Egmont Koblinger 2005-11-29 22:55:05 UTC
In current cvs, xorg-server.pc still doesn't contain any Requires line,
though the header files shipped by that package require some of the protocol
header files (xproto and friends).
Comment 11 Daniel Stone 2005-12-16 17:33:23 UTC
too late, too late, will be the cry, when the man with the bargains has passed
you by
Comment 12 Egmont Koblinger 2005-12-17 03:49:10 UTC
One more, in addition to comment 10:

xkbui.pc lacks Requires.private: xkbfile, though libxkbui.la mentions
/usr/lib/libxkbfile.la in dependency_libs.
Comment 13 Adam Jackson 2006-05-19 03:30:43 UTC
push to 7.2 tracker.

anyone happen to have the list of what external headers the server's headers
require?
Comment 14 Kevin E. Martin 2006-12-18 14:46:53 UTC
Reassigning to component owner so that someone can take a look at this.
Comment 15 Daniel Stone 2007-02-27 01:27:36 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 16 Adam Jackson 2007-04-08 13:36:27 UTC
Move to 7.3 tracker.
Comment 17 Eric Anholt 2007-09-04 17:10:43 UTC
This is certainly a valid bug, but we've had successful releases without it getting fixed.  Removing blocker.
Comment 18 Jeremy Huddleston Sequoia 2011-10-02 03:34:21 UTC
I just checked libXaw and libXt and it looks like they're ok.  I'm guessing 
this was fixed a while ago.  If any issues remain, please reopen.


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.