On 12/01/2012 21:50, Gaetan Nadon wrote: > On 12-01-12 11:49 AM, Jon TURNEY wrote: >> Since checking all those warning flags takes an amount of time I find >> irritating, >> here is an attempt at caching the results of checking compiler warning flag >> support >> >> A couple of aspects which need definitely need review or testing: >> >> * I've tried to get things right when not the first flag in the list of >> alternates is >> supported, but this really needs testing with a compiler other than gcc > I used an unrecognised flag such as "-i" and moved it around in the list. >> >> * The cache variable naming policy is a bit opaque me, so the names used may >> need corecting >> >> * I've tried to be careful, but who knows what portability sins I have >> committed :-) > I did not see anything suspicious, everything is autoconf blessed.
Thanks for the review. >> +dnl Some hackery here since AC_CACHE_VAL can't handle a non-literal varname >> and >> +dnl flag may contains spaces as it may contain multiple flags, but a >> cacheid can't >> AC_MSG_CHECKING([if $CC supports ]flag[]) >> - AC_LINK_IFELSE([AC_LANG_PROGRAM([int i;])], >> - [supported=yes], [supported=no]) >> - AC_MSG_RESULT([$supported]) >> + cacheid=`AS_ECHO_N([xorg_cv_cc_flag_]flag[]) | $SED 's/ /_/'` > Can you have a look at: > http://www.gnu.org/software/autoconf/manual/autoconf.html#index-AS_005fTR_005fSH-1519 > > # This outputs "Have it!". > header="sys/some file.h" > eval AS_TR_SH([HAVE_$header])=yes > if test "x$HAVE_sys_some_file_h" = xyes; then echo "Have it!"; fi > > This example seem to indicate you would not need to replace spaces with > underscore yourself. You are correct, AS_TR_SH applies a suitable transformation that the result is a valid shell variable name, so I don't need to use sed at all here. Updated patch with that removed attached. > I have one question regarding how you use the cache file. You are aware > that some of the Automake variables we create in the various xorg > modules do not have a unique name across packages. For example. in the > driver section, PKG_CHECK MODULES(XORG, ...) is very common. The > variable pkg_cv_XORG_LIBS does not have the same value for all drivers. > Using a cache file will almost always return the wrong value. How do you > work around this issue? At the moment, I am ignoring this issue, and just use a single cache file when jhbuilding the whole Xorg stack. This is probably the wrong thing to do. :-) I don't have a specific issue with the pkg_cv_XORG_LIBS variable for drivers, as I generally don't build the drivers, not being of much use on my platform of interest. Also, looking at pkg.m4, if I am reading it correctly, it seems to me that pkg_cv_ variables aren't used like normal cache variables: pkg-config is run (once) on each configure run. This might explain why I don't have more problems with my sloppy approach.
>From f72c2c5cc184fe5266d0366b1d87e9b70b0d593e Mon Sep 17 00:00:00 2001 From: Jon TURNEY <[email protected]> Date: Mon, 2 Jan 2012 19:42:10 +0000 Subject: [PATCH macros] Cache the results of checking compiler flag support in XORG_TESTSET_CFLAG Since checking all those warning flags takes an amount of time I find irritating, here is an attempt at caching the results of checking compiler warning flag support A couple of aspects which need definitely need review or testing: * I've tried to get things right when not the first flag in the list of alternates is supported, but this really needs testing with a compiler other than gcc * The cache variable naming policy is a bit opaque me, so the names used may need corecting * I've tried to be careful, but who knows what portability sins I have committed :-) This change reduces the time to run the ./configure script produced from a configure.ac containing just AC_INIT([test], 1.0) XORG_COMPILER_FLAGS XORG_CWARNFLAGS XORG_STRICT_OPTION AC_OUTPUT() from ~60s to ~20s on my cygwin machine, and from ~15s to ~9s on a linux VM running on the same hardware v2: AS_TR_SH transforms characters which are invalid in shell variable names, so we don't have to use sed to transform spaces ourself Signed-off-by: Jon TURNEY <[email protected]> --- xorg-macros.m4.in | 34 +++++++++++++++++++++------------- 1 files changed, 21 insertions(+), 13 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index 8197eb6..9e6acf5 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -1457,11 +1457,12 @@ xorg_testset_save_CFLAGS="$CFLAGS" if test "x$xorg_testset_unknown_warning_option" = "x" ; then CFLAGS="$CFLAGS -Werror=unknown-warning-option" - AC_MSG_CHECKING([if $CC supports -Werror=unknown-warning-option]) - AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])], - [xorg_testset_unknown_warning_option=yes], - [xorg_testset_unknown_warning_option=no]) - AC_MSG_RESULT([$xorg_testset_unknown_warning_option]) + AC_CACHE_CHECK([if $CC supports -Werror=unknown-warning-option], + xorg_cv_cc_flag_unknown_warning_option, + AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])], + [xorg_cv_cc_flag_unknown_warning_option=yes], + [xorg_cv_cc_flag_unknown_warning_option=no])) + xorg_testset_unknown_warning_option=$xorg_cv_cc_flag_unknown_warning_option CFLAGS="$xorg_testset_save_CFLAGS" fi @@ -1470,11 +1471,12 @@ if test "x$xorg_testset_unused_command_line_argument" = "x" ; then CFLAGS="$CFLAGS -Werror=unknown-warning-option" fi CFLAGS="$CFLAGS -Werror=unused-command-line-argument" - AC_MSG_CHECKING([if $CC supports -Werror=unused-command-line-argument]) - AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])], - [xorg_testset_unused_command_line_argument=yes], - [xorg_testset_unused_command_line_argument=no]) - AC_MSG_RESULT([$xorg_testset_unused_command_line_argument]) + AC_CACHE_CHECK([if $CC supports -Werror=unused-command-line-argument], + xorg_cv_cc_flag_unused_command_line_argument, + AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])], + [xorg_cv_cc_flag_unused_command_line_argument=yes], + [xorg_cv_cc_flag_unused_command_line_argument=no])) + xorg_testset_unused_command_line_argument=$xorg_cv_cc_flag_unused_command_line_argument CFLAGS="$xorg_testset_save_CFLAGS" fi @@ -1491,12 +1493,18 @@ m4_foreach([flag], m4_cdr($@), [ CFLAGS="$CFLAGS ]flag[" +dnl Some hackery here since AC_CACHE_VAL can't handle a non-literal varname AC_MSG_CHECKING([if $CC supports ]flag[]) - AC_LINK_IFELSE([AC_LANG_PROGRAM([int i;])], - [supported=yes], [supported=no]) - AC_MSG_RESULT([$supported]) + cacheid=`AS_ECHO_N([xorg_cv_cc_flag_]flag[])` + AC_CACHE_VAL(AS_TR_SH($cacheid), + [AC_LINK_IFELSE([AC_LANG_PROGRAM([int i;])], + [eval AS_TR_SH($cacheid)=yes], + [eval AS_TR_SH($cacheid)=no])]) + CFLAGS="$xorg_testset_save_CFLAGS" + eval supported=$AS_TR_SH($cacheid) + AC_MSG_RESULT([$supported]) if test "$supported" = "yes" ; then $1="$$1 ]flag[" found="yes" -- 1.7.5.1
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
