Re: [RFC PATCH 1/2] Makefile: add check-headers target

[David Aguilar <davvid@gmail.com>]

On Sat, Sep 06, 2014 at 11:20:32PM +0200, René Scharfe wrote:
> Am 06.09.2014 um 21:20 schrieb David Aguilar:
> >This allows us to ensure that each header can be included
> >individually without needing to include other headers first.
> 
> Sounds like a good objective.
> 
> >Signed-off-by: David Aguilar <davvid@gmail.com>
> >---
> >This patch demonstrates how to verify PATCH 2/2.
> >
> >  Makefile         |  6 ++++++
> >  check-headers.sh | 26 ++++++++++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> >  create mode 100755 check-headers.sh
> >
> >diff --git a/Makefile b/Makefile
> >index 30cc622..bc54024 100644
> >--- a/Makefile
> >+++ b/Makefile
> >@@ -2591,6 +2591,12 @@ check-docs::
> >  check-builtins::
> >  	./check-builtins.sh
> >
> >+### Make sure headers include their dependencies
> >+#
> >+check-headers::
> >+	./check-headers.sh $(CC) $(ALL_CFLAGS)
> >+
> >+
> >  ### Test suite coverage testing
> >  #
> >  .PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report
> >diff --git a/check-headers.sh b/check-headers.sh
> >new file mode 100755
> >index 0000000..bf85c41
> >--- /dev/null
> >+++ b/check-headers.sh
> >@@ -0,0 +1,26 @@
> >+#!/bin/sh
> >+
> >+exit_code=0
> >+
> >+maybe_exit () {
> >+	status="$1"
> >+	if test "$status" != 0
> >+	then
> >+		exit_code="$status"
> >+		if test -n "$CHECK_HEADERS_STOP"
> >+		then
> >+			exit "$status"
> >+		fi
> >+	fi
> >+}
> >+
> >+git ls-files *.h |
> 
> This checks all .h files in the top directory.  Would it be better
> to check all files in LIB_H instead?  Or even all .h files in the
> tree (using "git ls-files '*.h'")?  The latter might be difficult
> because some of the files in compat/ #include system-specific
> headers.

Ah, I hadn't thought of using LIB_H; that might be the most
practical solution.

Changing it to quoted '*.h' finds compat/ files which seemed
like too much trouble.  OTOH blindly using all *.h files (i.e.
not using git ls-files) picks up the generated common-cmds.h
which would require changing generate-cmdlist.h, and that seemed
like it might be out of scope for this patch.

I'd certainly be interested in improving this part, so I'll see if
we can reformulate this patch to use LIB_H.

I'll also fixup the unneeded strbuf.h addition in the reroll.

> 
> >+while read header
> >+do
> >+	echo "HEADER $header" &&
> >+	"$@" -Wno-unused -x c -c -o "$header".bin - <"$header" &&
> >+	rm "$header".bin ||
> >+	maybe_exit $?
> >+done
> >+
> >+exit $exit_code
> >
> 

-- 
David