git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Peter Kjellerstedt <peter.kjellerstedt@axis.com>,
	Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: [PATCH 1/2] clone: send diagnostic messages to stderr
Date: Wed, 18 Sep 2013 16:05:13 -0400	[thread overview]
Message-ID: <20130918200513.GA731@sigill.intra.peff.net> (raw)
In-Reply-To: <20130918200152.GA17074@sigill.intra.peff.net>

Putting messages like "Cloning into.." and "done" on stdout
is un-Unix and uselessly clutters the stdout channel. Send
them to stderr.

We have to tweak two tests to accommodate this:

  1. t5601 checks for doubled output due to forking, and
     doesn't actually care where the output goes; adjust it
     to check stderr.

  2. t5702 is trying to test whether progress output was
     sent to stderr, but naively does so by checking
     whether stderr produced any output. Instead, have it
     look for "%", a token found in progress output but not
     elsewhere (and which lets us avoid hard-coding the
     progress text in the test).

This should not regress any scripts that try to parse the
current output, as the output is already internationalized
and therefore unstable.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/clone.c          | 10 +++++-----
 t/t5601-clone.sh         |  2 +-
 t/t5702-clone-options.sh |  9 +++++----
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index ca3eb68..8723a3a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -379,7 +379,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 	}
 
 	if (0 <= option_verbosity)
-		printf(_("done.\n"));
+		fprintf(stderr, _("done.\n"));
 }
 
 static const char *junk_work_tree;
@@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,
 
 	if (check_connectivity) {
 		if (0 <= option_verbosity)
-			printf(_("Checking connectivity... "));
+			fprintf(stderr, _("Checking connectivity... "));
 		if (check_everything_connected_with_transport(iterate_ref_map,
 							      0, &rm, transport))
 			die(_("remote did not send all necessary objects"));
 		if (0 <= option_verbosity)
-			printf(_("done\n"));
+			fprintf(stderr, _("done\n"));
 	}
 
 	if (refs) {
@@ -849,9 +849,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	if (0 <= option_verbosity) {
 		if (option_bare)
-			printf(_("Cloning into bare repository '%s'...\n"), dir);
+			fprintf(stderr, _("Cloning into bare repository '%s'...\n"), dir);
 		else
-			printf(_("Cloning into '%s'...\n"), dir);
+			fprintf(stderr, _("Cloning into '%s'...\n"), dir);
 	}
 	init_db(option_template, INIT_DB_QUIET);
 	write_config(&option_config);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0629149..b3b11e6 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -36,7 +36,7 @@ test_expect_success C_LOCALE_OUTPUT 'output from clone' '
 
 test_expect_success C_LOCALE_OUTPUT 'output from clone' '
 	rm -fr dst &&
-	git clone -n "file://$(pwd)/src" dst >output &&
+	git clone -n "file://$(pwd)/src" dst >output 2>&1 &&
 	test $(grep Clon output | wc -l) = 1
 '
 
diff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh
index 85cadfa..d3dbdfe 100755
--- a/t/t5702-clone-options.sh
+++ b/t/t5702-clone-options.sh
@@ -19,17 +19,18 @@ test_expect_success 'redirected clone -v' '
 
 '
 
-test_expect_success 'redirected clone' '
+test_expect_success 'redirected clone does not show progress' '
 
 	git clone "file://$(pwd)/parent" clone-redirected >out 2>err &&
-	test_must_be_empty err
+	! grep % err
 
 '
-test_expect_success 'redirected clone -v' '
+
+test_expect_success 'redirected clone -v does show progress' '
 
 	git clone --progress "file://$(pwd)/parent" clone-redirected-progress \
 		>out 2>err &&
-	test -s err
+	grep % err
 
 '
 
-- 
1.8.4.rc4.16.g228394f

  reply	other threads:[~2013-09-18 20:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-18 16:52 git clone silently aborts if stdout gets a broken pipe Peter Kjellerstedt
2013-09-18 18:45 ` Jeff King
2013-09-18 19:04   ` Jeff King
2013-09-18 19:31     ` Junio C Hamano
2013-09-18 20:01       ` Jeff King
2013-09-18 20:05         ` Jeff King [this message]
2013-09-18 20:06         ` [PATCH 2/2] clone: treat "checking connectivity" like other progress Jeff King
2013-09-18 20:35           ` [PATCH 3/2] clone: always set transport options Jeff King
2013-09-19  7:54   ` git clone silently aborts if stdout gets a broken pipe Peter Kjellerstedt
2013-09-19  8:35     ` Jeff King
2013-09-19 15:48       ` Peter Kjellerstedt

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=20130918200513.GA731@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peter.kjellerstedt@axis.com \
    /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).