On Tue, Jun 15, 2010 at 7:45 AM, Gaetan Nadon <mems...@videotron.ca> wrote: > On Mon, 2010-06-14 at 21:03 -0700, Dan Nicholson wrote: > > On Mon, Jun 14, 2010 at 6:59 PM, Gaetan Nadon <mems...@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 <mems...@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 <mems...@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.
Right. It's already there in REQUIRED_MODULES. If you move it to SDK_REQUIRED_MODULES and remove the current reference in xorg-server.pc.in, then you've got your variable. > 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. Yes, there's almost no reason to ever use Requires. One exception is if you want to keep compatibility with pkg-config versions where the Requires.private handling was broken. This is the reason you see Requires used a lot in the xorg .pc files. Either way, it will only affect the libraries, so you can leave pixman and pciaccess under Requires and nothing will change. > 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. Future additions to {SDK_,}REQUIRED_MODULES would require a review of whether the module is exposed in the API. Not much you can do here except be careful when introducing API. -- Dan _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel