Re: [PATCH 2/2] Let package manager override CFLAGS and CPPFLAGS

Eric Biggers
2020-05-20T03:00:15Z

On Fri, May 15, 2020 at 04:56:49PM -0400, Jes Sorensen wrote:
> From: Jes Sorensen <jsorensen@fb.com>
>
> Package managers such as RPM wants to build everything with their
> preferred flags, and we shouldn't hard override flags.
>
> Signed-off-by: Jes Sorensen <jsorensen@fb.com>
> ---
> Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c5f46f4..0c2a621 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -32,7 +32,7 @@ cc-option = $(shell if $(CC) $(1) -c -x c /dev/null -o /dev/null &>/dev/null; \
> #### Common compiler flags. You can add additional flags by defining CFLAGS
> #### and/or CPPFLAGS in the environment or on the 'make' command line.

The above comment needs to be updated.

>
> -override CFLAGS := -O2 -Wall -Wundef \
> +CFLAGS := -O2 -Wall -Wundef \
> $(call cc-option,-Wdeclaration-after-statement) \
> $(call cc-option,-Wmissing-prototypes) \
> $(call cc-option,-Wstrict-prototypes) \
> @@ -40,7 +40,7 @@ override CFLAGS := -O2 -Wall -Wundef \
> $(call cc-option,-Wimplicit-fallthrough) \
> $(CFLAGS)

The user's $(CFLAGS) is already added at the end, so the -O2 can already be
overridden, e.g. with -O0. Is your concern just that this is bad practice?

Also, did you intentionally leave $(CFLAGS) at the end, rather than remove it as
might be expected?

> -override CPPFLAGS := -D_FILE_OFFSET_BITS=64 $(CPPFLAGS)
> +CPPFLAGS := -D_FILE_OFFSET_BITS=64 $(CPPFLAGS)

-D_FILE_OFFSET_BITS=64 is required for correctness.
So I think this part is good as-is.

- Eric