xorg/driver/xf86-input-evdev: [PATCH] Correct make distchek using the same solution used in other packages.

[Dan Nicholson]

On Wed, Feb 4, 2009 at 2:25 PM, Paulo César Pereira de Andrade
<pcpa at mandriva.com.br> wrote:
> Dan Nicholson wrote:
>
>>>  These cases should really be addressed in a different
>>> way, as the addition of a option that is only useful to
>>> pass distcheck is wrong.
>>
>> For the patch, please use backticks (``) rather than $() for command
>> substitution since it's more portable. Also, please use the variable
>
>  I just reverted to what was already in place, but I agree, and
> would prefer to remove gratuitous bash'isms.
>
>> $PKG_CONFIG. PKG_CHECK_MODULES (more specifically,
>> PKG_PROG_PKG_CONFIG) has already looked up the tool for us, and using
>> pkg-config directly means that we might not be honoring the user's
>> wishes. I know that's not how the app-defaults patches were handled,
>> but that doesn't make it right.
>
>  You mean, it creates a XORG_sdkdir shell variable?
> My reading of m4 cannot find it in pkg.m4 (maybe I did not look
> carefully enough ...) That would be very useful indeed to replace
> the current calls.

Not quite. What I mean is, we're doing this:

foo=`pkg-config --variable=bar baz`

Instead, the correct pkg-config to use has already been looked up and
put in $PKG_CONFIG by PKG_CHECK_MODULES. This could be
/usr/bin/pkg-config or pkg-config-i686-pc-linux-gnu or
$HOME/bin/pkg-config or whatever. The user may have specified
PKG_CONFIG from the environment. We should respect that like the
pkg.m4 checks do.


>>>  Another "cosmetic" thing that should be addressed is
>>> usage of something like:
>>> PKG_CHECK_MODULES(XORG, xorg-server xproto $REQUIRED_MODULES)
>>> AC_SUBST(XORG_CFLAGS)
>>>
>>>  First the automake macro says:
>>>
>>> Checking for XORG... yes
>>>
>>> while it should say something more like:
>>>
>>> Checking for xorg-server...
>>> Checking for xproto...
>>>
>>> or maybe in the same line, but not really a xorg issue,
>>> but a pkg-config issue?
>>
>> That's how PKG_CHECK_MODULES works. I think it used to spit out all
>> the deps it's looking up, but it gets out of hand when there are a lot
>> of them (look at the xserver to see how many modules are used at
>> once). You can look in config.log to see the details if you need to.
>
>  It is just illogical what it prints. If the problem is that
> there may be too much things to print, it could just put print
> something different, like:
> Checking for required pkg-config modules...

It prints XORG because that's the identifier we gave to
PKG_CHECK_MODULES. You could ask on the pkg-config list why it's like
that, but I suspect you'll just get pointed to an old discussion.
Shrug...

>>> The AC_SUBST issue is because there is a lot of mixed usage
>>> in Makefiles of @XORG_CFLAGS@ and $(XORG_CFLAGS)
>>
>> It doesn't usually matter in practice, but it's nicer to use the
>> variable $(var) rather than hardcoding the substitution so that a user
>> could override it if necessary:
>>
>> make XORG_CFLAGS="-I/i/know/what/i_m/doing"
>
>  When possible I have been using $(XORG_CFLAGS), but did not
> want to "gratuitously" do a s/@(\w+)@/$(\1)/g in Makefile.am's...
> but it could be a good thing to do, mainly for consistency.

A rainy day project if you feel like it. In practice, very few people
are going to find the need to override the variables, but it's nice to
not lose the possibility for no reason.

--
Dan