On Wed, Feb 4, 2009 at 11:48 AM, Paulo César Pereira de Andrade <p...@mandriva.com.br> 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 $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. > 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. > 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" -- Dan _______________________________________________ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg