Pedantic fixes to build system (configure, Makefile)

[whitslack]

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

• The changelog has been updated in the relevant commit(s) according to the guidelines.
• Tests have been added or modified to reflect the changes. (N/A)
• Documentation has been reviewed and updated as needed. (N/A)
• Related issues have been listed and linked, including any that this PR closes. (N/A)
• Important All PRs must consider how to reverse any persistent changes for tools/lightning-downgrade (N/A)

[Andezion]

Isnt the old CFLAGS = $(CPPFLAGS) $(CWARNFLAGS) . . . included CPPFLAGS directly, and new CFLAGS := $(CWARNFLAGS) $(CDEBUGFLAGS) . . . $(CFLAGS) does not - CPPFLAGS now only reaches the compiler through $(COMPILE.c), which the pr only wires into the generic %.o: %.c pattern rule. But Makefile has some explicit rules like:

ccan-tal.o: $(CCANDIR)/ccan/tal/tal.c
    @$(call VERBOSE, "cc $<", $(CC) $(CFLAGS) -c -o $@ $<)

And these still call $(CC) $(CFLAGS) directly?, so they now lose everything that moved into CPPFLAGS, plus any macOS Homebrew -I paths. The worst case is ccan-tal.o, and ccan/tal/tal.c has:

static void *null_alloc_failed(void)
{
#ifdef CCAN_TAL_NEVER_RETURN_NULL
    abort();
#else
    return NULL;
#endif
}

Without -DCCAN_TAL_NEVER_RETURN_NULL=1, an out of memory tal_alloc now returns NULL instead of aborting? - while the rest of the codebase (compiled through the fixed pattern rule) is written on the assumption tal never returns NULL?

Maybe we need to keep CPPFLAGS folded into CFLAGS (drop the split), or update every explicit compile rule to use $(COMPILE.c) or at minimum $(CC) $(CPPFLAGS) $(CFLAGS) instead of $(CC) $(CFLAGS)?

[whitslack]

Thanks. I missed that. I'm on vacation now, but I'll do a thorough review for other direct invocations of $(CC) after I return (in a few days). The correct usage is $(COMPILE.c) when $(CC) is being invoked to compile a C source file to produce an object file. There really isn't a common scenario in which $(CC) should be used in a Make recipe except perhaps when querying/testing for compiler features. When asking the compiler to do something, the more specific variables should be used.

[whitslack]

I think I have now fixed all the explicit compile rules to use $(COMPILE.c) rather than $(CC). There are still some instances in the Makefiles in the ccan and jsmn submodules, but I think that those Makefiles are not actually used by the CLN build and so can be ignored.

[Andezion]

Should we add $CFLAGS to all of these invocations, or keep -std=gnu11 centralized in one flags variable that all of them already reference??

[whitslack]

Some build systems put the -std= option directly in CC (and CXX if applicable). I don't really like that, as some code assumes that CC and friends contain simple path names with no arguments. However, there isn't a great place to specify the -std= option. It needs to be passed to the preprocessor, as feature test macros are affected by the C/C++ standard in use, but naïvely adding the -std= option to CPPFLAGS is suboptimal because C++ code will need a different standard than C code. Some build systems work around this by introducing a CXXCPPFLAGS variable that is specific to the C++ preprocessor, but that's very non-standard. I usually make the compromise of locally overriding the value of CPPFLAGS just for C++ source files. This won't be an issue for CLN for the time being since it currently has no C++ source files.