[PATCH][1/2] - Xserver Autoconfiguration Merge - Autotoolize Xserver

[Shawn Starr]

On April 19, 2005 10:02, GOMBAS Gabor wrote:
> Hi,
>
> On Mon, Apr 18, 2005 at 11:19:03PM -0400, Shawn Starr wrote:
> > Patches to autotoolize the Xorg Xserver
>
> Some comments/suggestions after a quick reading of the patch:
>
> - Neither AUTOMAKE_OPTIONS nor AM_INIT_AUTOMAKE states what is the
>   minimum version of automake you are willing to support. I suggest
>   requiring some recent version if you do not want to go through all the
>   bugs present in older versions. My suggestion is to require at least
>   automake-1.7 (1.6 still had some rather annoying "features").

Version Numbers have not been settled on yet. Though I don't we can use the 
newest versions.

> - Do not use AC_TRY_RUN/AC_TRY_COMPILE/AC_TRY_LINK, but use the
>   AC_*_IFELSE macros instead. The AC_TRY_* macros are declared
>   obsolote in recent versions of autoconf. The AC_*_IFELSE macros
>   already exist in autoconf-2.53 even if they were only announced in
>   autoconf-2.55. In fact, autoconf-2.53 already implements
>   AC_TRY_COMPILE as

That maybe the case. But again, we haven't settled on the versions to use yet. 

>   AC_DEFUN([AC_TRY_COMPILE],
>   [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[$1]], [[$2]])], [$3], [$4])])
>
> - Use AC_HELP_STRING for formatting the description of AC_ARG_WITH
>   and AC_ARG_ENABLE macros. This will give more uniform "configure
>   --help" output. Manally adding spaces to the description is more error
>   prone.

Agreed, I can fix this.

> - Minor style suggestion: decide between 'test x$var = xval' and
>   'test "$var" = val'. Mixing the two styles inside the same
>   configure.ac might be confusing.

Agreed, consistency can be corrected.

> - SysV shared memory check: I'd suggest only checking for the
>   availability of the _interface_ in configure.ac by using
>   AC_LINK_IFELSE, and not for the run-time availability using
>   AC_TRY_RUN. Reason: you have to do a run-time check anyway because the
>   user might boot a different kernel after compiling Xorg but before
>   starting the X server. Avoiding AC_TRY_RUN/AC_RUN_IFELSE as much as
>   possible makes life a lot easier when cross-compiling.

I can examine where we can do some cleanups.

> - Unaligned access checking and AC_RUN_IFELSE in general: If you cannot
>   avoid AC_RUN_IFELSE, always put it inside AC_CACHE_CHECK to ensure
>   that cross-compilation is still possible by using a pre-seeded
>   config.cache.

Sure, caching is a good thing.

> - The DEFAULT_VENDOR_RELEASE variable seems to be completely redundant.
>   The version is already passed to AC_INIT and it can be accessed using
>   the AC_PACKAGE_VERSION macro (or the PACKAGE_VERSION shell variable).

Unsure, I haven't examined what the variable does yet.

> - About macro argument quoting: although it is safe not to quote literal
>   string arguments I'd suggest still quoting them anyway. The extra
>   annoyance is small (just 2 characters), while there are more benefits:
>   - configure.ac will look more consistent.
>   - If somebody later replaces the string literal with a macro it will
>     still "just work" as expected. Without proper quoting "interesting"
>     things may happen.
>   - "Always quote" is much easier to learn for people not familiar with
>     autoconf/m4 quoting rules than "quote most of the time except...".

>     Well, there is one exception: AC_HELP_STRING; life is though...
>     (and there are other exceptions if you want to use the power of m4
>     not just the autoconf macros but then you already have to be
>     familiar with the quoting rules).

Sure, I can cleanup that. 

> Gabor

Keep in mind, this was the original DebriX autotoolizing effort. We are just 
building on their work and this is the discussions I was looking forward to 
seeing. I'll get right on it :-)

Shawn.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.x.org/archives/xorg-modular/attachments/20050419/878a7f38/attachment-0001.pgp