[PATCH] Allow the user to specify CFLAGS without breaking the build

[Carl-Daniel Hailfinger]

GNU make has a very interesting quirk: If you set a variable on the
command line, any changes to that variable in the Makefile are ignored
unless marked with the "override" keyword.

Use CFLAGS only for optimization and warning options, and use CPPFLAGS
for the dependency and other preprocessor related options.
That way packagers can specify their own CFLAGS without breaking the build.

As a side benefit, the ich_descriptors_tool Makefile now behaves exactly
the same whether called standalone or as part of the main Makefile.

Note: THIS WILL BREAK THE BUILD due to problems in the
ich_descriptors_tool code.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-gnumake_workaround_cflags_ich_descriptors_tool/Makefile
===================================================================
--- flashrom-gnumake_workaround_cflags_ich_descriptors_tool/Makefile	(Revision 1569)
+++ flashrom-gnumake_workaround_cflags_ich_descriptors_tool/Makefile	(Arbeitskopie)
@@ -26,6 +26,9 @@
 # If you want to cross-compile, just run e.g.
 # make CC=i586-pc-msdosdjgpp-gcc
 # You may have to specify STRIP/AR/RANLIB as well.
+#
+# Note for anyone editing this Makefile: gnumake will happily ignore any
+# changes in this Makefile to variables set on the command line.
 CC      ?= gcc
 STRIP   ?= strip
 INSTALL = install
@@ -83,7 +86,7 @@
 EXEC_SUFFIX := .exe
 CPPFLAGS += -I../libgetopt
 # DJGPP has odd uint*_t definitions which cause lots of format string warnings.
-CPPFLAGS += -Wno-format
+CFLAGS += -Wno-format
 # FIXME Check if we can achieve the same effect with -L../libgetopt -lgetopt
 LIBS += ../libgetopt/libgetopt.a
 # Bus Pirate, Serprog and PonyProg are not supported under DOS (missing serial support).
@@ -119,7 +122,7 @@
 ifeq ($(TARGET_OS), MinGW)
 EXEC_SUFFIX := .exe
 # MinGW doesn't have the ffs() function, but we can use gcc's __builtin_ffs().
-CFLAGS += -Dffs=__builtin_ffs
+CPPFLAGS += -Dffs=__builtin_ffs
 # libusb-win32/libftdi stuff is usually installed in /usr/local.
 CPPFLAGS += -I/usr/local/include
 LDFLAGS += -L/usr/local/lib
Index: flashrom-gnumake_workaround_cflags_ich_descriptors_tool/util/ich_descriptors_tool/Makefile
===================================================================
--- flashrom-gnumake_workaround_cflags_ich_descriptors_tool/util/ich_descriptors_tool/Makefile	(Revision 1569)
+++ flashrom-gnumake_workaround_cflags_ich_descriptors_tool/util/ich_descriptors_tool/Makefile	(Arbeitskopie)
@@ -1,4 +1,8 @@
-CC ?= gcc
+#
+# This file is part of the flashrom project.
+#
+# This Makefile works standalone, but it is usually called from the main
+# Makefile in the flashrom directory.

PROGRAM=ich_descriptors_tool
 EXTRAINCDIRS = ../../ .
@@ -6,20 +10,31 @@
 OBJATH = .obj
 SHAREDSRC = ich_descriptors.c
 SHAREDSRCDIR = ../..
+# If your compiler spits out excessive warnings, run make WARNERROR=no
+# You shouldn't have to change this flag.
+WARNERROR ?= yes

SRC = $(wildcard *.c)

-CFLAGS += -Wall
-CFLAGS += -MMD -MP -MF $(DEPPATH)/$(@F).d
-# enables functions that populate the descriptor structs from plain binary dumps
-CFLAGS += -D ICH_DESCRIPTORS_FROM_DUMP
-CFLAGS += $(patsubst %,-I%,$(EXTRAINCDIRS))
+CC ?= gcc

+# If the user has specified custom CFLAGS, all CFLAGS settings below will be
+# completely ignored by gnumake.
+CFLAGS ?= -Os -Wall -Wshadow
 ifeq ($(TARGET_OS), DOS)
 # DJGPP has odd uint*_t definitions which cause lots of format string warnings.
 CFLAGS += -Wno-format
 endif
+ifeq ($(WARNERROR), yes)
+CFLAGS += -Werror
+endif

+
+CPPFLAGS += -MMD -MP -MF $(DEPPATH)/$(@F).d
+# enables functions that populate the descriptor structs from plain binary dumps
+CPPFLAGS += -D ICH_DESCRIPTORS_FROM_DUMP
+CPPFLAGS += $(patsubst %,-I%,$(EXTRAINCDIRS))
+
 OBJ = $(OBJATH)/$(SRC:%.c=%.o)

SHAREDOBJ = $(OBJATH)/$(notdir $(SHAREDSRC:%.c=%.o))
@@ -27,12 +42,12 @@
 all:$(PROGRAM)$(EXEC_SUFFIX)

$(OBJ): $(OBJATH)/%.o : %.c
-	$(CC) $(CFLAGS) -o $@ -c $<
+	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $<

# this enables us to share source files without simultaneously sharing .o files
 # with flashrom, which would lead to unexpected results (w/o running make clean)
 $(SHAREDOBJ): $(OBJATH)/%.o : $(SHAREDSRCDIR)/%.c
-	$(CC) $(CFLAGS) -o $@ -c $<
+	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $<

$(PROGRAM)$(EXEC_SUFFIX): $(OBJ) $(SHAREDOBJ)
    $(CC) -o $(PROGRAM)$(EXEC_SUFFIX) $(OBJ) $(SHAREDOBJ)

[PATCH] Allow the user to specify CFLAGS without breaking the build

[Stefan Tauner]

On Mon, 13 Aug 2012 04:03:04 +0200
Carl-Daniel Hailfinger  wrote:
> Note: THIS WILL BREAK THE BUILD due to problems in the
> ich_descriptors_tool code.

pah! no idea what you are talking about! ;)

is there a reason why we use CPPFLAGS instead of another name that is
less likely to be used in the future? why not leave the CPPFLAGS
variable alone and use a new one for the mandatory flags?
> Signed-off-by: Carl-Daniel Hailfinger

Acked-by: Stefan Tauner

[PATCH] Allow the user to specify CFLAGS without breaking the build

[Carl-Daniel Hailfinger]

Am 13.08.2012 11:09 schrieb Stefan Tauner:
> On Mon, 13 Aug 2012 04:03:04 +0200
> Carl-Daniel Hailfinger  wrote:
> > Note: THIS WILL BREAK THE BUILD due to problems in the
> > ich_descriptors_tool code.
>
> pah! no idea what you are talking about! ;)

Thanks for fixing the code!
> is there a reason why we use CPPFLAGS instead of another name that is
> less likely to be used in the future? why not leave the CPPFLAGS
> variable alone and use a new one for the mandatory flags?

CPPFLAGS is the standard for preprocessor flags, and this is pretty much
exactly what we're trying to do.
> > Signed-off-by: Carl-Daniel Hailfinger
>
> Acked-by: Stefan Tauner

Thanks, committed in r1574.

Regards,
Carl-Daniel