Skip to content

Commit

Permalink
Merge dashpay#700: Allow overriding default flags
Browse files Browse the repository at this point in the history
ca739cb Compile with optimization flag -O2 by default instead of -O3 (Jonas Nick)
83fb1bc Remove -O2 from default CFLAGS because this would override the -O3 flag (see AC_PROG_CC in the Autoconf manual) (Jonas Nick)
ecba813 Append instead of Prepend user-CFLAGS to default CFLAGS allowing the user to override default variables (Jonas Nick)
613c34c Remove test in configure.ac because it doesn't have an effect (Jonas Nick)

Pull request description:

  Right now, it's not easy to reduce the optimization level with `CFLAGS` because `configure` overwrites any optimization flag with `-O3`. The [automake documentation](https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html) states that:

   > The reason ‘$(CPPFLAGS)’ appears after ‘$(AM_CPPFLAGS)’ or ‘$(mumble_CPPFLAGS)’ in the compile command is that users should always have the last say.

  and also that it's incorrect to redefine CFLAGS in the first place

  > You should never redefine a user variable such as CPPFLAGS in Makefile.am. [...] You should not add options to these user variables within configure either, for the same reason

  With this PR `CFLAGS` is still redefined, but user-provided flags appear after the default `CFLAGS` which means that they override the default flags (at least in clang and gcc). Otherwise, the default configuration is not changed. This also means that if CFLAGS are defined by the user, then -g is not added (which does not seem to make much sense). In order to keep the `-O3` despite the reordering we need to explicitly tell autoconf to not append `-O2` by setting the default to `-g` with `: ${CFLAGS="-g"}` as per [the manual](https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/autoconf.html#C-Compiler) (EDIT: link fix).

ACKs for top commit:
  real-or-random:
    ACK ca739cb
  theuni:
    ACK ca739cb.
  elichai:
    ACK ca739cb

Tree-SHA512: be92589faa461d245203385d44b489c7d6917b0c68472b8d7576806c0250cf5ff61d5c99ce04eebb8ff5279b9987185d4e5d2da979683fb1c489fdf3e5b59630
  • Loading branch information
real-or-random committed Mar 20, 2020
2 parents e9fccd4 + ca739cb commit ed1b911
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ AH_TOP([#ifndef LIBSECP256K1_CONFIG_H])
AH_TOP([#define LIBSECP256K1_CONFIG_H])
AH_BOTTOM([#endif /*LIBSECP256K1_CONFIG_H*/])
AM_INIT_AUTOMAKE([foreign subdir-objects])

# Set -g if CFLAGS are not already set, which matches the default autoconf
# behavior (see PROG_CC in the Autoconf manual) with the exception that we don't
# set -O2 here because we set it in any case (see further down).
: ${CFLAGS="-g"}
LT_INIT

dnl make the compilation flags quiet unless V=1 is used
Expand All @@ -19,10 +24,6 @@ AC_PATH_TOOL(RANLIB, ranlib)
AC_PATH_TOOL(STRIP, strip)
AX_PROG_CC_FOR_BUILD

if test "x$CFLAGS" = "x"; then
CFLAGS="-g"
fi

AM_PROG_CC_C_O

AC_PROG_CC_C89
Expand Down Expand Up @@ -63,11 +64,11 @@ case $host_os in
;;
esac

CFLAGS="$CFLAGS -W"
CFLAGS="-W $CFLAGS"

warn_CFLAGS="-std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings"
saved_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS $warn_CFLAGS"
CFLAGS="$warn_CFLAGS $CFLAGS"
AC_MSG_CHECKING([if ${CC} supports ${warn_CFLAGS}])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
Expand All @@ -76,7 +77,7 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
])

saved_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS -fvisibility=hidden"
CFLAGS="-fvisibility=hidden $CFLAGS"
AC_MSG_CHECKING([if ${CC} supports -fvisibility=hidden])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
Expand Down Expand Up @@ -175,10 +176,10 @@ AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"])

if test x"$enable_coverage" = x"yes"; then
AC_DEFINE(COVERAGE, 1, [Define this symbol to compile out all VERIFY code])
CFLAGS="$CFLAGS -O0 --coverage"
LDFLAGS="$LDFLAGS --coverage"
CFLAGS="-O0 --coverage $CFLAGS"
LDFLAGS="--coverage $LDFLAGS"
else
CFLAGS="$CFLAGS -O3"
CFLAGS="-O2 $CFLAGS"
fi

if test x"$use_ecmult_static_precomputation" != x"no"; then
Expand All @@ -196,7 +197,7 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then

warn_CFLAGS_FOR_BUILD="-Wall -Wextra -Wno-unused-function"
saved_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS $warn_CFLAGS_FOR_BUILD"
CFLAGS="$warn_CFLAGS_FOR_BUILD $CFLAGS"
AC_MSG_CHECKING([if native ${CC_FOR_BUILD} supports ${warn_CFLAGS_FOR_BUILD}])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
Expand Down

0 comments on commit ed1b911

Please sign in to comment.