Re: [PATCH 1/2] configure.ac: add new autoconf macro for checking valid compiler flags
Elia Pinto <gitter.spiros@gmail.com> writes:
> Add GIT_CC_CHECK_FLAG_APPEND, GIT_CC_CHECK_FLAGS_APPEND
> and GIT_CC_CHECK_LDFLAGS autoconf macro.
... which does what and for what purpose?
Don't explain it to me in your response, or tell me to read the patch text. I am speaking for those who have to read "git log" output down the line and need to get the big picture.
In the bigger picture, what is important is "why" than "how".
You add a handy way to give list of possible compilation flags and add as many of them as we can without triggering errors to CFLAGS and LDFLAGS. Why is that a good thing to do? What kind of compilation flags you are planning to throw at these macros? I think a mere mention of the latter will be sufficient to remind the reader why this change would be a good idea.
Show 62 quoted lines
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
> Makefile | 6 ++++--
> configure.ac | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 827006b..23485f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -344,10 +344,12 @@ GIT-VERSION-FILE: FORCE
> @$(SHELL_PATH) ./GIT-VERSION-GEN
> -include GIT-VERSION-FILE
>
> +GIT_CFLAGS =
> +GIT_LDFLAGS =
> # CFLAGS and LDFLAGS are for the users to override from the command line.
>
> -CFLAGS = -g -O2 -Wall
> -LDFLAGS =
> +CFLAGS = -g -O2 -Wall $(GIT_CFLAGS)
> +LDFLAGS = $(GIT_LDFLAGS)
> ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
> ALL_LDFLAGS = $(LDFLAGS)
> STRIP ?= strip
> diff --git a/configure.ac b/configure.ac
> index 6af9647..c67ca60 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -139,6 +139,51 @@ if test -n "$1"; then
> fi
> ])
>
> +
> +dnl Check if FLAG in ENV-VAR is supported by compiler and append it
> +dnl to WHERE-TO-APPEND variable
> +dnl GIT_CC_CHECK_FLAG_APPEND([WHERE-TO-APPEND], [ENV-VAR], [FLAG])
> +
> +AC_DEFUN([GIT_CC_CHECK_FLAG_APPEND], [
> + AC_CACHE_CHECK([if $CC supports flag $3 in envvar $2],
> + AS_TR_SH([cc_cv_$2_$3]),
> + [eval "AS_TR_SH([cc_save_$2])='${$2}'"
> + eval "AS_TR_SH([$2])='-Werror $3'"
> + AC_LINK_IFELSE([AC_LANG_SOURCE([int a = 0; int main(void) { return a; } ])],
> + [eval "AS_TR_SH([cc_cv_$2_$3])='yes'"],
> + [eval "AS_TR_SH([cc_cv_$2_$3])='no'"])
> + eval "AS_TR_SH([$2])='$cc_save_$2'"])
> +
> + AS_IF([eval test x$]AS_TR_SH([cc_cv_$2_$3])[ = xyes],
> + [eval "$1='${$1} $3'"])
> +])
> +
> +dnl GIT_CC_CHECK_FLAGS_APPEND([WHERE-TO-APPEND], [ENV-VAR], [FLAG1 FLAG2])
> +AC_DEFUN([GIT_CC_CHECK_FLAGS_APPEND], [
> + for flag in $3; do
> + GIT_CC_CHECK_FLAG_APPEND($1, $2, $flag)
> + done
> +])
> +
> +dnl Check if the flag is supported by linker (cacheable)
> +dnl GIT_CC_CHECK_LDFLAGS([FLAG], [ACTION-IF-FOUND],[ACTION-IF-NOT-FOUND])Missing SP after comma? I do not particularly mind commas without leading SP in m4 source, but please be consistent unless there is a compelling reason not to (and I do not think there is in this case).
Show 20 quoted lines
> +
> +AC_DEFUN([GIT_CC_CHECK_LDFLAGS], [
> + AC_CACHE_CHECK([if $CC supports $1 flag],
> + AS_TR_SH([cc_cv_ldflags_$1]),
> + [ac_save_LDFLAGS="$LDFLAGS"
> + LDFLAGS="$LDFLAGS $1"
> + AC_LINK_IFELSE([int main() { return 1; }],
> + [eval "AS_TR_SH([cc_cv_ldflags_$1])='yes'"],
> + [eval "AS_TR_SH([cc_cv_ldflags_$1])="])
> + LDFLAGS="$ac_save_LDFLAGS"
> + ])
> +
> + AS_IF([eval test x$]AS_TR_SH([cc_cv_ldflags_$1])[ = xyes],
> + [$2], [$3])
> +])
> +
> +
> ## Configure body starts here.
>
> AC_PREREQ(2.59)
Elia Pinto