[PATCH v3 1/2] dix: use one single function to register fpe fonts

[Dan Nicholson]

On Sat, Jun 19, 2010 at 9:29 AM, Vignatti Tiago (Nokia-D/Helsinki)
<tiago.vignatti at nokia.com> wrote:
> On Sat, Jun 19, 2010 at 05:34:52PM +0200, ext Dan Nicholson wrote:
>> On Wed, Jun 16, 2010 at 8:52 AM, Tiago Vignatti
>> <tiago.vignatti at nokia.com> wrote:
>> > X server doesn't need to understand fpe internals, so use
>> > register_fpe_functions from libXfont.
>> >
>> > It's required to get new version of libXfont, therefore adjust it to be passed
>> > to autoconf.
>> >
>> > Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
>> > Reviewed-by: Mikhail Gusarov <dottedmag at dottedmag.net>
>> > Reviewed-by: Alex Deucher <alexdeucher at gmail.com>
>> > ---
>> > changes from v2:
>> > - modified with Julien's suggestion to not check libXfont again due it's being
>> >  already being done later when REQUIRED_LIBS is checked.
>> >
>> >  configure.ac      |    9 +++++----
>> >  dix/dixfonts.c    |    4 +---
>> >  include/dixfont.h |    5 +----
>> >  3 files changed, 7 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index d41191f..bb4f445 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -794,7 +794,7 @@ 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]"
>> > -REQUIRED_LIBS="xfont xau"
>> > +REQUIRED_LIBS="xau"
>> >
>> >  dnl List of libraries that require a specific version
>> >  LIBAPPLEWM="applewm >= 1.4"
>> > @@ -803,6 +803,7 @@ LIBDRI="dri >= 7.8.0"
>> >  LIBDRM="libdrm >= 2.3.0"
>> >  LIBGL="gl >= 7.1.0"
>> >  LIBXEXT="xext >= 1.0.99.4"
>> > +LIBXFONT="xfont >= 1.4.2"
>> >  LIBXI="xi >= 1.2.99.1"
>> >  LIBXTST="xtst >= 1.0.99.2"
>> >  LIBPCIACCESS="pciaccess >= 0.8.0"
>> > @@ -812,10 +813,10 @@ LIBSELINUX="libselinux >= 2.0.86"
>> >  LIBDBUS="dbus-1 >= 1.0"
>> >  LIBPIXMAN="pixman-1 >= 0.15.20"
>> >
>> > -dnl Pixman is always required, but we separate it out so we can link
>> > -dnl specific modules against it
>> > +dnl Pixman and Xfont are always required. For pixman we separate it out so we
>> > +dnl can link specific modules against it
>> >  PKG_CHECK_MODULES(PIXMAN, $LIBPIXMAN)
>> > -REQUIRED_LIBS="$REQUIRED_LIBS $LIBPIXMAN"
>> > +REQUIRED_LIBS="$REQUIRED_LIBS $LIBPIXMAN $LIBXFONT"
>>
>> I don't understand this diff at all. Why not just do
>>
>> -REQUIRED_LIBS="xfont xau"
>> +REQUIRED_LIBS="xfont >= 1.4.2 xau"
>>
>> since it ends up in exactly the same place (REQUIRED_LIBS). It's being
>> used in exactly the same way it always was, but we just require a
>> newer version.
>>
>
> I've been cheated by the commentary "List of libraries that require a specific
> version", given the server now would require the given version of xfont. Even
> so I do agree with you that is much more cleaner simply put it REQUIRED_LIBS.
>
> Do you think I should move there and change the commentary a bit?

I would just add the version in configure.ac and explain in the commit
message why we need a specific version. All the DDXs were already
being linked to libXfont, now they just need a specific version.

In another commit I think you could clean up configure.ac by moving
all the module versions before REQUIRED_*.

XPROTO="xproto >= 7.0.17"
...
LIBXFONT="xfont >= 1.4.2"
...

REQUIRED_MODULES="$XPROTO ..."
REQUIRED_LIBS="$LIBXFONT ..."

On the other hand, if the versions are only ever being used in
REQUIRED_MODULES or REQUIRED_LIBS, then it doesn't help that much to
obfuscate it into another variable.

--
Dan