On Mon, 2020-10-26 at 15:10 -0700, Eric Biggers wrote: > [+Jes Sorensen] > > On Mon, Oct 26, 2020 at 08:48:31PM +0000, luca.boccassi@xxxxxxxxx wrote: > > From: Romain Perier <romain.perier@xxxxxxxxx> > > > > Currently, CFLAGS are defined by default. It has to effect to define its > > c-compiler options only when the variable is not defined on the cmdline > > by the user, it is not possible to merge or mix both, while it could > > be interesting for using the app warning cflags or the pkg-config > > cflags, while using the distributor flags. Most distributions packages > > use their own compilation flags, typically for hardening purpose but not > > only. This fixes the issue by using the override keyword. > > > > Signed-off-by: Romain Perier <romain.perier@xxxxxxxxx> > > --- > > Currently used in Debian, were we want to append context-specific > > compiler flags (eg: for compiler hardening options) without > > removing the default flags > > > > Makefile | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 6c6c8c9..5020cac 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -35,14 +35,15 @@ > > cc-option = $(shell if $(CC) $(1) -c -x c /dev/null -o /dev/null > /dev/null 2>&1; \ > > then echo $(1); fi) > > > > -CFLAGS ?= -O2 -Wall -Wundef \ > > +override CFLAGS := -O2 -Wall -Wundef \ > > $(call cc-option,-Wdeclaration-after-statement) \ > > $(call cc-option,-Wimplicit-fallthrough) \ > > $(call cc-option,-Wmissing-field-initializers) \ > > $(call cc-option,-Wmissing-prototypes) \ > > $(call cc-option,-Wstrict-prototypes) \ > > $(call cc-option,-Wunused-parameter) \ > > - $(call cc-option,-Wvla) > > + $(call cc-option,-Wvla) \ > > + $(CFLAGS) > > > > override CPPFLAGS := -Iinclude -D_FILE_OFFSET_BITS=64 $(CPPFLAGS) > > I had it like this originally, but Jes requested that it be changed to the > current way for rpm packaging. See the thread: > https://lkml.kernel.org/linux-fscrypt/20200515205649.1670512-3-Jes.Sorensen@xxxxxxxxx/T/#u > > Can we come to an agreement on one way to do it? > > To me, the approach with 'override' makes more sense. The only non-warning > option is -O2, and if someone doesn't want that, they can just specify > CFLAGS=-O0 and it will override -O2 (since the last option "wins"). > > Jes, can you explain why that way doesn't work with rpm? I see - I'm pretty sure what we want to override is the optimization flag (and any other flag that affect the binary, eg: debugging symbols), but not the other flags that you set (eg: warnings). So, how about this v2: >From b48d09b1868cfa50b2cd61eec2951f722943e421 Mon Sep 17 00:00:00 2001 From: Romain Perier <romain.perier@xxxxxxxxx> Date: Sun, 19 Apr 2020 09:24:09 +0200 Subject: [PATCH] override CFLAGS too Currently, CFLAGS are defined by default. It has to effect to define its c-compiler options only when the variable is not defined on the cmdline by the user, it is not possible to merge or mix both, while it could be interesting for using the app warning cflags or the pkg-config cflags, while using the distributor flags. Most distributions packages use their own compilation flags, typically for hardening purpose but not only. This fixes the issue by using the override keyword. Signed-off-by: Romain Perier <romain.perier@xxxxxxxxx> --- Makefile | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 6c6c8c9..82abfe9 100644 --- a/Makefile +++ b/Makefile @@ -35,14 +35,19 @@ cc-option = $(shell if $(CC) $(1) -c -x c /dev/null -o /dev/null > /dev/null 2>&1; \ then echo $(1); fi) -CFLAGS ?= -O2 -Wall -Wundef \ +# Flags that callers can override +CFLAGS ?= -O2 + +# Flags that we always want to use +INTERNAL_CFLAGS := -Wall -Wundef \ $(call cc-option,-Wdeclaration-after-statement) \ $(call cc-option,-Wimplicit-fallthrough) \ $(call cc-option,-Wmissing-field-initializers) \ $(call cc-option,-Wmissing-prototypes) \ $(call cc-option,-Wstrict-prototypes) \ $(call cc-option,-Wunused-parameter) \ - $(call cc-option,-Wvla) + $(call cc-option,-Wvla) \ + $(CFLAGS) override CPPFLAGS := -Iinclude -D_FILE_OFFSET_BITS=64 $(CPPFLAGS) @@ -65,7 +70,7 @@ PKGCONF ?= pkg-config .build-config: FORCE @flags=$$( \ echo 'CC=$(CC)'; \ - echo 'CFLAGS=$(CFLAGS)'; \ + echo 'CFLAGS=$(INTERNAL_CFLAGS)'; \ echo 'CPPFLAGS=$(CPPFLAGS)'; \ echo 'LDFLAGS=$(LDFLAGS)'; \ echo 'LDLIBS=$(LDLIBS)'; \ @@ -98,7 +103,7 @@ endif #### Library SOVERSION := 0 -LIB_CFLAGS := $(CFLAGS) -fvisibility=hidden +LIB_CFLAGS := $(INTERNAL_CFLAGS) -fvisibility=hidden LIB_SRC := $(wildcard lib/*.c) LIB_HEADERS := $(wildcard lib/*.h) $(COMMON_HEADERS) STATIC_LIB_OBJ := $(LIB_SRC:.c=.o) @@ -121,7 +126,7 @@ DEFAULT_TARGETS += libfsverity.a # Create shared library libfsverity.so.$(SOVERSION):$(SHARED_LIB_OBJ) $(QUIET_CCLD) $(CC) -o $@ -Wl,-soname=$@ -shared $+ \ - $(CFLAGS) $(LDFLAGS) $(LDLIBS) + $(INTERNAL_CFLAGS) $(LDFLAGS) $(LDLIBS) DEFAULT_TARGETS += libfsverity.so.$(SOVERSION) @@ -160,23 +165,23 @@ TEST_PROGRAMS := $(TEST_PROG_SRC:programs/%.c=%) # Compile program object files $(ALL_PROG_OBJ): %.o: %.c $(ALL_PROG_HEADERS) .build-config - $(QUIET_CC) $(CC) -o $@ -c $(CPPFLAGS) $(CFLAGS) $< + $(QUIET_CC) $(CC) -o $@ -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< # Link the fsverity program ifdef USE_SHARED_LIB fsverity: $(FSVERITY_PROG_OBJ) libfsverity.so $(QUIET_CCLD) $(CC) -o $@ $(FSVERITY_PROG_OBJ) \ - $(CFLAGS) $(LDFLAGS) -L. -lfsverity + $(INTERNAL_CFLAGS) $(LDFLAGS) -L. -lfsverity else fsverity: $(FSVERITY_PROG_OBJ) libfsverity.a - $(QUIET_CCLD) $(CC) -o $@ $+ $(CFLAGS) $(LDFLAGS) $(LDLIBS) + $(QUIET_CCLD) $(CC) -o $@ $+ $(INTERNAL_CFLAGS) $(LDFLAGS) $(LDLIBS) endif DEFAULT_TARGETS += fsverity # Link the test programs $(TEST_PROGRAMS): %: programs/%.o $(PROG_COMMON_OBJ) libfsverity.a - $(QUIET_CCLD) $(CC) -o $@ $+ $(CFLAGS) $(LDFLAGS) $(LDLIBS) + $(QUIET_CCLD) $(CC) -o $@ $+ $(INTERNAL_CFLAGS) $(LDFLAGS) $(LDLIBS) ############################################################################## -- 2.20.1
|