Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding default flags #700

Merged
merged 4 commits into from
Mar 20, 2020
Merged

Allow overriding default flags #700

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
613c34c
Remove test in configure.ac because it doesn't have an effect
jonasnick Dec 17, 2019
ecba813
Append instead of Prepend user-CFLAGS to default CFLAGS allowing the …
jonasnick Dec 17, 2019
83fb1bc
Remove -O2 from default CFLAGS because this would override the -O3 fl…
jonasnick Dec 17, 2019
ca739cb
Compile with optimization flag -O2 by default instead of -O3
jonasnick Feb 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions configure.ac
View file
Open in desktop
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"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we get rid of this entirely then because Autoconf's default is -g -O2?

If your answer is, maybe keep this variant still as a branch somewhere until we have further Concept ACKs on the idea of switching to -O2 :p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove this then if $CFLAGS aren't already set they will be set to -O2 ... -O2. The comment additionally serves as a hint to how autoconf works :) because I'd rather set : ${CFLAGS=""} in the future, because anything else doesn't make much sense to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay yes. Also, autoconf sets -O2 apparently only for gcc and not for other compilers according to its manual... So we need to keep the -O2 further below anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

83fb1bc

This causes multiple rm: conftest.dSYM: is a directory warnings on macOS (#896).

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 @@ -177,10 +178,10 @@ AC_CHECK_TYPES([__int128])

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 @@ -198,7 +199,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