Re: [PATCHv2] Makefile: add missing phony target

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Dec 11, 2015, 14:40 UTC
Message-ID: <vpq7fkl3v66.fsf@anie.imag.fr>
In-Reply-To: <1449843766-41658-1-git-send-email-gitter.spiros@gmail.com>

Elia Pinto <gitter.spiros@gmail.com> writes:

> This is the second version of this patch.
> Added the corrections suggested by Matthieu Moy ($gmane/282221)

Sorry, but my main concern was that the patch could not be reviewed in good conditions as-is, and I think it still cannot be. It's very hard to spot which .PHONY rules you're adding and which are just code movement. You should really split this into one "code movement" patch and one "actual bugfix" patch. Or someone with better eyes than me should review the patch ;-).

Show 14 quoted lines

> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a
>  test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
>  
> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1
>  check-sha1:: test-sha1$X
>  	./test-sha1.sh
>  
> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>  	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
>  		$(SPARSE_FLAGS) $<
>  
> -.PHONY: sparse $(SP_OBJ)
>  sparse: $(SP_OBJ)

This "sparse" movement looks again contradictory with the goal announced in the commit message.

Show 6 quoted lines

> @@ -2237,6 +2243,7 @@ check: common-cmds.h
>  		exit 1; \
>  	fi
>  
> +
>  ### Installation rules

Useless hunk.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/