Re: [PATCH] Makefile: detect errors in running spatch

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 10, 2017, 20:16 UTC
Message-ID: <xmqqbmt8yjii.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170310083117.cbflqx7zbe4s7cqv@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> It also doesn't help that shells are awkward at passing status out of a
> for-loop. I think the most "make-ish" way of doing this would actually
> be to lose the for loop and have a per-cocci-per-source target.

As we assume we can freely use GNUmake facilities, another option, (i.e. the most "gnumake-ish" way) may be to have it unroll the loop with $(foreach,...) so that the shell just sees a series of commands.

Show 33 quoted lines

> I don't know if that would make the patches harder to apply. The results
> aren't full patches, so I assume you usually do some kind of munging on
> them? I resorted to:
>
>   make coccicheck SPATCH='spatch --in-place'
>
>  Makefile | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 9ec6065cc..d97633892 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2336,9 +2336,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: $@; \