[PATCH] Makefile: detect errors in running spatch

From: Jeff King <peff@peff.net>
Date: Mar 29, 2017, 07:10 UTC
Message-ID: <20170329071055.bp7d2gwqdhspz4aq@sigill.intra.peff.net>
In-Reply-To: <20170328194255.vf7nfzzmmzxsbn36@sigill.intra.peff.net>

The "make coccicheck" target runs spatch against each source file. But it does so in a for loop, so "make" never sees the exit code of spatch. Worse, it redirects stderr to a log file, so the user has no indication of any failure. And then to top it all off, because we touched the patch file's mtime, make will refuse to repeat the command because it think the target is up-to-date.

So for example:

  $ make coccicheck SPATCH=does-not-exist
      SPATCH contrib/coccinelle/free.cocci
      SPATCH contrib/coccinelle/qsort.cocci
      SPATCH contrib/coccinelle/xstrdup_or_null.cocci
      SPATCH contrib/coccinelle/swap.cocci
      SPATCH contrib/coccinelle/strbuf.cocci
      SPATCH contrib/coccinelle/object_id.cocci
      SPATCH contrib/coccinelle/array.cocci
  $ make coccicheck SPATCH=does-not-exist
  make: Nothing to be done for 'coccicheck'.

With this patch, you get:

  $ make coccicheck SPATCH=does-not-exist
       SPATCH contrib/coccinelle/free.cocci
  /bin/sh: 4: does-not-exist: not found
  Makefile:2338: recipe for target 'contrib/coccinelle/free.cocci.patch' failed
  make: *** [contrib/coccinelle/free.cocci.patch] Error 1

It also dumps the log on failure, so any errors from spatch itself (like syntax errors in our .cocci files) will be seen by the user.

Signed-off-by: Jeff King <peff@peff.net>
---
This is a verbatim repost of:

  http://public-inbox.org/git/20170310083117.cbflqx7zbe4s7cqv@sigill.intra.peff.net/

I think this is a strict improvement over the status quo. The conversation in that thread turned to possible refactorings, but since those didn't materialize, I think we should apply this in the meantime.

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

diff --git a/Makefile b/Makefile
index 9f8b35ad4..9b36068ac 100644
--- a/Makefile
+++ b/Makefile
@@ -2348,9 +2348,17 @@ check: common-cmds.h
 C_SOURCES = $(patsubst %.o,%.c,$(C_OBJ))
 %.cocci.patch: %.cocci $(C_SOURCES)
 	@echo '    ' SPATCH $<; \
+	ret=0; \
 	for f in $(C_SOURCES); do \
-		$(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS); \
-	done >$@ 2>$@.log; \
+		$(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
+			{ ret=$$?; break; }; \
+	done >$@+ 2>$@.log; \
+	if test $$ret != 0; \
+	then \
+		cat $@.log; \
+		exit 1; \
+	fi; \
+	mv $@+ $@; \
 	if test -s $@; \
 	then \
 		echo '    ' SPATCH result: $@; \

-- 
2.12.2.920.gc31091ce4