threads / bug / 28293

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

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

## tl;dr

17 messages between Sep 3, 2011 and Sep 6, 2011.

replies: 16people: 4as markdown or json

Tobias Ulmer· Sep 3, 2011, 10:57 UTC · lore
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
Clemens Buchacher· Sep 3, 2011, 16:34 UTC · re: Tobias Ulmer · lore

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

Hello Tobias,
On Sat, Sep 03, 2011 at 12:57:23PM +0200, Tobias Ulmer wrote:
Show 5 quoted lines
> 
> 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

Clemens Buchacher· Sep 3, 2011, 16:34 UTC · re: Clemens Buchacher · lore

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

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
Clemens Buchacher· Sep 3, 2011, 16:34 UTC · re: Clemens Buchacher · lore

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

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
Clemens Buchacher· Sep 3, 2011, 16:34 UTC · re: Clemens Buchacher · lore

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

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
Junio C Hamano· Sep 5, 2011, 02:28 UTC · re: Clemens Buchacher · lore

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

Clemens Buchacher <drizzd@aon.at> writes:
Show 15 quoted lines
> @@ -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...).

Show 9 quoted lines
>  		}
>  		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.
Junio C Hamano· Sep 5, 2011, 03:01 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> 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
Junio C Hamano· Sep 5, 2011, 08:35 UTC · re: Clemens Buchacher · lore

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

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
Michael J Gruber· Sep 5, 2011, 09:23 UTC · re: Junio C Hamano · lore

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

Junio C Hamano venit, vidit, dixit 05.09.2011 10:35:
Show 38 quoted lines
> 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;
  }
Michael J Gruber· Sep 5, 2011, 14:17 UTC · re: Michael J Gruber · lore

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

Michael J Gruber venit, vidit, dixit 05.09.2011 11:23:
Show 58 quoted lines
> 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
Michael J Gruber· Sep 5, 2011, 14:24 UTC · re: Michael J Gruber · lore

[PATCH] t5541: avoid TAP test miscounting

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
Junio C Hamano· Sep 5, 2011, 19:34 UTC · re: Michael J Gruber · lore

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

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).

Michael J Gruber· Sep 6, 2011, 06:03 UTC · re: Junio C Hamano · lore

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

Junio C Hamano venit, vidit, dixit 05.09.2011 21:34:
Show 7 quoted lines
> 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
Junio C Hamano· Sep 6, 2011, 18:01 UTC · re: Clemens Buchacher · lore

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

Clemens Buchacher <drizzd@aon.at> writes:
Show 7 quoted lines
> 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.

Clemens Buchacher· Sep 6, 2011, 19:52 UTC · re: Junio C Hamano · lore

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

On Tue, Sep 06, 2011 at 11:01:44AM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> 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
Tobias Ulmer· Sep 4, 2011, 12:08 UTC · re: Clemens Buchacher · lore

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

On Sat, Sep 03, 2011 at 06:34:13PM +0200, Clemens Buchacher wrote:
Show 25 quoted lines
> 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
Junio C Hamano· Sep 4, 2011, 19:02 UTC · re: Tobias Ulmer · lore

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

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.

← back to recent threads