git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@imag.fr>
To: git@vger.kernel.org
Cc: Lars Schneider <larsxschneider@gmail.com>,
	Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: [RCF/PATCH] Makefile: move 'ifdef DEVELOPER' after config.mak* inclusion
Date: Tue, 31 May 2016 15:24:43 +0200	[thread overview]
Message-ID: <20160531132443.5033-1-Matthieu.Moy@imag.fr> (raw)

The DEVELOPER knob was introduced in 658df95 (add DEVELOPER makefile
knob to check for acknowledged warnings, 2016-02-25), and works well
when used as "make DEVELOPER=1", and when the configure script was not
used.

However, the advice given in CodingGuidelines to add DEVELOPER=1 to
config.mak does not: config.mak is included after testing for
DEVELOPER in the Makefile, and at least GNU Make's manual specifies
"Conditional directives are parsed immediately", hence the config.mak
declaration is not visible at the time the conditional is evaluated.

Also, when using the configure script to generate a
config.mak.autogen, the later file contained a "CFLAGS = <flags>"
initialization, which overrode the "CFLAGS += -W..." triggered by
DEVELOPER.

This patch fixes both issues.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
I'm surprised that no one noticed the issue. Probably because the
Makefile is silent by default. Did I miss anything obvious?

 Makefile | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 15fcd57..2226319 100644
--- a/Makefile
+++ b/Makefile
@@ -380,18 +380,6 @@ ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 STRIP ?= strip
 
-ifdef DEVELOPER
-CFLAGS += -Werror \
-	-Wdeclaration-after-statement \
-	-Wno-format-zero-length \
-	-Wold-style-definition \
-	-Woverflow \
-	-Wpointer-arith \
-	-Wstrict-prototypes \
-	-Wunused \
-	-Wvla
-endif
-
 # Create as necessary, replace existing, make ranlib unneeded.
 ARFLAGS = rcs
 
@@ -952,6 +940,18 @@ include config.mak.uname
 -include config.mak.autogen
 -include config.mak
 
+ifdef DEVELOPER
+CFLAGS += -Werror \
+	-Wdeclaration-after-statement \
+	-Wno-format-zero-length \
+	-Wold-style-definition \
+	-Woverflow \
+	-Wpointer-arith \
+	-Wstrict-prototypes \
+	-Wunused \
+	-Wvla
+endif
+
 ifndef sysconfdir
 ifeq ($(prefix),/usr)
 sysconfdir = /etc
-- 
2.8.2.397.gbe91ebf.dirty

             reply	other threads:[~2016-05-31 13:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-31 13:24 Matthieu Moy [this message]
2016-05-31 17:00 ` [RCF/PATCH] Makefile: move 'ifdef DEVELOPER' after config.mak* inclusion Junio C Hamano
2016-06-01  7:30 ` Jeff King
2016-06-01  7:57   ` Matthieu Moy
2016-06-01  8:00     ` [PATCH] Makefile: add $(DEVELOPER_CFLAGS) variable Matthieu Moy
2016-06-01  8:03     ` [RCF/PATCH] Makefile: move 'ifdef DEVELOPER' after config.mak* inclusion Jeff King
2016-06-01  8:05       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160531132443.5033-1-Matthieu.Moy@imag.fr \
    --to=matthieu.moy@imag.fr \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).