[BUG] git push --quiet fails with older versions

[BUG] git push --quiet fails with older versions

[Tobias Ulmer]

Hi guys,

my distro updated git to 1.7.6.1. Now git push --quiet, used in various
scripts, blows up against 1.7.6. I don't know how backwards compatible
git tries to be, but in my opinion that seems like a regression for a
patchlevel release.

The problem seems to be that the remote git receive-pack does not
understand --quiet. Maybe it is possible to check for the remote version
before sending incompatible commandline parameters?

Quick demo:

lead:ssh$ git version
git version 1.7.6.1
lead:ssh$ git push
Everything up-to-date
lead:ssh$ git push -q
usage: git receive-pack <git-dir>
fatal: The remote end hung up unexpectedly

The remote uses:
radon:~$ git version
git version 1.7.6

Please keep me in CC, I'm not subscribed.

Tobias

Re: [BUG] git push --quiet fails with older versions

[Clemens Buchacher]

Hello Tobias,

On Sat, Sep 03, 2011 at 12:57:23PM +0200, Tobias Ulmer wrote:
> 
> my distro updated git to 1.7.6.1. Now git push --quiet, used in various
> scripts, blows up against 1.7.6. I don't know how backwards compatible
> git tries to be, but in my opinion that seems like a regression for a
> patchlevel release.

Yes, that's a good point. I did not consider that.

> The problem seems to be that the remote git receive-pack does not
> understand --quiet. Maybe it is possible to check for the remote version
> before sending incompatible commandline parameters?

Indeed, that's what we must do. I am sending three patches in reply
to this email:

[PATCH 1/3] tests for push --quiet
[PATCH 2/3] fix push --quiet via http
[PATCH 3/3] push: old receive-pack does not understand --quiet

The third one fixes this issue on the client side.  It does not
depend on the first two. It applies to v1.7.6.1, maint, and master.
Would be great if you could test.

Thanks,
Clemens

[PATCH 1/3] tests for push --quiet

[Clemens Buchacher]

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 t/t5523-push-upstream.sh |    7 +++++++
 t/t5541-http-push.sh     |    8 ++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index c229fe6..9ee52cf 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -108,4 +108,11 @@ test_expect_failure TTY 'push --no-progress suppresses progress' '
 	! grep "Writing objects" err
 '
 
+test_expect_success TTY 'quiet push' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push --quiet --no-progress upstream master 2>&1 | tee output &&
+	test_cmp /dev/null output
+'
+
 test_done
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index a73c826..0dcb8df 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -5,6 +5,7 @@
 
 test_description='test smart pushing over http via http-backend'
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
 
 if test -n "$NO_CURL"; then
 	skip_all='skipping test, git built without http support'
@@ -154,5 +155,12 @@ test_expect_success 'push (chunked)' '
 	 test $HEAD = $(git rev-parse --verify HEAD))
 '
 
+test_expect_failure TTY 'quiet push' '
+	cd "$ROOT_PATH"/test_repo_clone &&
+	test_commit quiet &&
+	test_terminal git push --quiet --no-progress 2>&1 | tee output &&
+	test_cmp /dev/null output
+'
+
 stop_httpd
 test_done
-- 
1.7.6.1

[PATCH 2/3] fix push --quiet via http

[Clemens Buchacher]

A verbosity of 0 means quiet for remote helpers.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 remote-curl.c        |    2 +-
 t/t5541-http-push.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 5798aa5..2341106 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -762,7 +762,7 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
 		argv[argc++] = "--thin";
 	if (options.dry_run)
 		argv[argc++] = "--dry-run";
-	if (options.verbosity < 0)
+	if (options.verbosity == 0)
 		argv[argc++] = "--quiet";
 	else if (options.verbosity > 1)
 		argv[argc++] = "--verbose";
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 0dcb8df..e756a08 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -155,7 +155,7 @@ test_expect_success 'push (chunked)' '
 	 test $HEAD = $(git rev-parse --verify HEAD))
 '
 
-test_expect_failure TTY 'quiet push' '
+test_expect_success TTY 'quiet push' '
 	cd "$ROOT_PATH"/test_repo_clone &&
 	test_commit quiet &&
 	test_terminal git push --quiet --no-progress 2>&1 | tee output &&
-- 
1.7.6.1

[PATCH 3/3] push: old receive-pack does not understand --quiet

[Clemens Buchacher]

Commit 90a6c7d4 (propagate --quiet to send-pack/receive-pack)
introduced the --quiet option to receive-pack and made send-pack
pass that option. Older versions of receive-pack do not recognize
the option, however, and terminate immediately.

This change restores backwards compatibility by adding a 'quiet'
capability to receive-pack.

Reported-by: Tobias Ulmer <tobiasu@tmux.org>
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 builtin/receive-pack.c |   10 ++++++----
 builtin/send-pack.c    |   16 +++++++---------
 transport.c            |   10 +++-------
 3 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 60260d0..06c481a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -31,6 +31,7 @@ static int transfer_unpack_limit = -1;
 static int unpack_limit = 100;
 static int report_status;
 static int use_sideband;
+static int quiet;
 static int prefer_ofs_delta = 1;
 static int auto_update_server_info;
 static int auto_gc = 1;
@@ -114,7 +115,7 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void
 	else
 		packet_write(1, "%s %s%c%s%s\n",
 			     sha1_to_hex(sha1), path, 0,
-			     " report-status delete-refs side-band-64k",
+			     " report-status delete-refs side-band-64k quiet",
 			     prefer_ofs_delta ? " ofs-delta" : "");
 	sent_capabilities = 1;
 	return 0;
@@ -636,6 +637,8 @@ static struct command *read_head_info(void)
 				report_status = 1;
 			if (strstr(refname + reflen + 1, "side-band-64k"))
 				use_sideband = LARGE_PACKET_MAX;
+			if (strstr(refname + reflen + 1, "quiet"))
+				quiet = 1;
 		}
 		cmd = xcalloc(1, sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
@@ -669,7 +672,7 @@ static const char *parse_pack_header(struct pack_header *hdr)
 
 static const char *pack_lockfile;
 
-static const char *unpack(int quiet)
+static const char *unpack()
 {
 	struct pack_header hdr;
 	const char *hdr_err;
@@ -788,7 +791,6 @@ static void add_alternate_refs(void)
 
 int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 {
-	int quiet = 0;
 	int advertise_refs = 0;
 	int stateless_rpc = 0;
 	int i;
@@ -855,7 +857,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		const char *unpack_status = NULL;
 
 		if (!delete_only(commands))
-			unpack_status = unpack(quiet);
+			unpack_status = unpack();
 		execute_commands(commands, unpack_status);
 		if (pack_lockfile)
 			unlink_or_warn(pack_lockfile);
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 40a1675..a8d6b4c 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -263,6 +263,8 @@ int send_pack(struct send_pack_args *args,
 		args->use_ofs_delta = 1;
 	if (server_supports("side-band-64k"))
 		use_sideband = 1;
+	if (!server_supports("quiet"))
+		args->quiet = 0;
 
 	if (!remote_refs) {
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
@@ -301,11 +303,12 @@ int send_pack(struct send_pack_args *args,
 			char *old_hex = sha1_to_hex(ref->old_sha1);
 			char *new_hex = sha1_to_hex(ref->new_sha1);
 
-			if (!cmds_sent && (status_report || use_sideband)) {
-				packet_buf_write(&req_buf, "%s %s %s%c%s%s",
+			if (!cmds_sent && (status_report || use_sideband || args->quiet)) {
+				packet_buf_write(&req_buf, "%s %s %s%c%s%s%s",
 					old_hex, new_hex, ref->name, 0,
 					status_report ? " report-status" : "",
-					use_sideband ? " side-band-64k" : "");
+					use_sideband ? " side-band-64k" : "",
+					args->quiet ? " quiet" : "");
 			}
 			else
 				packet_buf_write(&req_buf, "%s %s %s",
@@ -492,13 +495,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 		fd[0] = 0;
 		fd[1] = 1;
 	} else {
-		struct strbuf sb = STRBUF_INIT;
-		strbuf_addstr(&sb, receivepack);
-		if (args.quiet)
-			strbuf_addstr(&sb, " --quiet");
-		conn = git_connect(fd, dest, sb.buf,
+		conn = git_connect(fd, dest, receivepack,
 			args.verbose ? CONNECT_VERBOSE : 0);
-		strbuf_release(&sb);
 	}
 
 	memset(&extra_have, 0, sizeof(extra_have));
diff --git a/transport.c b/transport.c
index 98c5778..c9c8056 100644
--- a/transport.c
+++ b/transport.c
@@ -482,18 +482,14 @@ static int set_git_option(struct git_transport_options *opts,
 static int connect_setup(struct transport *transport, int for_push, int verbose)
 {
 	struct git_transport_data *data = transport->data;
-	struct strbuf sb = STRBUF_INIT;
 
 	if (data->conn)
 		return 0;
 
-	strbuf_addstr(&sb, for_push ? data->options.receivepack :
-				 data->options.uploadpack);
-	if (for_push && transport->verbose < 0)
-		strbuf_addstr(&sb, " --quiet");
-	data->conn = git_connect(data->fd, transport->url, sb.buf,
+	data->conn = git_connect(data->fd, transport->url,
+				 for_push ? data->options.receivepack :
+				 data->options.uploadpack,
 				 verbose ? CONNECT_VERBOSE : 0);
-	strbuf_release(&sb);
 
 	return 0;
 }
-- 
1.7.6.1

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Junio C Hamano]

Clemens Buchacher <drizzd@aon.at> writes:

> @@ -114,7 +115,7 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void
>  	else
>  		packet_write(1, "%s %s%c%s%s\n",
>  			     sha1_to_hex(sha1), path, 0,
> -			     " report-status delete-refs side-band-64k",
> +			     " report-status delete-refs side-band-64k quiet",
>  			     prefer_ofs_delta ? " ofs-delta" : "");
>  	sent_capabilities = 1;
>  	return 0;
> @@ -636,6 +637,8 @@ static struct command *read_head_info(void)
>  				report_status = 1;
>  			if (strstr(refname + reflen + 1, "side-band-64k"))
>  				use_sideband = LARGE_PACKET_MAX;
> +			if (strstr(refname + reflen + 1, "quiet"))
> +				quiet = 1;

Side note.

We may want to make sure that this is not part of a different token word
(if we knew better, we would have written the other side so that we can
just test against " quiet ", but that is not possible, sigh...).

>  		}
>  		cmd = xcalloc(1, sizeof(struct command) + len - 80);
>  		hashcpy(cmd->old_sha1, old_sha1);
> @@ -669,7 +672,7 @@ static const char *parse_pack_header(struct pack_header *hdr)
>  
>  static const char *pack_lockfile;
>  
> -static const char *unpack(int quiet)
> +static const char *unpack()

I'll amend this as "static const char *unpack(void)" while applying.

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Clemens Buchacher <drizzd@aon.at> writes:
> ...
>> @@ -636,6 +637,8 @@ static struct command *read_head_info(void)
>>  				report_status = 1;
>>  			if (strstr(refname + reflen + 1, "side-band-64k"))
>>  				use_sideband = LARGE_PACKET_MAX;
>> +			if (strstr(refname + reflen + 1, "quiet"))
>> +				quiet = 1;
>
> Side note.
>
> We may want to make sure that this is not part of a different token word
> (if we knew better, we would have written the other side so that we can
> just test against " quiet ", but that is not possible, sigh...).

... but we can do this.

-- >8 --
Subject: [PATCH] server_supports(): parse feature list more carefully

We have been carefully choosing feature names used in the protocol
extensions so that the vocabulary does not contain a word that is a
substring of another word, so it is not a real problem, but we have
recently added "quiet" feature word, which would mean we cannot later
add some other word with "quiet" (e.g. "quiet-push"), which is awkward.

Let's make sure that we can eventually be able to do so by teaching the
clients and servers that feature words consist of non whitespace
letters. This parser also allows us to later add features with parameters
e.g. "feature=1.5" (parameter values need to be quoted for whitespaces,
but we will worry about the detauls when we do introduce them).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/receive-pack.c |    7 ++++---
 cache.h                |    1 +
 connect.c              |   23 +++++++++++++++++++++--
 upload-pack.c          |   22 +++++++++++++---------
 4 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b71a1ca..927f307 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -600,11 +600,12 @@ static struct command *read_head_info(void)
 		refname = line + 82;
 		reflen = strlen(refname);
 		if (reflen + 82 < len) {
-			if (strstr(refname + reflen + 1, "report-status"))
+			const char *feature_list = refname + reflen + 1;
+			if (parse_feature_request(feature_list, "report-status"))
 				report_status = 1;
-			if (strstr(refname + reflen + 1, "side-band-64k"))
+			if (parse_feature_request(feature_list, "side-band-64k"))
 				use_sideband = LARGE_PACKET_MAX;
-			if (strstr(refname + reflen + 1, "quiet"))
+			if (parse_feature_request(feature_list, "quiet"))
 				quiet = 1;
 		}
 		cmd = xcalloc(1, sizeof(struct command) + len - 80);
diff --git a/cache.h b/cache.h
index e11cf6a..2933d04 100644
--- a/cache.h
+++ b/cache.h
@@ -996,6 +996,7 @@ struct extra_have_objects {
 };
 extern struct ref **get_remote_heads(int in, struct ref **list, int nr_match, char **match, unsigned int flags, struct extra_have_objects *);
 extern int server_supports(const char *feature);
+extern const char *parse_feature_request(const char *features, const char *feature);
 
 extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
 
diff --git a/connect.c b/connect.c
index 2119c3f..2184fd8 100644
--- a/connect.c
+++ b/connect.c
@@ -104,8 +104,27 @@ struct ref **get_remote_heads(int in, struct ref **list,
 
 int server_supports(const char *feature)
 {
-	return server_capabilities &&
-		strstr(server_capabilities, feature) != NULL;
+	return !!parse_feature_request(server_capabilities, feature);
+}
+
+const char *parse_feature_request(const char *feature_list, const char *feature)
+{
+	int len;
+
+	if (!feature_list)
+		return NULL;
+
+	len = strlen(feature);
+	while (*feature_list) {
+		const char *found = strstr(feature_list, feature);
+		if (!found)
+			return NULL;
+		if ((feature_list == found || isspace(found[-1])) &&
+		    (!found[len] || isspace(found[len]) || found[len] == '='))
+			return found;
+		feature_list = found + 1;
+	}
+	return NULL;
 }
 
 int path_match(const char *path, int nr, char **match)
diff --git a/upload-pack.c b/upload-pack.c
index ce5cbbe..a47a556 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -509,6 +509,7 @@ static void receive_needs(void)
 		write_str_in_full(debug_fd, "#S\n");
 	for (;;) {
 		struct object *o;
+		const char *features;
 		unsigned char sha1_buf[20];
 		len = packet_read_line(0, line, sizeof(line));
 		reset_timeout();
@@ -540,23 +541,26 @@ static void receive_needs(void)
 		    get_sha1_hex(line+5, sha1_buf))
 			die("git upload-pack: protocol error, "
 			    "expected to get sha, not '%s'", line);
-		if (strstr(line+45, "multi_ack_detailed"))
+
+		features = line + 45;
+
+		if (parse_feature_request(features, "multi_ack_detailed"))
 			multi_ack = 2;
-		else if (strstr(line+45, "multi_ack"))
+		else if (parse_feature_request(features, "multi_ack"))
 			multi_ack = 1;
-		if (strstr(line+45, "no-done"))
+		if (parse_feature_request(features, "no-done"))
 			no_done = 1;
-		if (strstr(line+45, "thin-pack"))
+		if (parse_feature_request(features, "thin-pack"))
 			use_thin_pack = 1;
-		if (strstr(line+45, "ofs-delta"))
+		if (parse_feature_request(features, "ofs-delta"))
 			use_ofs_delta = 1;
-		if (strstr(line+45, "side-band-64k"))
+		if (parse_feature_request(features, "side-band-64k"))
 			use_sideband = LARGE_PACKET_MAX;
-		else if (strstr(line+45, "side-band"))
+		else if (parse_feature_request(features, "side-band"))
 			use_sideband = DEFAULT_PACKET_MAX;
-		if (strstr(line+45, "no-progress"))
+		if (parse_feature_request(features, "no-progress"))
 			no_progress = 1;
-		if (strstr(line+45, "include-tag"))
+		if (parse_feature_request(features, "include-tag"))
 			use_include_tag = 1;
 
 		/* We have sent all our refs already, and the other end
-- 
1.7.7.rc0.175.gb3212

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Junio C Hamano]

Hmm, with this whole series merged to 'pu', I somehow am getting this
error:

$ make -j8 \
    DEFAULT_TEST_TARGET=prove \
    GIT_PROVE_OPTS=-j8 \
    T=t5541-http-push.sh test
*** prove ***
t5541-http-push.sh .. All 1 subtests passed 

Test Summary Report
-------------------
t5541-http-push.sh (Wstat: 0 Tests: 1 Failed: 0)
  Parse errors: No plan found in TAP output
Files=1, Tests=1,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.06 cusr  0.01 csys =  0.09 CPU)
Result: FAIL
make[1]: *** [prove] Error 1
make[1]: Leaving directory `/srv/project/git/git.git/t'
make: *** [test] Error 2

Without prove (drop "DEFAULT_TEST_TARGET=prove" from the command line),
I do not see the breakage.

*** t5541-http-push.sh ***
ok 1 - set up terminal for tests
# passed all 1 test(s)
1..1 # SKIP Network testing disabled (define GIT_TEST_HTTPD to enable)
make aggregate-results
make[3]: Entering directory `/srv/project/git/git.git/t'
for f in test-results/t*-*.counts; do \
                echo "$f"; \
        done | '/bin/sh' ./aggregate-results.sh
fixed   0
success 1
failed  0
broken  0
total   1

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 05.09.2011 10:35:
> Hmm, with this whole series merged to 'pu', I somehow am getting this
> error:
> 
> $ make -j8 \
>     DEFAULT_TEST_TARGET=prove \
>     GIT_PROVE_OPTS=-j8 \
>     T=t5541-http-push.sh test
> *** prove ***
> t5541-http-push.sh .. All 1 subtests passed 
> 
> Test Summary Report
> -------------------
> t5541-http-push.sh (Wstat: 0 Tests: 1 Failed: 0)
>   Parse errors: No plan found in TAP output
> Files=1, Tests=1,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.06 cusr  0.01 csys =  0.09 CPU)
> Result: FAIL
> make[1]: *** [prove] Error 1
> make[1]: Leaving directory `/srv/project/git/git.git/t'
> make: *** [test] Error 2
> 
> Without prove (drop "DEFAULT_TEST_TARGET=prove" from the command line),
> I do not see the breakage.
> 
> *** t5541-http-push.sh ***
> ok 1 - set up terminal for tests
> # passed all 1 test(s)
> 1..1 # SKIP Network testing disabled (define GIT_TEST_HTTPD to enable)
> make aggregate-results
> make[3]: Entering directory `/srv/project/git/git.git/t'
> for f in test-results/t*-*.counts; do \
>                 echo "$f"; \
>         done | '/bin/sh' ./aggregate-results.sh
> fixed   0
> success 1
> failed  0
> broken  0
> total   1
> 

Being cc'ed makes me feel guilty but I don't know what for... Anyway, in
case you need more test points, with pu at v1.7.7-rc0-315-g55af6ac  and
prove I get:

*** prove ***
t5541-http-push.sh .. skipped: Network testing disabled (define
GIT_TEST_HTTPD to enable)
Files=1, Tests=0,  0 wallclock secs ( 0.04 usr  0.01 sys +  0.00 cusr
0.02 csys =  0.07 CPU)
Result: NOTESTS

Patch 3/3 does not apply (am) to pu but leaves neither changes nor
conflicts in my wt. -3 gives a conflict which I don't know how to
resolve without digging further, see below.

Michael

The conflict in send-pack.c is simple, but what about:

diff --cc builtin/receive-pack.c
index 0b42e21,06c481a..0000000
--- i/builtin/receive-pack.c
+++ w/builtin/receive-pack.c
@@@ -29,9 -29,9 +29,10 @@@ static int receive_fsck_objects
  static int receive_unpack_limit = -1;
  static int transfer_unpack_limit = -1;
  static int unpack_limit = 100;
 +static unsigned long limit_pack_size, limit_commit_count,
limit_object_count;
  static int report_status;
  static int use_sideband;
+ static int quiet;
  static int prefer_ofs_delta = 1;
  static int auto_update_server_info;
  static int auto_gc = 1;
@@@ -144,8 -113,10 +145,15 @@@ static int show_ref(const char *path, c
        if (sent_capabilities)
                packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
        else
++<<<<<<< HEAD
 +              packet_write(1, "%s %s%c%s\n",
 +                           sha1_to_hex(sha1), path, 0, capabilities());
++=======
+               packet_write(1, "%s %s%c%s%s\n",
+                            sha1_to_hex(sha1), path, 0,
+                            " report-status delete-refs side-band-64k
quiet",
+                            prefer_ofs_delta ? " ofs-delta" : "");
++>>>>>>> push: old receive-pack does not understand --quiet
        sent_capabilities = 1;
        return 0;
  }

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Michael J Gruber]

Michael J Gruber venit, vidit, dixit 05.09.2011 11:23:
> Junio C Hamano venit, vidit, dixit 05.09.2011 10:35:
>> Hmm, with this whole series merged to 'pu', I somehow am getting this
>> error:
>>
>> $ make -j8 \
>>     DEFAULT_TEST_TARGET=prove \
>>     GIT_PROVE_OPTS=-j8 \
>>     T=t5541-http-push.sh test
>> *** prove ***
>> t5541-http-push.sh .. All 1 subtests passed 
>>
>> Test Summary Report
>> -------------------
>> t5541-http-push.sh (Wstat: 0 Tests: 1 Failed: 0)
>>   Parse errors: No plan found in TAP output
>> Files=1, Tests=1,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.06 cusr  0.01 csys =  0.09 CPU)
>> Result: FAIL
>> make[1]: *** [prove] Error 1
>> make[1]: Leaving directory `/srv/project/git/git.git/t'
>> make: *** [test] Error 2
>>
>> Without prove (drop "DEFAULT_TEST_TARGET=prove" from the command line),
>> I do not see the breakage.
>>
>> *** t5541-http-push.sh ***
>> ok 1 - set up terminal for tests
>> # passed all 1 test(s)
>> 1..1 # SKIP Network testing disabled (define GIT_TEST_HTTPD to enable)
>> make aggregate-results
>> make[3]: Entering directory `/srv/project/git/git.git/t'
>> for f in test-results/t*-*.counts; do \
>>                 echo "$f"; \
>>         done | '/bin/sh' ./aggregate-results.sh
>> fixed   0
>> success 1
>> failed  0
>> broken  0
>> total   1
>>
> 
> Being cc'ed makes me feel guilty but I don't know what for... Anyway, in
> case you need more test points, with pu at v1.7.7-rc0-315-g55af6ac  and
> prove I get:
> 
> *** prove ***
> t5541-http-push.sh .. skipped: Network testing disabled (define
> GIT_TEST_HTTPD to enable)
> Files=1, Tests=0,  0 wallclock secs ( 0.04 usr  0.01 sys +  0.00 cusr
> 0.02 csys =  0.07 CPU)
> Result: NOTESTS
> 
> Patch 3/3 does not apply (am) to pu but leaves neither changes nor
> conflicts in my wt. -3 gives a conflict which I don't know how to
> resolve without digging further, see below.
> 
> Michael
> 
> The conflict in send-pack.c is simple, but what about:
...

OK, I resolved this now, and am getting the same error as you, and am
stumped. But for me, 1/3 introduces that problem already. In fact, the hunk

--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -5,6 +5,7 @@

 test_description='test smart pushing over http via http-backend'
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh

suffices to trigger the problem. The test run in lib-terminal increases
the count but the output is lost. Patch upcoming.


Michael

[PATCH] t5541: avoid TAP test miscounting

[Michael J Gruber]

lib-terminal.sh runs a test and thus increases the test count, but the
output is lost so that TAP produces a "no plan found error".

Move the lib-terminal call after the lib-httpd and make TAP happy
(though still leave me clueless).

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t5541-http-push.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 0dcb8df..a18265c 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -5,7 +5,6 @@
 
 test_description='test smart pushing over http via http-backend'
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-terminal.sh
 
 if test -n "$NO_CURL"; then
 	skip_all='skipping test, git built without http support'
@@ -15,6 +14,7 @@ fi
 ROOT_PATH="$PWD"
 LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5541'}
 . "$TEST_DIRECTORY"/lib-httpd.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
 start_httpd
 
 test_expect_success 'setup remote repository' '
-- 
1.7.7.rc0.328.g9d6c7

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Being cc'ed makes me feel guilty but I don't know what for...

No guilt involved. 28d836c (test: allow running the tests under "prove",
2010-10-14) made you a good _suspect_ for having more clues than I do to
resolve it, iow, it was asking for help, not pointing any finger.

Anyhow, thanks for a quick workaround, even though I still do not
understand what the issue is (that is, what is wrong with the
t/lib-terminal helper).

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 05.09.2011 21:34:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Being cc'ed makes me feel guilty but I don't know what for...
> 
> No guilt involved. 28d836c (test: allow running the tests under "prove",
> 2010-10-14) made you a good _suspect_ for having more clues than I do to
> resolve it, iow, it was asking for help, not pointing any finger.

I plead non-guilty on the account of "prove-clue". All evidence to the
contrary is fabricated ;)

> Anyhow, thanks for a quick workaround, even though I still do not
> understand what the issue is (that is, what is wrong with the
> t/lib-terminal helper).

Seriously, I use prove as my standard target because I prefer its output
for parallel tests, but I have no idea how to debug the test output
(which TAP wants to parse). I've made sure that my workaround works with
and without the httpd runs, and with -j1 and -j8. My blackbox
understanding tells me that a subtest counter used to be increased and
the respective output quelled - maybe by a lib*.sh which used to be
sourced after lib-terminal.sh, because after rearranging the output is
there.

Michael

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Junio C Hamano]

Clemens Buchacher <drizzd@aon.at> writes:

> Commit 90a6c7d4 (propagate --quiet to send-pack/receive-pack)
> introduced the --quiet option to receive-pack and made send-pack
> pass that option. Older versions of receive-pack do not recognize
> the option, however, and terminate immediately.
>
> This change restores backwards compatibility by adding a 'quiet'
> capability to receive-pack.

Wouldn't this mean that there is no point in adding --quiet command line
option to the receive-pack command? IOW, shouldn't parts of 90a6c7d
(propagate --quiet to send-pack/receive-pack, 2011-07-30) be reverted?

At this late stage in the release cycle, I would rather prefer to revert
the whole commit and leave anything new for the next cycle.

Re: [PATCH 3/3] push: old receive-pack does not understand --quiet

[Clemens Buchacher]

On Tue, Sep 06, 2011 at 11:01:44AM -0700, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
> 
> > Commit 90a6c7d4 (propagate --quiet to send-pack/receive-pack)
> > introduced the --quiet option to receive-pack and made send-pack
> > pass that option. Older versions of receive-pack do not recognize
> > the option, however, and terminate immediately.
> >
> > This change restores backwards compatibility by adding a 'quiet'
> > capability to receive-pack.
> 
> Wouldn't this mean that there is no point in adding --quiet command line
> option to the receive-pack command? IOW, shouldn't parts of 90a6c7d
> (propagate --quiet to send-pack/receive-pack, 2011-07-30) be reverted?
> 
> At this late stage in the release cycle, I would rather prefer to revert
> the whole commit and leave anything new for the next cycle.

Sure, we can do that. Only then 1.7.6.1 will be broken against all
versions except 1.7.6.1 on the server side. Right now it is only
broken against 1.7.6 and older versions.

But I am ok with it either way. I will have to check how much
resolving needs to be done after the revert.

Clemens

Re: [BUG] git push --quiet fails with older versions

[Tobias Ulmer]

On Sat, Sep 03, 2011 at 06:34:13PM +0200, Clemens Buchacher wrote:
> Hello Tobias,
> 
> On Sat, Sep 03, 2011 at 12:57:23PM +0200, Tobias Ulmer wrote:
> > 
> > my distro updated git to 1.7.6.1. Now git push --quiet, used in various
> > scripts, blows up against 1.7.6. I don't know how backwards compatible
> > git tries to be, but in my opinion that seems like a regression for a
> > patchlevel release.
> 
> Yes, that's a good point. I did not consider that.
> 
> > The problem seems to be that the remote git receive-pack does not
> > understand --quiet. Maybe it is possible to check for the remote version
> > before sending incompatible commandline parameters?
> 
> Indeed, that's what we must do. I am sending three patches in reply
> to this email:
> 
> [PATCH 1/3] tests for push --quiet
> [PATCH 2/3] fix push --quiet via http
> [PATCH 3/3] push: old receive-pack does not understand --quiet
> 
> The third one fixes this issue on the client side.  It does not
> depend on the first two. It applies to v1.7.6.1, maint, and master.
> Would be great if you could test.

Hi Clemens,

I've just gotten round to test patch 3/3 and it works perfectly against
1.7.6 as well as 1.7.6.1. It suppresses the output if the remote git
version is 1.7.6.1, like it should.

Thanks for your quick response,
Tobias

> 
> Thanks,
> Clemens

Re: [BUG] git push --quiet fails with older versions

[Junio C Hamano]

Tobias Ulmer <tobiasu@tmux.org> writes:

> my distro updated git to 1.7.6.1. Now git push --quiet, used in various
> scripts, blows up against 1.7.6.

This kind of regression is not acceptable not just for point maintenance
release, but for feature releases. 1.7.7 client must work with 1.7.6
server just fine without such a breakage. I should have been more careful
when reviewing.

Thanks for a report, Tobias, and thanks for a quick fix, Clemens.