From: Junio C Hamano <gitster@pobox•com>
To: Elia Pinto <gitter.spiros@gmail•com>
Cc: git@vger•kernel.org, jnareb@gmail•com
Subject: Re: [PATCH 1/2] configure.ac: add new autoconf macro for checking valid compiler flags
Date: Mon, 03 Nov 2014 11:39:44 -0800 [thread overview]
Message-ID: <xmqq61ewrs3j.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1415026641-24767-2-git-send-email-gitter.spiros@gmail.com> (Elia Pinto's message of "Mon, 3 Nov 2014 06:57:20 -0800")
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.
>
> 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).
> +
> +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)
next prev parent reply other threads:[~2014-11-03 19:39 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-03 14:57 [PATCH 0/2] add some new autoconf macros for searching the possible warning flags allowed by the current version of the gcc compiler Elia Pinto
2014-11-03 14:57 ` [PATCH 1/2] configure.ac: add new autoconf macro for checking valid compiler flags Elia Pinto
2014-11-03 19:39 ` Junio C Hamano [this message]
2014-11-03 14:57 ` [PATCH 2/2] configure.ac: use GIT_CC_CHECK_FLAG_APPEND for adding --with-gcc-warnings configure option Elia Pinto
2014-11-03 19:49 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq61ewrs3j.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitter.spiros@gmail$(echo .)com \
--cc=jnareb@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox