[PATCH xserver] config: declare xserver private dependencies in xorg-server.pc

Gaetan Nadon memsize at videotron.ca
Tue Jun 15 07:45:21 PDT 2010


On Mon, 2010-06-14 at 21:03 -0700, Dan Nicholson wrote:

> On Mon, Jun 14, 2010 at 6:59 PM, Gaetan Nadon <memsize at videotron.ca> wrote:
> > On Mon, 2010-06-14 at 15:07 -0700, Dan Nicholson wrote:
> >
> > On Sun, Jun 13, 2010 at 1:49 PM, Gaetan Nadon <memsize at videotron.ca> wrote:
> >> Any module (drivers) depending on xserver also depends on some of the
> >> server private dependencies. Any driver including xf86.h depends on
> >> xext, kbproto, inputproto and randr.
> >>
> >> These dependencies are in separate packages, so anything can happen,
> >> removal, wrong version, etc... and the driver fails during compilation.
> >> Having the private dependencies declared will ensure all packages the
> >> server depends on are present and at the correct version.
> >>
> >> Currently each module attempts to check for server dependencies with
> >> various degrees of accuracy. With this patch, the driver will only need
> >> to check for its own explicit dependencies.
> >>
> >> Signed-off-by: Gaetan Nadon <memsize at videotron.ca>
> >> ---
> >>  configure.ac      |    9 ++++++++-
> >>  xorg-server.pc.in |    1 +
> >>  2 files changed, 9 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/configure.ac b/configure.ac
> >> index 4ada8f5..cb33637 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -793,9 +793,13 @@ WINDOWSWMPROTO="windowswmproto"
> >>  APPLEWMPROTO="applewmproto >= 1.4"
> >>
> >>  dnl Core modules for most extensions, et al.
> >> -REQUIRED_MODULES="[randrproto >= 1.2.99.3] [renderproto >= 0.11]
> >> [fixesproto >= 4.1] [damageproto >= 1.1] [xcmiscproto >= 1.2.0] [xextproto
> >> >= 7.0.99.3] [xproto >= 7.0.17] [xtrans >= 1.2.2] [bigreqsproto >= 1.1.0]
> >> fontsproto [inputproto >= 1.9.99.902] [kbproto >= 1.0.3]"
> >> +SDK_REQUIRED_MODULES="[randrproto >= 1.2.99.3] [renderproto >= 0.11]
> >> [xextproto >= 7.0.99.3] [inputproto >= 1.9.99.902] [kbproto >= 1.0.3]"
> >> +REQUIRED_MODULES="[fixesproto >= 4.1] [damageproto >= 1.1] [xcmiscproto
> >> >= 1.2.0] [xproto >= 7.0.17] [xtrans >= 1.2.2] [bigreqsproto >= 1.1.0]
> >> fontsproto $SDK_REQUIRED_MODULES"
> >>  REQUIRED_LIBS="xfont xau"
> >>
> >> +# Make SDK_REQUIRED_MODULES available for inclusion in xorg-server.pc
> >> +AC_SUBST(SDK_REQUIRED_MODULES)
> >> +
> >>  dnl List of libraries that require a specific version
> >>  LIBAPPLEWM="applewm >= 1.4"
> >>  LIBDMX="dmx >= 1.0.99.1"
> >> @@ -947,6 +951,7 @@ if test "x$XV" = xyes; then
> >>        AC_DEFINE(XV, 1, [Support Xv extension])
> >>        AC_DEFINE(XvExtension, 1, [Build Xv extension])
> >>        REQUIRED_MODULES="$REQUIRED_MODULES $VIDEOPROTO"
> >> +       SDK_REQUIRED_MODULES="$SDK_REQUIRED_MODULES $VIDEOPROTO"
> >>  else
> >>        XVMC=no
> >>  fi
> >> @@ -1036,6 +1041,7 @@ case "$DRI2,$HAVE_DRI2PROTO" in
> >>        yes,yes | auto,yes)
> >>                AC_DEFINE(DRI2, 1, [Build DRI2 extension])
> >>                DRI2=yes
> >> +               SDK_REQUIRED_MODULES="$SDK_REQUIRED_MODULES $DRI2PROTO"
> >>                ;;
> >>  esac
> >>  AM_CONDITIONAL(DRI2, test "x$DRI2" = xyes)
> >> @@ -1074,6 +1080,7 @@ if test "x$XINERAMA" = xyes; then
> >>        AC_DEFINE(XINERAMA, 1, [Support Xinerama extension])
> >>        AC_DEFINE(PANORAMIX, 1, [Internal define for Xinerama])
> >>        REQUIRED_MODULES="$REQUIRED_MODULES $XINERAMAPROTO"
> >> +       SDK_REQUIRED_MODULES="$SDK_REQUIRED_MODULES $XINERAMAPROTO"
> >>  fi
> >>
> >>  AM_CONDITIONAL(XACE, [test "x$XACE" = xyes])
> >> diff --git a/xorg-server.pc.in b/xorg-server.pc.in
> >> index 44f886a..1fa4fb5 100644
> >> --- a/xorg-server.pc.in
> >> +++ b/xorg-server.pc.in
> >> @@ -16,5 +16,6 @@ Name: xorg-server
> >>  Description: Modular X.Org X Server
> >>  Version: @PACKAGE_VERSION@
> >>  Requires: pixman-1 pciaccess xproto >= 7.0.17
> >> +Requires.private: @SDK_REQUIRED_MODULES@
> >
> > Could you also move the xproto requirement from REQUIRED_MODULES to
> > SDK_REQUIRED_MODULES so it's always populated from configure.ac? Maybe
> > that can go in another commit. Probably all the modules should be in
> > Requires.private, but that's probably another commit, too.
> >
> > I originally put all modules (save for xproto) in Requires.private. Julien
> > suggested to put only the modules exposed in the sdk.
> 
> Hmm, not sure why that would be since it wouldn't make a difference
> for the proto packages. pkg-config --cflags will return the settings
> whether they're in Requires or Requires.private, even on the broken
> pkg-config releases.


I thought private meant the cflags would not be returned.

> 
> > As for xproto, all drivers use it explicitly and need the include path. I
> > thought it was safer to leave it there as there might be an older driver
> > that did not list xproto on the PKG_CHECK_MODULES statement and could
> > potentially break. I am not sure I understand why you were suggesting that.
> 
> Right now we have xproto >= 7.0.17 explicitly in both configure and
> xorg-server.pc.in. If it's only in configure, then when you bump the
> requirement for some reason, you don't risk the .pc file getting out
> of sync.

That's an xserver issue where xproto >= 7.0.17 is hand-coded in 2
places, configure.ac and xorg-server.pc.
A variable should be used. 

> 
> I don't know how it would break any drivers. The installed .pc file
> will contain xproto >= 7.0.17 whether you substitute it from configure
> or not. Whether the drivers have listed xproto in their
> PKG_CHECK_MODULES shouldn't make a difference on how the xserver
> specifies its requirement.
> 

Correct, with my misunderstanding cleared. That leaves one question:
what's the difference between Requires and Requires.private?


        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 corresponds to
                   a library, Requires.private shall be used exclusively to specify
                   the dependencies between the libraries.


Reading http://people.freedesktop.org/~dbn/pkg-config-guide.html:


        Finally, the Cflags contains the compiler flags for using the library. 
        Unlike the Libs field, there is not a private variant of Cflags. 
        This is because the data types and macro definitions are needed regardless of the linking scenario.


My conclusion is that all the proto packages the server depends on
should be declared in Requires.private, including xproto. Any driver
wishing to use a specific protocol should check for it in
PKG_CHECK_MODULES. That is what they do today. In additional of working
correctly, this code will also reflect and document the system design.

This only leaves the question of "other modules" upon which the server
depends but not the drivers. It is tempting to craft a shorter list as
this patch did, but it is not future proof. One day, a header from, say,
damageproto can find it's way through xf86.h and no one will think of
updating SDK_REQUIRED_MODULES. I think it is both correct and less error
prone for the future to use the current REQUIRED_MODULES list. This is
also consistent with "drivers are part of the server" architecture
statement.

I'll post a new version.

Thanks

> --
> Dan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100615/4c394392/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100615/4c394392/attachment-0001.pgp>


More information about the xorg-devel mailing list