git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "H.Merijn Brand" <h.m.brand@xs4all.nl>
Cc: Miklos Vajna <vmiklos@frugalware.org>, git@vger.kernel.org
Subject: Re: Install issues
Date: Tue, 11 Nov 2008 13:11:45 -0800	[thread overview]
Message-ID: <7vskpyynla.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20081110173101.3d76613b@pc09.procura.nl> (H. Merijn Brand's message of "Mon, 10 Nov 2008 17:31:01 +0100")

"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:

> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>  	./test-sha1.sh
>  
>  check: common-cmds.h
> +	@`sparse </dev/null 2>/dev/null` || (\
> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> +	    echo "Did you mean 'make test' instead?" ;\
> +	    exit 1 )

When you mean "grouping", using {} is much clearer to convey your
intention.  Use of needless (subshell) forces the reader to wonder if you
wanted to do something that affects the environment for later commands
inside, and in this case you didn't.

Why do you have sparse check inside a backtick to produce a string to be
interpreted as a command to be executed?

How about doing this instead?  'sparse' without any parameter exits with
success status silently; when you do not have the command, the shell will
complain with "sparse: command not found" anyway, so you only need to
suggest "make 'test'" and nothing else.

-- >8 --
Subject: Makefile: help people who run 'make check' by mistake

The target to run self test is 'make test', but there are people who try
'make check' and worse yet do not have sparse installed.

Suggest 'make test' target when they do not have 'sparse'.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git c/Makefile w/Makefile
index 40309e1..d3137ca 100644
--- c/Makefile
+++ w/Makefile
@@ -1355,7 +1355,16 @@ check-sha1:: test-sha1$X
 	./test-sha1.sh
 
 check: common-cmds.h
-	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
+	if sparse; \
+	then \
+		for i in *.c; \
+		do \
+			sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; \
+		done; \
+	else \
+		echo 2>&1 "Did you mean 'make test'?"; \
+		exit 1; \
+	fi
 
 remove-dashes:
 	./fixup-builtins $(BUILT_INS) $(PROGRAMS) $(SCRIPTS)

      parent reply	other threads:[~2008-11-11 21:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-10 11:17 Install issues H.Merijn Brand
2008-11-10 11:39 ` Miklos Vajna
2008-11-10 16:31   ` H.Merijn Brand
2008-11-10 17:51     ` Miklos Vajna
2008-11-11  7:59       ` H.Merijn Brand
2008-11-11  9:17         ` Andreas Ericsson
2008-11-11 17:47       ` Junio C Hamano
2008-11-11 18:02         ` H.Merijn Brand
2008-11-11 23:03           ` Boyd Lynn Gerber
2008-11-11 23:35             ` H.Merijn Brand
2008-11-11 23:42               ` Junio C Hamano
2008-11-11 23:53                 ` H.Merijn Brand
2008-11-12  8:14         ` Andreas Ericsson
2008-11-11 21:11     ` Junio C Hamano [this message]

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=7vskpyynla.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=h.m.brand@xs4all.nl \
    --cc=vmiklos@frugalware.org \
    /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).