Pedantic fixes to build system (configure, Makefile)

bitcoin/test/Makefile

@@ -9,6 +9,6 @@ ALL_TEST_PROGRAMS += $(BITCOIN_TEST_PROGRAMS)
 ALL_C_SOURCES += $(BITCOIN_TEST_PROGRAMS:=.c)
 
 # This needs to know what level of optimization we're using.
-bitcoin/test/run-secret_eq_consttime.o: CFLAGS += -DCOPTFLAGS="\"${COPTFLAGS}\""
+bitcoin/test/run-secret_eq_consttime.o: CPPFLAGS += -DCOPTFLAGS="\"${COPTFLAGS}\""
 
 check-units: $(BITCOIN_TEST_PROGRAMS:%=unittest/%)

common/test/run-close_tx.c

@@ -7,7 +7,7 @@
 #include <common/close_tx.h>
 #include <common/setup.h>
 #include <common/utils.h>
-#include <external/libwally-core/include/wally_script.h>
+#include <wally_script.h>
 #include <stdio.h>
 
 static void test_create_simple_close_tx_basic(void)

configure

@@ -27,14 +27,19 @@ if [ -z "$IN_NIX_SHELL" ]; then
     fi
 fi
 
-: ${PKG_CONFIG=pkg-config}
-
-# You can set PG_CONFIG in the environment to direct configure to call
-# a specific 'pg_config' binary. If you set it to an empty string, then
-# PostgreSQL support will be explicitly disabled, even if a 'pg_config'
-# binary exists in your PATH. If you leave it unset, then the following
-# line enables the automagic detection that most users want.
-: ${PG_CONFIG=pg_config}
+: "${PKG_CONFIG=pkg-config}"
+
+# You can set PG_CONFIG in the environment to direct configure to call a
+# specific 'pg_config' binary. If you set it to an empty string, then automagic
+# detection of PostgreSQL will be explicitly disabled even if a 'pg_config'
+# binary exists in your PATH. However, fallback detection will still happen.
+# You can disable PostgreSQL support entirely by setting POSTGRES_LDLIBS to an
+# empty string, or you can set POSTGRES_INCLUDE and POSTGRES_LDLIBS (which
+# specify options to pass to the C preprocessor and linker respectively) to
+# attempt to build against a specific PostgreSQL installation. If you leave
+# all three variables unset, then the following line enables the automagic
+# detection that most users want.
+: "${PG_CONFIG=pg_config}"
 
 usage_with_default()
 {
@@ -147,12 +152,12 @@ check_command()
     name="$1"
     shift 1
 
-    echo -n "checking for $name... "
+    printf 'checking for %s... ' "${name}" >&2
     if "$@" >/dev/null 2>&1 </dev/null; then
-	echo 'found'
+	echo 'found' >&2
 	return 0
     fi
-    echo 'not found'
+    echo 'not found' >&2
     return 1
 }
 
@@ -183,10 +188,17 @@ set_defaults()
     # which matters since you might explicitly set of these blank.
     PREFIX=${PREFIX:-/usr/local}
     CC=${CC:-cc}
+    # Always select our C dialect, even when the user sets CPPFLAGS (as many
+    # packaging tools do), unless they chose a -std= option themselves. Put it
+    # first so that a later -std= in CPPFLAGS still wins.
+    case " $CPPFLAGS " in
+	*" -std="*) ;;
+	*) CPPFLAGS="-std=gnu11${CPPFLAGS:+ $CPPFLAGS}";;
+    esac
             # Detect macOS and use appropriate debug flags for libbacktrace compatibility
     if [ "$(uname -s)" = "Darwin" ]; then
         # Always override to avoid DWARF 5
-        CDEBUGFLAGS="-std=gnu11 -g -gdwarf-4 -fno-standalone-debug -fstack-protector-strong"
+        CDEBUGFLAGS="-g -gdwarf-4 -fno-standalone-debug -fstack-protector-strong"
         # Set SDKROOT for macOS
         SDKROOT="$(xcrun --sdk macosx --show-sdk-path)"
 
@@ -195,7 +207,7 @@ set_defaults()
             echo "Warning: dsymutil not found. Install Xcode Command Line Tools for better debug support."
         fi
     else
-        CDEBUGFLAGS=${CDEBUGFLAGS--std=gnu11 -g -fstack-protector-strong}
+        CDEBUGFLAGS=${CDEBUGFLAGS--g -fstack-protector-strong}
     fi
     DEBUGBUILD=${DEBUGBUILD:-0}
     COMPAT=${COMPAT:-1}
@@ -236,7 +248,7 @@ have_function_sections()
     TMPCFILE=$CONFIG_VAR_FILE.$$.c
     TMPOBJFILE=$CONFIG_VAR_FILE.$$.o
 
-    echo "int foo(void); int foo(void) { return 0; }" > $TMPCFILE
+    echo "int main(void); int main(void) { return 0; }" > $TMPCFILE
     # We *want* this to fail if we get a warning, hence use -Werror.
     $1 $2 -Werror -ffunction-sections -Wl,--gc-sections $TMPCFILE -o $TMPOBJFILE
 }
@@ -249,11 +261,14 @@ usage()
     set_defaults
     DEFAULT_COPTFLAGS="$(default_coptflags $DEBUGBUILD)"
     # We assume we have a modern gcc.
-    DEFAULT_CWARNFLAGS="$(default_cwarnflags ""$DEFAULT_COPTFLAGS"" 1 1 ""$DEBUGBUILD"")"
+    DEFAULT_CWARNFLAGS="$(default_cwarnflags "$DEFAULT_COPTFLAGS" 1 1 "$DEBUGBUILD")"
     usage_with_default "CC" "$CC"
     usage_with_default "CWARNFLAGS" "$DEFAULT_CWARNFLAGS"
     usage_with_default "COPTFLAGS" "$DEFAULT_COPTFLAGS"
     usage_with_default "CDEBUGFLAGS" "$CDEBUGFLAGS"
+    usage_with_default "CFLAGS" "$CFLAGS"
+    usage_with_default "CPPFLAGS" "$CPPFLAGS"
+    usage_with_default "LDFLAGS" "$LDFLAGS"
     if [ "$(uname -s)" = "Darwin" ]; then
         echo "    Note: On macOS, -g is used instead of -g3 for libbacktrace compatibility"
     fi
@@ -290,9 +305,9 @@ usage()
 add_var()
 {
     if [ -n "$2" ]; then
-        echo "Setting $1... $2"
+        echo "Setting $1... $2" >&2
     else
-        echo "$1 not found"
+        echo "$1 not found" >&2
     fi
     echo "$1=$2" >> $CONFIG_VAR_FILE.$$
     [ -z "$3" ] || echo "#define $1 $2" >> "$3"
@@ -327,6 +342,9 @@ for opt in "$@"; do
 	CWARNFLAGS=*) CWARNFLAGS="${opt#CWARNFLAGS=}";;
 	CDEBUGFLAGS=*) CDEBUGFLAGS="${opt#CDEBUGFLAGS=}";;
 	COPTFLAGS=*) COPTFLAGS="${opt#COPTFLAGS=}";;
+	CFLAGS=*) CFLAGS="${opt#CFLAGS=}";;
+	CPPFLAGS=*) CPPFLAGS="${opt#CPPFLAGS=}";;
+	LDFLAGS=*) LDFLAGS="${opt#LDFLAGS=}";;
 	PYTEST=*) PYTEST="${opt#PYTEST=}";;
 	--prefix=*) PREFIX="${opt#--prefix=}";;
 	--enable-debugbuild) DEBUGBUILD=1;;
@@ -347,10 +365,10 @@ for opt in "$@"; do
 	--disable-fuzzing) FUZZING=0;;
 	--enable-rust) RUST=1;;
 	--disable-rust) RUST=0;;
-	--help|-h) usage;;
+	--help|-h) usage >&2;;
 	*)
 	    echo "Unknown option '$opt'" >&2
-	    usage
+	    usage >&2
 	    ;;
     esac
 done
@@ -366,7 +384,7 @@ fi
 
 if [ "$ASAN" = "1" ]; then
     if [ "$VALGRIND" = "1" ]; then
-	echo "Address sanitizer (ASAN) and valgrind cannot be enabled at the same time"
+	echo "Address sanitizer (ASAN) and valgrind cannot be enabled at the same time" >&2
 	exit 1
     fi
 
@@ -390,26 +408,28 @@ EOF
 fi
 
 # We call this first, so we can make sure configurator runs with it as a sanity check!
-if have_function_sections $CC "${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS"; then
-    HAVE_FUNCTION_SECTIONS=1
-    LDFLAGS="-Wl,--gc-sections"
-    COPTFLAGS="$COPTFLAGS -ffunction-sections"
-else
-    HAVE_FUNCTION_SECTIONS=0
-    LDFLAGS=
+if have_function_sections $CC "${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS $CFLAGS $CPPFLAGS $LDFLAGS"; then
+    case "$LDFLAGS" in
+	*'--gc-sections'*) ;;
+	*) LDFLAGS="-Wl,--gc-sections $LDFLAGS";;
+    esac
+    case " $COPTFLAGS " in
+	*' -ffunction-sections '*) ;;
+	*) COPTFLAGS="$COPTFLAGS -ffunction-sections";;
+    esac
 fi
 
 # We assume warning flags don't affect congfigurator that much!
-echo -n "Compiling $CONFIGURATOR..."
-$CC ${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS $LDFLAGS -o $CONFIGURATOR $CONFIGURATOR.c
-echo "done"
+printf 'Compiling %s...' "${CONFIGURATOR}" >&2
+$CC ${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS $CFLAGS $CPPFLAGS $LDFLAGS -o $CONFIGURATOR $CONFIGURATOR.c
+echo "done" >&2
 
 if [ "$CLANG_COVERAGE" = "1" ]; then
 	case "$CC" in
 		(*"clang"*)
 			;;
 		(*)
-			echo "Clang coverage requires building with CC=clang."
+			echo "Clang coverage requires building with CC=clang." >&2
 			exit 1
 			;;
 	esac
@@ -420,32 +440,44 @@ if [ "$FUZZING" = "1" ]; then
 		(*"clang"*)
 			;;
 		(*)
-			echo "Fuzzing is currently only supported with clang."
+			echo "Fuzzing is currently only supported with clang." >&2
 			exit 1
 			;;
 	esac
 fi
 
 
-SQLITE3_CFLAGS=""
-SQLITE3_LDLIBS="-lsqlite3"
-if command -v "${PKG_CONFIG}" >/dev/null; then
-    SQLITE3_CFLAGS="$("${PKG_CONFIG}" --silence-errors --cflags sqlite3 || :)"
-    SQLITE3_LDLIBS="$("${PKG_CONFIG}" --silence-errors --libs sqlite3 || :)"
+# Setting SQLITE3_CFLAGS or SQLITE3_LDLIBS in the environment (even to an empty
+# string) disables automagic detection via pkg-config as well as the fallback
+# default values.
+if [ -z "${SQLITE3_CFLAGS+x}" ] && [ -z "${SQLITE3_LDLIBS+x}" ]; then
+    SQLITE3_LDLIBS="-lsqlite3"
+    if command -v "${PKG_CONFIG}" >/dev/null; then
+	SQLITE3_CFLAGS="$("${PKG_CONFIG}" --silence-errors --cflags sqlite3 || :)"
+	SQLITE3_LDLIBS="$("${PKG_CONFIG}" --silence-errors --libs sqlite3 || :)"
+    fi
 fi
 
-SODIUM_CFLAGS=""
-SODIUM_LDLIBS="-lsodium"
-if command -v "${PKG_CONFIG}" >/dev/null; then
-    SODIUM_CFLAGS="$("${PKG_CONFIG}" --silence-errors --cflags libsodium || :)"
-    SODIUM_LDLIBS="$("${PKG_CONFIG}" --silence-errors --libs libsodium || :)"
+# Setting SODIUM_CFLAGS or SODIUM_LDLIBS in the environment (even to an empty
+# string) disables automagic detection via pkg-config as well as the fallback
+# default values.
+if [ -z "${SODIUM_CFLAGS+x}" ] && [ -z "${SODIUM_LDLIBS+x}" ]; then
+    SODIUM_LDLIBS="-lsodium"
+    if command -v "${PKG_CONFIG}" >/dev/null; then
+	SODIUM_CFLAGS="$("${PKG_CONFIG}" --silence-errors --cflags libsodium || :)"
+	SODIUM_LDLIBS="$("${PKG_CONFIG}" --silence-errors --libs libsodium || :)"
+    fi
 fi
 
-POSTGRES_INCLUDE=""
-POSTGRES_LDLIBS=""
-if command -v "${PG_CONFIG}" >/dev/null; then
-    POSTGRES_INCLUDE="-I$("${PG_CONFIG}" --includedir)"
-    POSTGRES_LDLIBS="-L$("${PG_CONFIG}" --libdir) -lpq"
+# Setting POSTGRES_INCLUDE or POSTGRES_LDLIBS in the environment (even to an
+# empty string) disables automagic detection via pg_config as well as the
+# fallback default values.
+if [ -z "${POSTGRES_INCLUDE+x}" ] && [ -z "${POSTGRES_LDLIBS+x}" ]; then
+    POSTGRES_LDLIBS="-lpq"
+    if command -v "${PG_CONFIG}" >/dev/null; then
+	POSTGRES_INCLUDE="-I$("${PG_CONFIG}" --includedir)"
+	POSTGRES_LDLIBS="-L$("${PG_CONFIG}" --libdir) -lpq"
+    fi
 fi
 
 # Clean up on exit.
@@ -455,7 +487,7 @@ CONFIGURATOR_PATH_FLAGS=
 [ -z "$CPATH" ] || CONFIGURATOR_PATH_FLAGS="$CONFIGURATOR_PATH_FLAGS -I$CPATH"
 [ -z "$LIBRARY_PATH" ] || CONFIGURATOR_PATH_FLAGS="$CONFIGURATOR_PATH_FLAGS -L$LIBRARY_PATH"
 
-$CONFIGURATOR --extra-tests --autotools-style --var-file=$CONFIG_VAR_FILE.$$ --header-file=$CONFIG_HEADER.$$ --configurator-cc="$CONFIGURATOR_CC" --wrapper="$CONFIGURATOR_WRAPPER" "$CC" ${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS $CSANFLAGS $CONFIGURATOR_PATH_FLAGS $SQLITE3_CFLAGS $SODIUM_CFLAGS $POSTGRES_INCLUDE <<EOF
+$CONFIGURATOR --extra-tests --autotools-style --var-file=$CONFIG_VAR_FILE.$$ --header-file=$CONFIG_HEADER.$$ --configurator-cc="$CONFIGURATOR_CC" --wrapper="$CONFIGURATOR_WRAPPER" "$CC" ${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS $CSANFLAGS $SQLITE3_CFLAGS $SODIUM_CFLAGS $CFLAGS $CONFIGURATOR_PATH_FLAGS $POSTGRES_INCLUDE $CPPFLAGS $LDFLAGS <<EOF
 
 var=HAVE_ZLIB
 desc=zlib support
@@ -633,6 +665,9 @@ add_var CONFIGURATOR_CC "$CONFIGURATOR_CC"
 add_var CWARNFLAGS "$CWARNFLAGS"
 add_var CDEBUGFLAGS "$CDEBUGFLAGS"
 add_var COPTFLAGS "$COPTFLAGS"
+add_var CFLAGS "$CFLAGS"
+add_var CPPFLAGS "$CPPFLAGS"
+add_var LDFLAGS "$LDFLAGS"
 if [ -n "${SDKROOT:-}" ]; then
     add_var SDKROOT "$SDKROOT"
 fi
@@ -660,7 +695,6 @@ add_var FUZZING "$FUZZING"
 add_var RUST "$RUST"
 add_var PYTHON "$PYTHON"
 add_var SED "$SED"
-add_var HAVE_FUNCTION_SECTIONS "$HAVE_FUNCTION_SECTIONS"
 
 # Hack to avoid sha256 name clash with libwally: will be fixed when that
 # becomes a standalone shared lib.

lightningd/test/run-close_tx_check.c

@@ -2,7 +2,7 @@
 #include "../simple_close_control.c"
 #include <common/close_tx.h>
 #include <common/setup.h>
-#include <external/libwally-core/include/wally_script.h>
+#include <wally_script.h>
 #include <stdio.h>
 
 /* AUTOGENERATED MOCKS START */

tools/Makefile

@@ -11,13 +11,13 @@ ALL_PROGRAMS += $(TOOLS)
 # We force make to relink this every time, to detect version changes.
 # Do it atomically, otherwise parallel builds can get upset!
 tools/headerversions: $(FORCE) tools/headerversions.o libccan.a
-	@trap "rm -f $@.tmp.$$$$" EXIT; $(LINK.o) tools/headerversions.o libccan.a $(LOADLIBES) $(LDLIBS) -o $@.tmp.$$$$ && mv -f $@.tmp.$$$$ $@
+	@trap "rm -f $@.tmp.$$$$" EXIT; $(LINK.c) tools/headerversions.o libccan.a $(LOADLIBES) $(LDLIBS) -o $@.tmp.$$$$ && mv -f $@.tmp.$$$$ $@
 
 $(TOOLS): libcommon.a
 
 tools/headerversions.o: ccan/config.h
 tools/lightning-hsmtool: tools/lightning-hsmtool.o
-tools/lightning-downgrade.o: CFLAGS:=$(CFLAGS) -DCLN_PREV_VERSION=$(CLN_PREV_VERSION)
+tools/lightning-downgrade.o: CPPFLAGS += -DCLN_PREV_VERSION=$(CLN_PREV_VERSION)
 
 tools/lightning-downgrade:			\
 	db/exec.o				\

tools/update-mocks.sh

@@ -13,8 +13,12 @@ START=$(grep -F -n '/* AUTOGENERATED MOCKS START */' "$FILE" | cut -d: -f1)
 END=$(grep -F -n '/* AUTOGENERATED MOCKS END */' "$FILE" | cut -d: -f1)
 
 function make_binary() {
+    local LDFLAGS
+    # Do not combine the following assignment with the preceding declaration, as that breaks the error logic.
+    LDFLAGS="$(sed -Ene 's/^LDFLAGS=(.*)$/\1/p' config.vars)" || exit
+    LDFLAGS+="${LDFLAGS:+ }-Wl,--no-gc-sections${*:+ $*}"
     # Make sure we don't optimize out options, and we use debug build
-    $MAKE SUPPRESS_GENERATION=1 "${FILE/%.c/}" DEBUGBUILD=1 COPTFLAGS="" HAVE_FUNCTION_SECTIONS=0 2> "${BASE}.err" >/dev/null
+    $MAKE SUPPRESS_GENERATION=1 "${FILE/%.c/}" DEBUGBUILD=1 COPTFLAGS= LDFLAGS="${LDFLAGS}" 2>"${BASE}.err" >/dev/null
 }
 
 if [ -n "$START" ]; then
@@ -29,7 +33,7 @@ if [ -n "$START" ]; then
 	# this in the output:
 	# ld: error: too many errors emitted, stopping now (use -error-limit=0 to see all errors)
 	if grep -q 'too many errors emitted.*-error-limit=0' "${BASE}.err"; then
-	    LDFLAGS=-Wl,-error-limit=0 make_binary || :
+	    make_binary -Wl,-error-limit=0 || :
 	fi
 	tools/mockup.sh "$FILE" < "${BASE}.err" >> "${BASE}.stubs"
 	# If there are no link errors, maybe compile fail for other reason?