[PATCH v2 1/2] receive-pack: switch global variable 'commands' to a parameter

Subsystems: the rest

STALE3729d

10 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH v2 1/2] receive-pack: switch global variable 'commands' to a parameter

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

Receive-pack is inconsistent in its usage of the 'commands'
variable; though it is setup as a global and accessed that way by
execute_commands(), report(), and run_receive_hook(), it is also
passed as a parameter to delete_only() and run_update_post_hook().

For consistency, make it local to cmd_receive_pack and pass it as a
parameter. As long as we're cleaning up, also make our use of the
names 'commands' and 'cmd' consistent.

Signed-off-by: Jay Soffian <redacted>
---
 builtin/receive-pack.c |   60 +++++++++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 33 deletions(-)

Changes from v1:

- Minor reword of commit message
- Removed two extraneous whitespace changes

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0559fcc..fffb6ea 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -134,8 +134,6 @@ struct command {
 	char ref_name[FLEX_ARRAY]; /* more */
 };
 
-static struct command *commands;
-
 static const char pre_receive_hook[] = "hooks/pre-receive";
 static const char post_receive_hook[] = "hooks/post-receive";
 
@@ -188,7 +186,7 @@ static int copy_to_sideband(int in, int out, void *arg)
 	return 0;
 }
 
-static int run_receive_hook(const char *hook_name)
+static int run_receive_hook(struct command *commands, const char *hook_name)
 {
 	static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
 	struct command *cmd;
@@ -447,15 +445,15 @@ static const char *update(struct command *cmd)
 
 static char update_post_hook[] = "hooks/post-update";
 
-static void run_update_post_hook(struct command *cmd)
+static void run_update_post_hook(struct command *commands)
 {
-	struct command *cmd_p;
+	struct command *cmd;
 	int argc;
 	const char **argv;
 	struct child_process proc;
 
-	for (argc = 0, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
-		if (cmd_p->error_string)
+	for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
+		if (cmd->error_string)
 			continue;
 		argc++;
 	}
@@ -464,12 +462,12 @@ static void run_update_post_hook(struct command *cmd)
 	argv = xmalloc(sizeof(*argv) * (2 + argc));
 	argv[0] = update_post_hook;
 
-	for (argc = 1, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
+	for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
 		char *p;
-		if (cmd_p->error_string)
+		if (cmd->error_string)
 			continue;
-		p = xmalloc(strlen(cmd_p->ref_name) + 1);
-		strcpy(p, cmd_p->ref_name);
+		p = xmalloc(strlen(cmd->ref_name) + 1);
+		strcpy(p, cmd->ref_name);
 		argv[argc] = p;
 		argc++;
 	}
@@ -488,37 +486,32 @@ static void run_update_post_hook(struct command *cmd)
 	}
 }
 
-static void execute_commands(const char *unpacker_error)
+static void execute_commands(struct command *commands, const char *unpacker_error)
 {
-	struct command *cmd = commands;
+	struct command *cmd;
 	unsigned char sha1[20];
 
 	if (unpacker_error) {
-		while (cmd) {
+		for (cmd = commands; cmd; cmd = cmd->next)
 			cmd->error_string = "n/a (unpacker error)";
-			cmd = cmd->next;
-		}
 		return;
 	}
 
-	if (run_receive_hook(pre_receive_hook)) {
-		while (cmd) {
+	if (run_receive_hook(commands, pre_receive_hook)) {
+		for (cmd = commands; cmd; cmd = cmd->next)
 			cmd->error_string = "pre-receive hook declined";
-			cmd = cmd->next;
-		}
 		return;
 	}
 
 	head_name = resolve_ref("HEAD", sha1, 0, NULL);
 
-	while (cmd) {
+	for (cmd = commands; cmd; cmd = cmd->next)
 		cmd->error_string = update(cmd);
-		cmd = cmd->next;
-	}
 }
 
-static void read_head_info(void)
+static struct command *read_head_info(void)
 {
+	struct command *commands = NULL;
 	struct command **p = &commands;
 	for (;;) {
 		static char line[1000];
@@ -557,6 +550,7 @@ static void read_head_info(void)
 		*p = cmd;
 		p = &cmd->next;
 	}
+	return commands;
 }
 
 static const char *parse_pack_header(struct pack_header *hdr)
@@ -643,7 +637,7 @@ static const char *unpack(void)
 	}
 }
 
-static void report(const char *unpack_status)
+static void report(struct command *commands, const char *unpack_status)
 {
 	struct command *cmd;
 	struct strbuf buf = STRBUF_INIT;
@@ -667,12 +661,12 @@ static void report(const char *unpack_status)
 	strbuf_release(&buf);
 }
 
-static int delete_only(struct command *cmd)
+static int delete_only(struct command *commands)
 {
-	while (cmd) {
+	struct command *cmd;
+	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!is_null_sha1(cmd->new_sha1))
 			return 0;
-		cmd = cmd->next;
 	}
 	return 1;
 }
@@ -722,6 +716,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	int stateless_rpc = 0;
 	int i;
 	char *dir = NULL;
+	struct command *commands;
 
 	argv++;
 	for (i = 1; i < argc; i++) {
@@ -772,18 +767,17 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	if (advertise_refs)
 		return 0;
 
-	read_head_info();
-	if (commands) {
+	if ((commands = read_head_info()) != NULL) {
 		const char *unpack_status = NULL;
 
 		if (!delete_only(commands))
 			unpack_status = unpack();
-		execute_commands(unpack_status);
+		execute_commands(commands, unpack_status);
 		if (pack_lockfile)
 			unlink_or_warn(pack_lockfile);
 		if (report_status)
-			report(unpack_status);
-		run_receive_hook(post_receive_hook);
+			report(commands, unpack_status);
+		run_receive_hook(commands, post_receive_hook);
 		run_update_post_hook(commands);
 		if (auto_gc) {
 			const char *argv_gc_auto[] = {
-- 
1.7.0.3.436.g2b878

[PATCH v2 2/2] receive-pack: detect aliased updates which can occur with symrefs

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

When pushing to a remote repo the sending side filters out aliased
updates (e.g., foo:baz bar:baz). However, it is not possible for the
sender to know if two refs are aliased on the receiving side via
symrefs. Here is one such scenario:

  $ git init origin
  $ (cd origin && touch file && git add file && git commit -a -m intial)
  $ git clone --bare origin origin.git
  $ rm -rf origin

  $ git clone origin.git client

  $ git clone --mirror client backup.git &&
  $ (cd backup.git && git remote set-head origin --auto)

  $ (cd client &&
	git remote add --mirror backup ../backup.git &&
	echo change1 > file && git commit -a -m change1 &&
	git push origin &&
	git push backup
	)

The push to backup fails with:

  Counting objects: 5, done.
  Writing objects: 100% (3/3), 244 bytes, done.
  Total 3 (delta 0), reused 0 (delta 0)
  Unpacking objects: 100% (3/3), done.
  error: Ref refs/remotes/origin/master is at ef3... but expected 262...
  remote: error: failed to lock refs/remotes/origin/master
  To ../backup.git
     262cd57..ef307ff  master -> master
     262cd57..ef307ff  origin/HEAD -> origin/HEAD
   ! [remote rejected] origin/master -> origin/master (failed to lock)
  error: failed to push some refs to '../backup.git'

The reason is that refs/remotes/origin/HEAD is a symref to
refs/remotes/origin/master, but it is not possible for the sending side
to unambiguously know this.

This commit fixes the issue by having receive-pack ignore any update to
a symref whose target is being identically updated. If a symref and its
target are being updated inconsistently, then the update for both fails
with an error message ("refusing inconsistent update...") to help
diagnose the situation.

Signed-off-by: Jay Soffian <redacted>
---
 builtin/receive-pack.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++-
 t/t5516-fetch-push.sh  |   45 ++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 1 deletions(-)

Changes from v1 (incorporating Junio's feedback):
- Reformatted commit message; minor rewording.
- Detect situation where there is an inconsistent aliased update and
  give a better diagnostic than "failed to lock"
- Add additional test case for inconsistent update situation

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index fffb6ea..414446b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -9,6 +9,7 @@
 #include "object.h"
 #include "remote.h"
 #include "transport.h"
+#include "string-list.h"
 
 static const char receive_pack_usage[] = "git receive-pack <git-dir>";
 
@@ -129,6 +130,7 @@ static void write_head_info(void)
 struct command {
 	struct command *next;
 	const char *error_string;
+	unsigned int skip_update;
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
 	char ref_name[FLEX_ARRAY]; /* more */
@@ -486,6 +488,61 @@ static void run_update_post_hook(struct command *commands)
 	}
 }
 
+static void check_aliased_update(struct command *cmd, struct string_list *list)
+{
+	struct string_list_item *item;
+	struct command *dst_cmd;
+	unsigned char sha1[20];
+	char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
+	int flag;
+
+	const char *dst_name = resolve_ref(cmd->ref_name, sha1, 0, &flag);
+
+	if (!(flag & REF_ISSYMREF))
+		return;
+
+	if ((item = string_list_lookup(dst_name, list)) == NULL)
+		return;
+
+	cmd->skip_update = 1;
+
+	dst_cmd = (struct command *) item->util;
+
+	if (!hashcmp(cmd->old_sha1, dst_cmd->old_sha1) &&
+	    !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
+		return;
+
+	strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
+	strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+	rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
+		 " its target '%s' (%s..%s)",
+		 cmd->ref_name, cmd_oldh, cmd_newh,
+		 dst_cmd->ref_name, dst_oldh, dst_newh);
+
+	cmd->error_string = dst_cmd->error_string =
+		"inconsistent aliased update";
+}
+
+static void check_aliased_updates(struct command *commands)
+{
+	struct command *cmd;
+	struct string_list ref_list = { NULL, 0, 0, 0 };
+
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		struct string_list_item *item =
+			string_list_append(cmd->ref_name, &ref_list);
+		item->util = (void *)cmd;
+	}
+	sort_string_list(&ref_list);
+
+	for (cmd = commands; cmd; cmd = cmd->next)
+		check_aliased_update(cmd, &ref_list);
+
+	string_list_clear(&ref_list, 0);
+}
+
 static void execute_commands(struct command *commands, const char *unpacker_error)
 {
 	struct command *cmd;
@@ -503,9 +560,11 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
+	check_aliased_updates(commands);
+
 	head_name = resolve_ref("HEAD", sha1, 0, NULL);
 
-	for (cmd = commands; cmd; cmd = cmd->next)
+	for (cmd = commands; cmd && !cmd->skip_update; cmd = cmd->next)
 		cmd->error_string = update(cmd);
 }
 
@@ -545,6 +604,7 @@ static struct command *read_head_info(void)
 		hashcpy(cmd->old_sha1, old_sha1);
 		hashcpy(cmd->new_sha1, new_sha1);
 		memcpy(cmd->ref_name, line + 82, len - 81);
+		cmd->skip_update = 0;
 		cmd->error_string = NULL;
 		cmd->next = NULL;
 		*p = cmd;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2de98e6..bc3f24f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -660,6 +660,51 @@ test_expect_success 'push with branches containing #' '
 	git checkout master
 '
 
+test_expect_success 'push into aliased refs (consistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(cd child2 &&
+		: >path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		git branch bar &&
+		git push ../child1 foo bar
+	)
+'
+
+test_expect_success 'push into aliased refs (inconsistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(cd child2 &&
+		: >path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		: >path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch bar &&
+		test_must_fail git push ../child1 foo bar 2>stderr &&
+		grep "refusing inconsistent update" stderr
+	)
+'
+
 test_expect_success 'push --porcelain' '
 	mk_empty &&
 	echo >.git/foo  "To testrepo" &&
-- 
1.7.0.3.436.g2b878

Re: [PATCH v2 2/2] receive-pack: detect aliased updates which can occur with symrefs

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

On Mon, Apr 19, 2010 at 12:25 PM, Jay Soffian [off-list ref] wrote:
When pushing to a remote repo the sending side filters out aliased
updates (e.g., foo:baz bar:baz). However, it is not possible for the
sender to know if two refs are aliased on the receiving side via
symrefs. Here is one such scenario:
Don't receive this. There's a logic error in it. I'll send a follow up shortly.

j.

[PATCH v2 2/2] receive-pack: detect aliased updates which can occur with symrefs

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

When pushing to a remote repo the sending side filters out aliased
updates (e.g., foo:baz bar:baz). However, it is not possible for the
sender to know if two refs are aliased on the receiving side via
symrefs. Here is one such scenario:

  $ git init origin
  $ (cd origin && touch file && git add file && git commit -a -m intial)
  $ git clone --bare origin origin.git
  $ rm -rf origin

  $ git clone origin.git client

  $ git clone --mirror client backup.git &&
  $ (cd backup.git && git remote set-head origin --auto)

  $ (cd client &&
	git remote add --mirror backup ../backup.git &&
	echo change1 > file && git commit -a -m change1 &&
	git push origin &&
	git push backup
	)

The push to backup fails with:

  Counting objects: 5, done.
  Writing objects: 100% (3/3), 244 bytes, done.
  Total 3 (delta 0), reused 0 (delta 0)
  Unpacking objects: 100% (3/3), done.
  error: Ref refs/remotes/origin/master is at ef3... but expected 262...
  remote: error: failed to lock refs/remotes/origin/master
  To ../backup.git
     262cd57..ef307ff  master -> master
     262cd57..ef307ff  origin/HEAD -> origin/HEAD
   ! [remote rejected] origin/master -> origin/master (failed to lock)
  error: failed to push some refs to '../backup.git'

The reason is that refs/remotes/origin/HEAD is a symref to
refs/remotes/origin/master, but it is not possible for the sending side
to unambiguously know this.

This commit fixes the issue by having receive-pack ignore any update to
a symref whose target is being identically updated. If a symref and its
target are being updated inconsistently, then the update for both fails
with an error message ("refusing inconsistent update...") to help
diagnose the situation.

Signed-off-by: Jay Soffian <redacted>
---
 builtin/receive-pack.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++-
 t/t5516-fetch-push.sh  |   45 +++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 1 deletions(-)

Changes from v1 (incorporating Junio's feedback):
- Reformatted commit message; minor rewording.
- Detect situation where there is an inconsistent aliased update and
  give a better diagnostic than "failed to lock"
- Add additional test case for inconsistent update situation

[
This one is correct, the interdiff from
[off-list ref] is

  diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
  index 414446b..7b269d2 100644
  --- a/builtin/receive-pack.c
  +++ b/builtin/receive-pack.c
  @@ -512,6 +512,8 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
   	    !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
   		return;
   
  +	dst_cmd->skip_update = 1;
  +
   	strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
   	strcat(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
   	strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
  )
]
  
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index fffb6ea..7b269d2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -9,6 +9,7 @@
 #include "object.h"
 #include "remote.h"
 #include "transport.h"
+#include "string-list.h"
 
 static const char receive_pack_usage[] = "git receive-pack <git-dir>";
 
@@ -129,6 +130,7 @@ static void write_head_info(void)
 struct command {
 	struct command *next;
 	const char *error_string;
+	unsigned int skip_update;
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
 	char ref_name[FLEX_ARRAY]; /* more */
@@ -486,6 +488,63 @@ static void run_update_post_hook(struct command *commands)
 	}
 }
 
+static void check_aliased_update(struct command *cmd, struct string_list *list)
+{
+	struct string_list_item *item;
+	struct command *dst_cmd;
+	unsigned char sha1[20];
+	char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
+	int flag;
+
+	const char *dst_name = resolve_ref(cmd->ref_name, sha1, 0, &flag);
+
+	if (!(flag & REF_ISSYMREF))
+		return;
+
+	if ((item = string_list_lookup(dst_name, list)) == NULL)
+		return;
+
+	cmd->skip_update = 1;
+
+	dst_cmd = (struct command *) item->util;
+
+	if (!hashcmp(cmd->old_sha1, dst_cmd->old_sha1) &&
+	    !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
+		return;
+
+	dst_cmd->skip_update = 1;
+
+	strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
+	strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+	rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
+		 " its target '%s' (%s..%s)",
+		 cmd->ref_name, cmd_oldh, cmd_newh,
+		 dst_cmd->ref_name, dst_oldh, dst_newh);
+
+	cmd->error_string = dst_cmd->error_string =
+		"inconsistent aliased update";
+}
+
+static void check_aliased_updates(struct command *commands)
+{
+	struct command *cmd;
+	struct string_list ref_list = { NULL, 0, 0, 0 };
+
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		struct string_list_item *item =
+			string_list_append(cmd->ref_name, &ref_list);
+		item->util = (void *)cmd;
+	}
+	sort_string_list(&ref_list);
+
+	for (cmd = commands; cmd; cmd = cmd->next)
+		check_aliased_update(cmd, &ref_list);
+
+	string_list_clear(&ref_list, 0);
+}
+
 static void execute_commands(struct command *commands, const char *unpacker_error)
 {
 	struct command *cmd;
@@ -503,9 +562,11 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
+	check_aliased_updates(commands);
+
 	head_name = resolve_ref("HEAD", sha1, 0, NULL);
 
-	for (cmd = commands; cmd; cmd = cmd->next)
+	for (cmd = commands; cmd && !cmd->skip_update; cmd = cmd->next)
 		cmd->error_string = update(cmd);
 }
 
@@ -545,6 +606,7 @@ static struct command *read_head_info(void)
 		hashcpy(cmd->old_sha1, old_sha1);
 		hashcpy(cmd->new_sha1, new_sha1);
 		memcpy(cmd->ref_name, line + 82, len - 81);
+		cmd->skip_update = 0;
 		cmd->error_string = NULL;
 		cmd->next = NULL;
 		*p = cmd;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2de98e6..bc3f24f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -660,6 +660,51 @@ test_expect_success 'push with branches containing #' '
 	git checkout master
 '
 
+test_expect_success 'push into aliased refs (consistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(cd child2 &&
+		: >path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		git branch bar &&
+		git push ../child1 foo bar
+	)
+'
+
+test_expect_success 'push into aliased refs (inconsistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(cd child2 &&
+		: >path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		: >path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch bar &&
+		test_must_fail git push ../child1 foo bar 2>stderr &&
+		grep "refusing inconsistent update" stderr
+	)
+'
+
 test_expect_success 'push --porcelain' '
 	mk_empty &&
 	echo >.git/foo  "To testrepo" &&
-- 
1.7.0.3.436.g2b878

[PATCH v3 0/3] js/maint-receive-pack-symref-alias

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

- Incorporated Junio's feedback from v1 and v2.
- Rebuilt on-top of f78683f, which is where v1 was applied in pu.
- Added an additional patch to the series to reformat the tests in
  t5516-fetch-push.sh to use a consistent style, per Junio's tweaks to the
  test I wrote for v1.

Jay Soffian (3):
  receive-pack: switch global variable 'commands' to a parameter
  t5516-fetch-push.sh: style cleanup
  receive-pack: detect aliased updates which can occur with symrefs

 builtin-receive-pack.c |  128 ++++++++++++++++++++++++++++++++++--------------
 t/t5516-fetch-push.sh  |  118 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 186 insertions(+), 60 deletions(-)

interdiff from what's in pu:
 builtin-receive-pack.c |   76 +++++++++++++++++++++++++------------
 t/t5516-fetch-push.sh  |   98 ++++++++++++++++++++++++++++++++++++------------
 2 files changed, 126 insertions(+), 48 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index a2e3bc8..bb34757 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -130,6 +130,7 @@ static void write_head_info(void)
 struct command {
 	struct command *next;
 	const char *error_string;
+	unsigned int skip_update;
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
 	char ref_name[FLEX_ARRAY]; /* more */
@@ -487,30 +488,67 @@ static void run_update_post_hook(struct command *commands)
 	}
 }
 
-static int aliased_ref(struct command *cmd, struct string_list *list)
+static void check_aliased_update(struct command *cmd, struct string_list *list)
 {
 	struct string_list_item *item;
+	struct command *dst_cmd;
 	unsigned char sha1[20];
+	char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
 	int flag;
 
 	const char *dst_name = resolve_ref(cmd->ref_name, sha1, 0, &flag);
 
 	if (!(flag & REF_ISSYMREF))
-		return 0;
+		return;
+
+	if ((item = string_list_lookup(dst_name, list)) == NULL)
+		return;
+
+	cmd->skip_update = 1;
+
+	dst_cmd = (struct command *) item->util;
+
+	if (!hashcmp(cmd->old_sha1, dst_cmd->old_sha1) &&
+	    !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
+		return;
+
+	dst_cmd->skip_update = 1;
+
+	strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
+	strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+	rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
+		 " its target '%s' (%s..%s)",
+		 cmd->ref_name, cmd_oldh, cmd_newh,
+		 dst_cmd->ref_name, dst_oldh, dst_newh);
 
-	if ((item = string_list_lookup(dst_name, list)) != NULL) {
-		struct command *other_cmd = (struct command *) item->util;
-		return (!(hashcmp(cmd->old_sha1, other_cmd->old_sha1) &&
-			hashcmp(cmd->new_sha1, other_cmd->new_sha1)));
+	cmd->error_string = dst_cmd->error_string =
+		"inconsistent aliased update";
+}
+
+static void check_aliased_updates(struct command *commands)
+{
+	struct command *cmd;
+	struct string_list ref_list = { NULL, 0, 0, 0 };
+
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		struct string_list_item *item =
+			string_list_append(cmd->ref_name, &ref_list);
+		item->util = (void *)cmd;
 	}
-	return 0;
+	sort_string_list(&ref_list);
+
+	for (cmd = commands; cmd; cmd = cmd->next)
+		check_aliased_update(cmd, &ref_list);
+
+	string_list_clear(&ref_list, 0);
 }
 
 static void execute_commands(struct command *commands, const char *unpacker_error)
 {
 	struct command *cmd;
 	unsigned char sha1[20];
-	struct string_list ref_list = { NULL, 0, 0, 0 };
 
 	if (unpacker_error) {
 		for (cmd = commands; cmd; cmd = cmd->next)
@@ -524,27 +562,19 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
-	head_name = resolve_ref("HEAD", sha1, 0, NULL);
+	check_aliased_updates(commands);
 
-	for (cmd = commands; cmd; cmd = cmd->next) {
-		struct string_list_item *item =
-			string_list_append(cmd->ref_name, &ref_list);
-		item->util = (void *)cmd;
-	}
-	sort_string_list(&ref_list);
+	head_name = resolve_ref("HEAD", sha1, 0, NULL);
 
-	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (!aliased_ref(cmd, &ref_list))
+	for (cmd = commands; cmd; cmd = cmd->next)
+		if (!cmd->skip_update)
 			cmd->error_string = update(cmd);
-	}
-	string_list_clear(&ref_list, 0);
 }
 
 static struct command *read_head_info(void)
 {
 	struct command *commands = NULL;
 	struct command **p = &commands;
-
 	for (;;) {
 		static char line[1000];
 		unsigned char old_sha1[20], new_sha1[20];
@@ -573,12 +603,10 @@ static struct command *read_head_info(void)
 			if (strstr(refname + reflen + 1, "side-band-64k"))
 				use_sideband = LARGE_PACKET_MAX;
 		}
-		cmd = xmalloc(sizeof(struct command) + len - 80);
+		cmd = xcalloc(1, sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
 		hashcpy(cmd->new_sha1, new_sha1);
 		memcpy(cmd->ref_name, line + 82, len - 81);
-		cmd->error_string = NULL;
-		cmd->next = NULL;
 		*p = cmd;
 		p = &cmd->next;
 	}
@@ -671,8 +699,8 @@ static const char *unpack(void)
 
 static void report(struct command *commands, const char *unpack_status)
 {
-	struct strbuf buf = STRBUF_INIT;
 	struct command *cmd;
+	struct strbuf buf = STRBUF_INIT;
 
 	packet_buf_write(&buf, "unpack %s\n",
 			 unpack_status ? unpack_status : "ok");
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 350e734..f0813e0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -64,13 +64,13 @@ check_push_result () {
 
 test_expect_success setup '
 
-	: >path1 &&
+	>path1 &&
 	git add path1 &&
 	test_tick &&
 	git commit -a -m repo &&
 	the_first_commit=$(git show-ref -s --verify refs/heads/master) &&
 
-	: >path2 &&
+	>path2 &&
 	git add path2 &&
 	test_tick &&
 	git commit -a -m second &&
@@ -483,8 +483,10 @@ git config --remove-section remote.there
 test_expect_success 'push with dry-run' '
 
 	mk_test heads/master &&
-	(cd testrepo &&
-	 old_commit=$(git show-ref -s --verify refs/heads/master)) &&
+	(
+		cd testrepo &&
+		old_commit=$(git show-ref -s --verify refs/heads/master)
+	) &&
 	git push --dry-run testrepo &&
 	check_push_result $old_commit heads/master
 '
@@ -493,10 +495,13 @@ test_expect_success 'push updates local refs' '
 
 	mk_test heads/master &&
 	mk_child child &&
-	(cd child &&
+	(
+		cd child &&
 		git pull .. master &&
 		git push &&
-	test $(git rev-parse master) = $(git rev-parse remotes/origin/master))
+		test $(git rev-parse master) = \
+			$(git rev-parse remotes/origin/master)
+	)
 
 '
 
@@ -506,10 +511,13 @@ test_expect_success 'push updates up-to-date local refs' '
 	mk_child child1 &&
 	mk_child child2 &&
 	(cd child1 && git pull .. master && git push) &&
-	(cd child2 &&
+	(
+		cd child2 &&
 		git pull ../child1 master &&
 		git push &&
-	test $(git rev-parse master) = $(git rev-parse remotes/origin/master))
+		test $(git rev-parse master) = \
+			$(git rev-parse remotes/origin/master)
+	)
 
 '
 
@@ -517,9 +525,11 @@ test_expect_success 'push preserves up-to-date packed refs' '
 
 	mk_test heads/master &&
 	mk_child child &&
-	(cd child &&
+	(
+		cd child &&
 		git push &&
-	! test -f .git/refs/remotes/origin/master)
+		! test -f .git/refs/remotes/origin/master
+	)
 
 '
 
@@ -530,11 +540,13 @@ test_expect_success 'push does not update local refs on failure' '
 	mkdir testrepo/.git/hooks &&
 	echo exit 1 >testrepo/.git/hooks/pre-receive &&
 	chmod +x testrepo/.git/hooks/pre-receive &&
-	(cd child &&
+	(
+		cd child &&
 		git pull .. master
 		test_must_fail git push &&
 		test $(git rev-parse master) != \
-			$(git rev-parse remotes/origin/master))
+			$(git rev-parse remotes/origin/master)
+	)
 
 '
 
@@ -575,34 +587,41 @@ test_expect_success 'push --delete refuses src:dest refspecs' '
 
 test_expect_success 'warn on push to HEAD of non-bare repository' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
-		git config receive.denyCurrentBranch warn) &&
+		git config receive.denyCurrentBranch warn
+	) &&
 	git push testrepo master 2>stderr &&
 	grep "warning: updating the current branch" stderr
 '
 
 test_expect_success 'deny push to HEAD of non-bare repository' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
-		git config receive.denyCurrentBranch true) &&
+		git config receive.denyCurrentBranch true
+	) &&
 	test_must_fail git push testrepo master
 '
 
 test_expect_success 'allow push to HEAD of bare repository (bare)' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
 		git config receive.denyCurrentBranch true &&
-		git config core.bare true) &&
+		git config core.bare true
+	) &&
 	git push testrepo master 2>stderr &&
 	! grep "warning: updating the current branch" stderr
 '
 
 test_expect_success 'allow push to HEAD of non-bare repository (config)' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
 		git config receive.denyCurrentBranch false
 	) &&
@@ -615,7 +634,8 @@ test_expect_success 'fetch with branches' '
 	git branch second $the_first_commit &&
 	git checkout second &&
 	echo ".." > testrepo/.git/branches/branch1 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git fetch branch1 &&
 		r=$(git show-ref -s --verify refs/heads/branch1) &&
 		test "z$r" = "z$the_commit" &&
@@ -627,7 +647,8 @@ test_expect_success 'fetch with branches' '
 test_expect_success 'fetch with branches containing #' '
 	mk_empty &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git fetch branch2 &&
 		r=$(git show-ref -s --verify refs/heads/branch2) &&
 		test "z$r" = "z$the_first_commit" &&
@@ -641,7 +662,8 @@ test_expect_success 'push with branches' '
 	git checkout second &&
 	echo "testrepo" > .git/branches/branch1 &&
 	git push branch1 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		r=$(git show-ref -s --verify refs/heads/master) &&
 		test "z$r" = "z$the_first_commit" &&
 		test 1 = $(git for-each-ref refs/heads | wc -l)
@@ -652,7 +674,8 @@ test_expect_success 'push with branches containing #' '
 	mk_empty &&
 	echo "testrepo#branch3" > .git/branches/branch2 &&
 	git push branch2 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		r=$(git show-ref -s --verify refs/heads/branch3) &&
 		test "z$r" = "z$the_first_commit" &&
 		test 1 = $(git for-each-ref refs/heads | wc -l)
@@ -660,7 +683,7 @@ test_expect_success 'push with branches containing #' '
 	git checkout master
 '
 
-test_expect_success 'push into aliased refs' '
+test_expect_success 'push into aliased refs (consistent)' '
 	mk_test heads/master &&
 	mk_child child1 &&
 	mk_child child2 &&
@@ -682,4 +705,31 @@ test_expect_success 'push into aliased refs' '
 	)
 '
 
+test_expect_success 'push into aliased refs (inconsistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(
+		cd child2 &&
+		>path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		>path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch bar &&
+		test_must_fail git push ../child1 foo bar 2>stderr &&
+		grep "refusing inconsistent update" stderr
+	)
+'
+
 test_done

[PATCH v3 1/3] receive-pack: switch global variable 'commands' to a parameter

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

Receive-pack is inconsistent in its usage of the 'commands'
variable; though it is setup as a global and accessed that way by
execute_commands(), report(), and run_receive_hook(), it is also
passed as a parameter to delete_only() and run_update_post_hook().

For consistency, make it local to cmd_receive_pack and pass it as a
parameter. As long as we're cleaning up, also make our use of the
names 'commands' and 'cmd' consistent.

Signed-off-by: Jay Soffian <redacted>
---
 builtin-receive-pack.c |   60 +++++++++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 0559fcc..fffb6ea 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -134,8 +134,6 @@ struct command {
 	char ref_name[FLEX_ARRAY]; /* more */
 };
 
-static struct command *commands;
-
 static const char pre_receive_hook[] = "hooks/pre-receive";
 static const char post_receive_hook[] = "hooks/post-receive";
 
@@ -188,7 +186,7 @@ static int copy_to_sideband(int in, int out, void *arg)
 	return 0;
 }
 
-static int run_receive_hook(const char *hook_name)
+static int run_receive_hook(struct command *commands, const char *hook_name)
 {
 	static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
 	struct command *cmd;
@@ -447,15 +445,15 @@ static const char *update(struct command *cmd)
 
 static char update_post_hook[] = "hooks/post-update";
 
-static void run_update_post_hook(struct command *cmd)
+static void run_update_post_hook(struct command *commands)
 {
-	struct command *cmd_p;
+	struct command *cmd;
 	int argc;
 	const char **argv;
 	struct child_process proc;
 
-	for (argc = 0, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
-		if (cmd_p->error_string)
+	for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
+		if (cmd->error_string)
 			continue;
 		argc++;
 	}
@@ -464,12 +462,12 @@ static void run_update_post_hook(struct command *cmd)
 	argv = xmalloc(sizeof(*argv) * (2 + argc));
 	argv[0] = update_post_hook;
 
-	for (argc = 1, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
+	for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
 		char *p;
-		if (cmd_p->error_string)
+		if (cmd->error_string)
 			continue;
-		p = xmalloc(strlen(cmd_p->ref_name) + 1);
-		strcpy(p, cmd_p->ref_name);
+		p = xmalloc(strlen(cmd->ref_name) + 1);
+		strcpy(p, cmd->ref_name);
 		argv[argc] = p;
 		argc++;
 	}
@@ -488,37 +486,32 @@ static void run_update_post_hook(struct command *cmd)
 	}
 }
 
-static void execute_commands(const char *unpacker_error)
+static void execute_commands(struct command *commands, const char *unpacker_error)
 {
-	struct command *cmd = commands;
+	struct command *cmd;
 	unsigned char sha1[20];
 
 	if (unpacker_error) {
-		while (cmd) {
+		for (cmd = commands; cmd; cmd = cmd->next)
 			cmd->error_string = "n/a (unpacker error)";
-			cmd = cmd->next;
-		}
 		return;
 	}
 
-	if (run_receive_hook(pre_receive_hook)) {
-		while (cmd) {
+	if (run_receive_hook(commands, pre_receive_hook)) {
+		for (cmd = commands; cmd; cmd = cmd->next)
 			cmd->error_string = "pre-receive hook declined";
-			cmd = cmd->next;
-		}
 		return;
 	}
 
 	head_name = resolve_ref("HEAD", sha1, 0, NULL);
 
-	while (cmd) {
+	for (cmd = commands; cmd; cmd = cmd->next)
 		cmd->error_string = update(cmd);
-		cmd = cmd->next;
-	}
 }
 
-static void read_head_info(void)
+static struct command *read_head_info(void)
 {
+	struct command *commands = NULL;
 	struct command **p = &commands;
 	for (;;) {
 		static char line[1000];
@@ -557,6 +550,7 @@ static void read_head_info(void)
 		*p = cmd;
 		p = &cmd->next;
 	}
+	return commands;
 }
 
 static const char *parse_pack_header(struct pack_header *hdr)
@@ -643,7 +637,7 @@ static const char *unpack(void)
 	}
 }
 
-static void report(const char *unpack_status)
+static void report(struct command *commands, const char *unpack_status)
 {
 	struct command *cmd;
 	struct strbuf buf = STRBUF_INIT;
@@ -667,12 +661,12 @@ static void report(const char *unpack_status)
 	strbuf_release(&buf);
 }
 
-static int delete_only(struct command *cmd)
+static int delete_only(struct command *commands)
 {
-	while (cmd) {
+	struct command *cmd;
+	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!is_null_sha1(cmd->new_sha1))
 			return 0;
-		cmd = cmd->next;
 	}
 	return 1;
 }
@@ -722,6 +716,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	int stateless_rpc = 0;
 	int i;
 	char *dir = NULL;
+	struct command *commands;
 
 	argv++;
 	for (i = 1; i < argc; i++) {
@@ -772,18 +767,17 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	if (advertise_refs)
 		return 0;
 
-	read_head_info();
-	if (commands) {
+	if ((commands = read_head_info()) != NULL) {
 		const char *unpack_status = NULL;
 
 		if (!delete_only(commands))
 			unpack_status = unpack();
-		execute_commands(unpack_status);
+		execute_commands(commands, unpack_status);
 		if (pack_lockfile)
 			unlink_or_warn(pack_lockfile);
 		if (report_status)
-			report(unpack_status);
-		run_receive_hook(post_receive_hook);
+			report(commands, unpack_status);
+		run_receive_hook(commands, post_receive_hook);
 		run_update_post_hook(commands);
 		if (auto_gc) {
 			const char *argv_gc_auto[] = {
-- 
1.7.0.3.436.g2b878

[PATCH v3 2/3] t5516-fetch-push.sh: style cleanup

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

Cleanup t5516-fetch-push.sh to use prevailing test script style

Signed-off-by: Jay Soffian <redacted>
---
 t/t5516-fetch-push.sh |   69 ++++++++++++++++++++++++++++++++----------------
 1 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 0f04b2e..3148789 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -64,13 +64,13 @@ check_push_result () {
 
 test_expect_success setup '
 
-	: >path1 &&
+	>path1 &&
 	git add path1 &&
 	test_tick &&
 	git commit -a -m repo &&
 	the_first_commit=$(git show-ref -s --verify refs/heads/master) &&
 
-	: >path2 &&
+	>path2 &&
 	git add path2 &&
 	test_tick &&
 	git commit -a -m second &&
@@ -483,8 +483,10 @@ git config --remove-section remote.there
 test_expect_success 'push with dry-run' '
 
 	mk_test heads/master &&
-	(cd testrepo &&
-	 old_commit=$(git show-ref -s --verify refs/heads/master)) &&
+	(
+		cd testrepo &&
+		old_commit=$(git show-ref -s --verify refs/heads/master)
+	) &&
 	git push --dry-run testrepo &&
 	check_push_result $old_commit heads/master
 '
@@ -493,10 +495,13 @@ test_expect_success 'push updates local refs' '
 
 	mk_test heads/master &&
 	mk_child child &&
-	(cd child &&
+	(
+		cd child &&
 		git pull .. master &&
 		git push &&
-	test $(git rev-parse master) = $(git rev-parse remotes/origin/master))
+		test $(git rev-parse master) = \
+			$(git rev-parse remotes/origin/master)
+	)
 
 '
 
@@ -506,10 +511,13 @@ test_expect_success 'push updates up-to-date local refs' '
 	mk_child child1 &&
 	mk_child child2 &&
 	(cd child1 && git pull .. master && git push) &&
-	(cd child2 &&
+	(
+		cd child2 &&
 		git pull ../child1 master &&
 		git push &&
-	test $(git rev-parse master) = $(git rev-parse remotes/origin/master))
+		test $(git rev-parse master) = \
+			$(git rev-parse remotes/origin/master)
+	)
 
 '
 
@@ -517,9 +525,11 @@ test_expect_success 'push preserves up-to-date packed refs' '
 
 	mk_test heads/master &&
 	mk_child child &&
-	(cd child &&
+	(
+		cd child &&
 		git push &&
-	! test -f .git/refs/remotes/origin/master)
+		! test -f .git/refs/remotes/origin/master
+	)
 
 '
 
@@ -530,11 +540,13 @@ test_expect_success 'push does not update local refs on failure' '
 	mkdir testrepo/.git/hooks &&
 	echo exit 1 >testrepo/.git/hooks/pre-receive &&
 	chmod +x testrepo/.git/hooks/pre-receive &&
-	(cd child &&
+	(
+		cd child &&
 		git pull .. master
 		test_must_fail git push &&
 		test $(git rev-parse master) != \
-			$(git rev-parse remotes/origin/master))
+			$(git rev-parse remotes/origin/master)
+	)
 
 '
 
@@ -575,34 +587,41 @@ test_expect_success 'push --delete refuses src:dest refspecs' '
 
 test_expect_success 'warn on push to HEAD of non-bare repository' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
-		git config receive.denyCurrentBranch warn) &&
+		git config receive.denyCurrentBranch warn
+	) &&
 	git push testrepo master 2>stderr &&
 	grep "warning: updating the current branch" stderr
 '
 
 test_expect_success 'deny push to HEAD of non-bare repository' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
-		git config receive.denyCurrentBranch true) &&
+		git config receive.denyCurrentBranch true
+	) &&
 	test_must_fail git push testrepo master
 '
 
 test_expect_success 'allow push to HEAD of bare repository (bare)' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
 		git config receive.denyCurrentBranch true &&
-		git config core.bare true) &&
+		git config core.bare true
+	) &&
 	git push testrepo master 2>stderr &&
 	! grep "warning: updating the current branch" stderr
 '
 
 test_expect_success 'allow push to HEAD of non-bare repository (config)' '
 	mk_test heads/master
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git checkout master &&
 		git config receive.denyCurrentBranch false
 	) &&
@@ -615,7 +634,8 @@ test_expect_success 'fetch with branches' '
 	git branch second $the_first_commit &&
 	git checkout second &&
 	echo ".." > testrepo/.git/branches/branch1 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git fetch branch1 &&
 		r=$(git show-ref -s --verify refs/heads/branch1) &&
 		test "z$r" = "z$the_commit" &&
@@ -627,7 +647,8 @@ test_expect_success 'fetch with branches' '
 test_expect_success 'fetch with branches containing #' '
 	mk_empty &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		git fetch branch2 &&
 		r=$(git show-ref -s --verify refs/heads/branch2) &&
 		test "z$r" = "z$the_first_commit" &&
@@ -641,7 +662,8 @@ test_expect_success 'push with branches' '
 	git checkout second &&
 	echo "testrepo" > .git/branches/branch1 &&
 	git push branch1 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		r=$(git show-ref -s --verify refs/heads/master) &&
 		test "z$r" = "z$the_first_commit" &&
 		test 1 = $(git for-each-ref refs/heads | wc -l)
@@ -652,7 +674,8 @@ test_expect_success 'push with branches containing #' '
 	mk_empty &&
 	echo "testrepo#branch3" > .git/branches/branch2 &&
 	git push branch2 &&
-	(cd testrepo &&
+	(
+		cd testrepo &&
 		r=$(git show-ref -s --verify refs/heads/branch3) &&
 		test "z$r" = "z$the_first_commit" &&
 		test 1 = $(git for-each-ref refs/heads | wc -l)
-- 
1.7.0.3.436.g2b878

[PATCH v3 3/3] receive-pack: detect aliased updates which can occur with symrefs

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

When pushing to a remote repo the sending side filters out aliased
updates (e.g., foo:baz bar:baz). However, it is not possible for the
sender to know if two refs are aliased on the receiving side via
symrefs. Here is one such scenario:

  $ git init origin
  $ (cd origin && touch file && git add file && git commit -a -m intial)
  $ git clone --bare origin origin.git
  $ rm -rf origin

  $ git clone origin.git client

  $ git clone --mirror client backup.git &&
  $ (cd backup.git && git remote set-head origin --auto)

  $ (cd client &&
	git remote add --mirror backup ../backup.git &&
	echo change1 > file && git commit -a -m change1 &&
	git push origin &&
	git push backup
	)

The push to backup fails with:

  Counting objects: 5, done.
  Writing objects: 100% (3/3), 244 bytes, done.
  Total 3 (delta 0), reused 0 (delta 0)
  Unpacking objects: 100% (3/3), done.
  error: Ref refs/remotes/origin/master is at ef3... but expected 262...
  remote: error: failed to lock refs/remotes/origin/master
  To ../backup.git
     262cd57..ef307ff  master -> master
     262cd57..ef307ff  origin/HEAD -> origin/HEAD
   ! [remote rejected] origin/master -> origin/master (failed to lock)
  error: failed to push some refs to '../backup.git'

The reason is that refs/remotes/origin/HEAD is a symref to
refs/remotes/origin/master, but it is not possible for the sending side
to unambiguously know this.

This commit fixes the issue by having receive-pack ignore any update to
a symref whose target is being identically updated. If a symref and its
target are being updated inconsistently, then the update for both fails
with an error message ("refusing inconsistent update...") to help
diagnose the situation.

Signed-off-by: Jay Soffian <redacted>
---
 builtin-receive-pack.c |   68 +++++++++++++++++++++++++++++++++++++++++++++---
 t/t5516-fetch-push.sh  |   49 ++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index fffb6ea..bb34757 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -9,6 +9,7 @@
 #include "object.h"
 #include "remote.h"
 #include "transport.h"
+#include "string-list.h"
 
 static const char receive_pack_usage[] = "git receive-pack <git-dir>";
 
@@ -129,6 +130,7 @@ static void write_head_info(void)
 struct command {
 	struct command *next;
 	const char *error_string;
+	unsigned int skip_update;
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
 	char ref_name[FLEX_ARRAY]; /* more */
@@ -486,6 +488,63 @@ static void run_update_post_hook(struct command *commands)
 	}
 }
 
+static void check_aliased_update(struct command *cmd, struct string_list *list)
+{
+	struct string_list_item *item;
+	struct command *dst_cmd;
+	unsigned char sha1[20];
+	char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
+	int flag;
+
+	const char *dst_name = resolve_ref(cmd->ref_name, sha1, 0, &flag);
+
+	if (!(flag & REF_ISSYMREF))
+		return;
+
+	if ((item = string_list_lookup(dst_name, list)) == NULL)
+		return;
+
+	cmd->skip_update = 1;
+
+	dst_cmd = (struct command *) item->util;
+
+	if (!hashcmp(cmd->old_sha1, dst_cmd->old_sha1) &&
+	    !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
+		return;
+
+	dst_cmd->skip_update = 1;
+
+	strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
+	strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+	rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
+		 " its target '%s' (%s..%s)",
+		 cmd->ref_name, cmd_oldh, cmd_newh,
+		 dst_cmd->ref_name, dst_oldh, dst_newh);
+
+	cmd->error_string = dst_cmd->error_string =
+		"inconsistent aliased update";
+}
+
+static void check_aliased_updates(struct command *commands)
+{
+	struct command *cmd;
+	struct string_list ref_list = { NULL, 0, 0, 0 };
+
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		struct string_list_item *item =
+			string_list_append(cmd->ref_name, &ref_list);
+		item->util = (void *)cmd;
+	}
+	sort_string_list(&ref_list);
+
+	for (cmd = commands; cmd; cmd = cmd->next)
+		check_aliased_update(cmd, &ref_list);
+
+	string_list_clear(&ref_list, 0);
+}
+
 static void execute_commands(struct command *commands, const char *unpacker_error)
 {
 	struct command *cmd;
@@ -503,10 +562,13 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
+	check_aliased_updates(commands);
+
 	head_name = resolve_ref("HEAD", sha1, 0, NULL);
 
 	for (cmd = commands; cmd; cmd = cmd->next)
-		cmd->error_string = update(cmd);
+		if (!cmd->skip_update)
+			cmd->error_string = update(cmd);
 }
 
 static struct command *read_head_info(void)
@@ -541,12 +603,10 @@ static struct command *read_head_info(void)
 			if (strstr(refname + reflen + 1, "side-band-64k"))
 				use_sideband = LARGE_PACKET_MAX;
 		}
-		cmd = xmalloc(sizeof(struct command) + len - 80);
+		cmd = xcalloc(1, sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
 		hashcpy(cmd->new_sha1, new_sha1);
 		memcpy(cmd->ref_name, line + 82, len - 81);
-		cmd->error_string = NULL;
-		cmd->next = NULL;
 		*p = cmd;
 		p = &cmd->next;
 	}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 3148789..f0813e0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -683,4 +683,53 @@ test_expect_success 'push with branches containing #' '
 	git checkout master
 '
 
+test_expect_success 'push into aliased refs (consistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(
+		cd child2 &&
+		>path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		git branch bar &&
+		git push ../child1 foo bar
+	)
+'
+
+test_expect_success 'push into aliased refs (inconsistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(
+		cd child2 &&
+		>path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		>path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch bar &&
+		test_must_fail git push ../child1 foo bar 2>stderr &&
+		grep "refusing inconsistent update" stderr
+	)
+'
+
 test_done
-- 
1.7.0.3.436.g2b878

[PATCH v3 3/3] receive-pack: detect aliased updates which can occur with symrefs

From: Jay Soffian <hidden>
Date: 2016-06-15 22:48:40 

When pushing to a remote repo the sending side filters out aliased
updates (e.g., foo:baz bar:baz). However, it is not possible for the
sender to know if two refs are aliased on the receiving side via
symrefs. Here is one such scenario:

  $ git init origin
  $ (cd origin && touch file && git add file && git commit -a -m intial)
  $ git clone --bare origin origin.git
  $ rm -rf origin

  $ git clone origin.git client

  $ git clone --mirror client backup.git &&
  $ (cd backup.git && git remote set-head origin --auto)

  $ (cd client &&
	git remote add --mirror backup ../backup.git &&
	echo change1 > file && git commit -a -m change1 &&
	git push origin &&
	git push backup
	)

The push to backup fails with:

  Counting objects: 5, done.
  Writing objects: 100% (3/3), 244 bytes, done.
  Total 3 (delta 0), reused 0 (delta 0)
  Unpacking objects: 100% (3/3), done.
  error: Ref refs/remotes/origin/master is at ef3... but expected 262...
  remote: error: failed to lock refs/remotes/origin/master
  To ../backup.git
     262cd57..ef307ff  master -> master
     262cd57..ef307ff  origin/HEAD -> origin/HEAD
   ! [remote rejected] origin/master -> origin/master (failed to lock)
  error: failed to push some refs to '../backup.git'

The reason is that refs/remotes/origin/HEAD is a symref to
refs/remotes/origin/master, but it is not possible for the sending side
to unambiguously know this.

This commit fixes the issue by having receive-pack ignore any update to
a symref whose target is being identically updated. If a symref and its
target are being updated inconsistently, then the update for both fails
with an error message ("refusing inconsistent update...") to help
diagnose the situation.

Signed-off-by: Jay Soffian <redacted>
---
 builtin-receive-pack.c |   68 +++++++++++++++++++++++++++++++++++++++++++++---
 t/t5516-fetch-push.sh  |   49 ++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index fffb6ea..bb34757 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -9,6 +9,7 @@
 #include "object.h"
 #include "remote.h"
 #include "transport.h"
+#include "string-list.h"
 
 static const char receive_pack_usage[] = "git receive-pack <git-dir>";
 
@@ -129,6 +130,7 @@ static void write_head_info(void)
 struct command {
 	struct command *next;
 	const char *error_string;
+	unsigned int skip_update;
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
 	char ref_name[FLEX_ARRAY]; /* more */
@@ -486,6 +488,63 @@ static void run_update_post_hook(struct command *commands)
 	}
 }
 
+static void check_aliased_update(struct command *cmd, struct string_list *list)
+{
+	struct string_list_item *item;
+	struct command *dst_cmd;
+	unsigned char sha1[20];
+	char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
+	int flag;
+
+	const char *dst_name = resolve_ref(cmd->ref_name, sha1, 0, &flag);
+
+	if (!(flag & REF_ISSYMREF))
+		return;
+
+	if ((item = string_list_lookup(dst_name, list)) == NULL)
+		return;
+
+	cmd->skip_update = 1;
+
+	dst_cmd = (struct command *) item->util;
+
+	if (!hashcmp(cmd->old_sha1, dst_cmd->old_sha1) &&
+	    !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
+		return;
+
+	dst_cmd->skip_update = 1;
+
+	strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
+	strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
+	strcat(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+	rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
+		 " its target '%s' (%s..%s)",
+		 cmd->ref_name, cmd_oldh, cmd_newh,
+		 dst_cmd->ref_name, dst_oldh, dst_newh);
+
+	cmd->error_string = dst_cmd->error_string =
+		"inconsistent aliased update";
+}
+
+static void check_aliased_updates(struct command *commands)
+{
+	struct command *cmd;
+	struct string_list ref_list = { NULL, 0, 0, 0 };
+
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		struct string_list_item *item =
+			string_list_append(cmd->ref_name, &ref_list);
+		item->util = (void *)cmd;
+	}
+	sort_string_list(&ref_list);
+
+	for (cmd = commands; cmd; cmd = cmd->next)
+		check_aliased_update(cmd, &ref_list);
+
+	string_list_clear(&ref_list, 0);
+}
+
 static void execute_commands(struct command *commands, const char *unpacker_error)
 {
 	struct command *cmd;
@@ -503,10 +562,13 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
+	check_aliased_updates(commands);
+
 	head_name = resolve_ref("HEAD", sha1, 0, NULL);
 
 	for (cmd = commands; cmd; cmd = cmd->next)
-		cmd->error_string = update(cmd);
+		if (!cmd->skip_update)
+			cmd->error_string = update(cmd);
 }
 
 static struct command *read_head_info(void)
@@ -541,12 +603,10 @@ static struct command *read_head_info(void)
 			if (strstr(refname + reflen + 1, "side-band-64k"))
 				use_sideband = LARGE_PACKET_MAX;
 		}
-		cmd = xmalloc(sizeof(struct command) + len - 80);
+		cmd = xcalloc(1, sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
 		hashcpy(cmd->new_sha1, new_sha1);
 		memcpy(cmd->ref_name, line + 82, len - 81);
-		cmd->error_string = NULL;
-		cmd->next = NULL;
 		*p = cmd;
 		p = &cmd->next;
 	}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 3148789..f0813e0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -683,4 +683,53 @@ test_expect_success 'push with branches containing #' '
 	git checkout master
 '
 
+test_expect_success 'push into aliased refs (consistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(
+		cd child2 &&
+		>path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		git branch bar &&
+		git push ../child1 foo bar
+	)
+'
+
+test_expect_success 'push into aliased refs (inconsistent)' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git branch foo &&
+		git symbolic-ref refs/heads/bar refs/heads/foo
+		git config receive.denyCurrentBranch false
+	) &&
+	(
+		cd child2 &&
+		>path2 &&
+		git add path2 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch foo &&
+		>path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m child2 &&
+		git branch bar &&
+		test_must_fail git push ../child1 foo bar 2>stderr &&
+		grep "refusing inconsistent update" stderr
+	)
+'
+
 test_done
-- 
1.7.0.3.436.g2b878

Re: [PATCH v3 3/3] receive-pack: detect aliased updates which can occur with symrefs

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2016-06-15 22:48:57 

On Mon, Apr 19, 2010 at 22:19, Jay Soffian [off-list ref] wrote:
quoted hunk
When pushing to a remote repo the sending side filters out aliased
updates (e.g., foo:baz bar:baz). However, it is not possible for the
sender to know if two refs are aliased on the receiving side via
symrefs. Here is one such scenario:

 $ git init origin
 $ (cd origin && touch file && git add file && git commit -a -m intial)
 $ git clone --bare origin origin.git
 $ rm -rf origin

 $ git clone origin.git client

 $ git clone --mirror client backup.git &&
 $ (cd backup.git && git remote set-head origin --auto)

 $ (cd client &&
       git remote add --mirror backup ../backup.git &&
       echo change1 > file && git commit -a -m change1 &&
       git push origin &&
       git push backup
       )

The push to backup fails with:

 Counting objects: 5, done.
 Writing objects: 100% (3/3), 244 bytes, done.
 Total 3 (delta 0), reused 0 (delta 0)
 Unpacking objects: 100% (3/3), done.
 error: Ref refs/remotes/origin/master is at ef3... but expected 262...
 remote: error: failed to lock refs/remotes/origin/master
 To ../backup.git
    262cd57..ef307ff  master -> master
    262cd57..ef307ff  origin/HEAD -> origin/HEAD
  ! [remote rejected] origin/master -> origin/master (failed to lock)
 error: failed to push some refs to '../backup.git'

The reason is that refs/remotes/origin/HEAD is a symref to
refs/remotes/origin/master, but it is not possible for the sending side
to unambiguously know this.

This commit fixes the issue by having receive-pack ignore any update to
a symref whose target is being identically updated. If a symref and its
target are being updated inconsistently, then the update for both fails
with an error message ("refusing inconsistent update...") to help
diagnose the situation.

Signed-off-by: Jay Soffian <redacted>
---
 builtin-receive-pack.c |   68 +++++++++++++++++++++++++++++++++++++++++++++---
 t/t5516-fetch-push.sh  |   49 ++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index fffb6ea..bb34757 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -9,6 +9,7 @@
 #include "object.h"
 #include "remote.h"
 #include "transport.h"
+#include "string-list.h"

 static const char receive_pack_usage[] = "git receive-pack <git-dir>";

@@ -129,6 +130,7 @@ static void write_head_info(void)
 struct command {
       struct command *next;
       const char *error_string;
+       unsigned int skip_update;
       unsigned char old_sha1[20];
       unsigned char new_sha1[20];
       char ref_name[FLEX_ARRAY]; /* more */
@@ -486,6 +488,63 @@ static void run_update_post_hook(struct command *commands)
       }
 }

+static void check_aliased_update(struct command *cmd, struct string_list *list)
+{
+       struct string_list_item *item;
+       struct command *dst_cmd;
+       unsigned char sha1[20];
+       char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
+       int flag;
+
+       const char *dst_name = resolve_ref(cmd->ref_name, sha1, 0, &flag);
+
+       if (!(flag & REF_ISSYMREF))
+               return;
+
+       if ((item = string_list_lookup(dst_name, list)) == NULL)
+               return;
+
+       cmd->skip_update = 1;
+
+       dst_cmd = (struct command *) item->util;
+
+       if (!hashcmp(cmd->old_sha1, dst_cmd->old_sha1) &&
+           !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
+               return;
+
+       dst_cmd->skip_update = 1;
+
+       strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
+       strcat(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
+       strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
+       strcat(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+       rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
+                " its target '%s' (%s..%s)",
+                cmd->ref_name, cmd_oldh, cmd_newh,
+                dst_cmd->ref_name, dst_oldh, dst_newh);
+
+       cmd->error_string = dst_cmd->error_string =
+               "inconsistent aliased update";
+}
+
+static void check_aliased_updates(struct command *commands)
+{
+       struct command *cmd;
+       struct string_list ref_list = { NULL, 0, 0, 0 };
+
+       for (cmd = commands; cmd; cmd = cmd->next) {
+               struct string_list_item *item =
+                       string_list_append(cmd->ref_name, &ref_list);
+               item->util = (void *)cmd;
+       }
+       sort_string_list(&ref_list);
+
+       for (cmd = commands; cmd; cmd = cmd->next)
+               check_aliased_update(cmd, &ref_list);
+
+       string_list_clear(&ref_list, 0);
+}
+
 static void execute_commands(struct command *commands, const char *unpacker_error)
 {
       struct command *cmd;
@@ -503,10 +562,13 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
               return;
       }

+       check_aliased_updates(commands);
+
       head_name = resolve_ref("HEAD", sha1, 0, NULL);

       for (cmd = commands; cmd; cmd = cmd->next)
-               cmd->error_string = update(cmd);
+               if (!cmd->skip_update)
+                       cmd->error_string = update(cmd);
 }

 static struct command *read_head_info(void)
@@ -541,12 +603,10 @@ static struct command *read_head_info(void)
                       if (strstr(refname + reflen + 1, "side-band-64k"))
                               use_sideband = LARGE_PACKET_MAX;
               }
-               cmd = xmalloc(sizeof(struct command) + len - 80);
+               cmd = xcalloc(1, sizeof(struct command) + len - 80);
               hashcpy(cmd->old_sha1, old_sha1);
               hashcpy(cmd->new_sha1, new_sha1);
               memcpy(cmd->ref_name, line + 82, len - 81);
-               cmd->error_string = NULL;
-               cmd->next = NULL;
               *p = cmd;
               p = &cmd->next;
       }
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 3148789..f0813e0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -683,4 +683,53 @@ test_expect_success 'push with branches containing #' '
       git checkout master
 '

+test_expect_success 'push into aliased refs (consistent)' '
+       mk_test heads/master &&
+       mk_child child1 &&
+       mk_child child2 &&
+       (
+               cd child1 &&
+               git branch foo &&
+               git symbolic-ref refs/heads/bar refs/heads/foo
+               git config receive.denyCurrentBranch false
+       ) &&
+       (
+               cd child2 &&
+               >path2 &&
+               git add path2 &&
+               test_tick &&
+               git commit -a -m child2 &&
+               git branch foo &&
+               git branch bar &&
+               git push ../child1 foo bar
+       )
+'
+
+test_expect_success 'push into aliased refs (inconsistent)' '
+       mk_test heads/master &&
+       mk_child child1 &&
+       mk_child child2 &&
+       (
+               cd child1 &&
+               git branch foo &&
+               git symbolic-ref refs/heads/bar refs/heads/foo
+               git config receive.denyCurrentBranch false
+       ) &&
+       (
+               cd child2 &&
+               >path2 &&
+               git add path2 &&
+               test_tick &&
+               git commit -a -m child2 &&
+               git branch foo &&
+               >path3 &&
+               git add path3 &&
+               test_tick &&
+               git commit -a -m child2 &&
+               git branch bar &&
+               test_must_fail git push ../child1 foo bar 2>stderr &&
+               grep "refusing inconsistent update" stderr
+       )
+'
+
 test_done
--
1.7.0.3.436.g2b878
I've been having troubles running t5516-fetch-push.sh which I traced
down to this patch, here's the gist of it:

    $ make
    rm -f -r test-results
    make aggregate-results-and-cleanup
    make[1]: Entering directory `/home/avar/g/git/t'
    echo "*** t5516-fetch-push.sh ***"; GIT_CONFIG=.git/config
'/bin/sh' t5516-fetch-push.sh
    *** t5516-fetch-push.sh ***
    *   ok 1: setup
    *   ok 2: fetch without wildcard
    *   ok 3: fetch with wildcard
    *   ok 4: fetch with insteadOf
    *   ok 5: fetch with pushInsteadOf (should not rewrite)
    *   ok 6: push without wildcard
    *   ok 7: push with wildcard
    *   ok 8: push with insteadOf
    *   ok 9: push with pushInsteadOf
    *   ok 10: push with pushInsteadOf and explicit pushurl
(pushInsteadOf should not rewrite)
    *   ok 11: push with matching heads
    *   ok 12: push with matching heads on the command line
    *   ok 13: failed (non-fast-forward) push with matching heads
    *   ok 14: push --force with matching heads
    *   ok 15: push with matching heads and forced update
    *   ok 16: push with no ambiguity (1)
    *   ok 17: push with no ambiguity (2)
    *   ok 18: push with colon-less refspec, no ambiguity
    *   ok 19: push with weak ambiguity (1)
    *   ok 20: push with weak ambiguity (2)
    *   ok 21: push with ambiguity
    *   ok 22: push with colon-less refspec (1)
    *   ok 23: push with colon-less refspec (2)
    *   ok 24: push with colon-less refspec (3)
    *   ok 25: push with colon-less refspec (4)
    *   ok 26: push head with non-existant, incomplete dest
    *   ok 27: push tag with non-existant, incomplete dest
    *   ok 28: push sha1 with non-existant, incomplete dest
    *   ok 29: push ref expression with non-existant, incomplete dest
    *   ok 30: push with HEAD
    *   ok 31: push with HEAD nonexisting at remote
    *   ok 32: push with +HEAD
    *   ok 33: push HEAD with non-existant, incomplete dest
    *   ok 34: push with config remote.*.push = HEAD
    *   ok 35: push with config remote.*.pushurl
    *   ok 36: push with dry-run
    *   ok 37: push updates local refs
    *   ok 38: push updates up-to-date local refs
    *   ok 39: push preserves up-to-date packed refs
    *   ok 40: push does not update local refs on failure
    *   ok 41: allow deleting an invalid remote ref
    *   ok 42: allow deleting a ref using --delete
    *   ok 43: allow deleting a tag using --delete
    *   ok 44: push --delete without args aborts
    *   ok 45: push --delete refuses src:dest refspecs
    *   ok 46: warn on push to HEAD of non-bare repository
    *   ok 47: deny push to HEAD of non-bare repository
    *   ok 48: allow push to HEAD of bare repository (bare)
    *   ok 49: allow push to HEAD of non-bare repository (config)
    *   ok 50: fetch with branches
    *   ok 51: fetch with branches containing #
    *   ok 52: push with branches
    *   ok 53: push with branches containing #
    *   ok 54: push into aliased refs (consistent)
    *** buffer overflow detected ***: receive-pack terminated
    ======= Backtrace: =========
    /lib/tls/i686/cmov/libc.so.6(__fortify_fail+0x50)[0x402a0350]
    /lib/tls/i686/cmov/libc.so.6(+0xe128a)[0x4029f28a]
    /lib/tls/i686/cmov/libc.so.6(+0xe05ba)[0x4029e5ba]
    receive-pack[0x8094de8]
    receive-pack[0x804b8ab]
    receive-pack[0x804be73]
    /lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe6)[0x401d4bd6]
    receive-pack[0x804b211]
    ======= Memory map: ========
    08048000-0813e000 r-xp 00000000 08:02 6792494
/home/avar/g/git/git-receive-pack
    0813e000-0813f000 r--p 000f6000 08:02 6792494
/home/avar/g/git/git-receive-pack
    0813f000-08143000 rw-p 000f7000 08:02 6792494
/home/avar/g/git/git-receive-pack
    08143000-08189000 rw-p 00000000 00:00 0
    0996b000-0998c000 rw-p 00000000 00:00 0          [heap]
    40000000-4001b000 r-xp 00000000 08:02 5496926    /lib/ld-2.11.1.so
    4001b000-4001c000 r--p 0001a000 08:02 5496926    /lib/ld-2.11.1.so
    4001c000-4001d000 rw-p 0001b000 08:02 5496926    /lib/ld-2.11.1.so
    4001d000-4001e000 r-xp 00000000 00:00 0          [vdso]
    4001e000-40020000 rw-p 00000000 00:00 0
    4003d000-40050000 r-xp 00000000 08:02 5497495    /lib/libz.so.1.2.3.3
    40050000-40051000 r--p 00012000 08:02 5497495    /lib/libz.so.1.2.3.3
    40051000-40052000 rw-p 00013000 08:02 5497495    /lib/libz.so.1.2.3.3
    40052000-4018a000 r-xp 00000000 08:02 4431990
/lib/i686/cmov/libcrypto.so.0.9.8
    4018a000-40192000 r--p 00137000 08:02 4431990
/lib/i686/cmov/libcrypto.so.0.9.8
    40192000-401a0000 rw-p 0013f000 08:02 4431990
/lib/i686/cmov/libcrypto.so.0.9.8
    401a0000-401a5000 rw-p 00000000 00:00 0
    401a5000-401ba000 r-xp 00000000 08:02 7234200
/lib/tls/i686/cmov/libpthread-2.11.1.so
    401ba000-401bb000 r--p 00014000 08:02 7234200
/lib/tls/i686/cmov/libpthread-2.11.1.so
    401bb000-401bc000 rw-p 00015000 08:02 7234200
/lib/tls/i686/cmov/libpthread-2.11.1.so
    401bc000-401be000 rw-p 00000000 00:00 0
    401be000-40311000 r-xp 00000000 08:02 7233919
/lib/tls/i686/cmov/libc-2.11.1.so
    40311000-40312000 ---p 00153000 08:02 7233919
/lib/tls/i686/cmov/libc-2.11.1.so
    40312000-40314000 r--p 00153000 08:02 7233919
/lib/tls/i686/cmov/libc-2.11.1.so
    40314000-40315000 rw-p 00155000 08:02 7233919
/lib/tls/i686/cmov/libc-2.11.1.so
    40315000-40318000 rw-p 00000000 00:00 0
    40318000-4031a000 r-xp 00000000 08:02 7233925
/lib/tls/i686/cmov/libdl-2.11.1.so
    4031a000-4031b000 r--p 00001000 08:02 7233925
/lib/tls/i686/cmov/libdl-2.11.1.so
    4031b000-4031c000 rw-p 00002000 08:02 7233925
/lib/tls/i686/cmov/libdl-2.11.1.so
    4031c000-4031d000 rw-p 00000000 00:00 0
    4031d000-4033a000 r-xp 00000000 08:02 7217157    /lib/libgcc_s.so.1
    4033a000-4033b000 r--p 0001c000 08:02 7217157    /lib/libgcc_s.so.1
    4033b000-4033c000 rw-p 0001d000 08:02 7217157    /lib/libgcc_s.so.1
    bf9b1000-bf9c6000 rw-p 00000000 00:00 0          [stack]
    * FAIL 55: push into aliased refs (inconsistent)
    	
    		mk_test heads/master &&
    		mk_child child1 &&
    		mk_child child2 &&
    		(
    			cd child1 &&
    			git branch foo &&
    			git symbolic-ref refs/heads/bar refs/heads/foo
    			git config receive.denyCurrentBranch false
    		) &&
    		(
    			cd child2 &&
    			>path2 &&
    			git add path2 &&
    			test_tick &&
    			git commit -a -m child2 &&
    			git branch foo &&
    			>path3 &&
    			git add path3 &&
    			test_tick &&
    			git commit -a -m child2 &&
    			git branch bar &&
    			test_must_fail git push ../child1 foo bar 2>stderr &&
    			grep "refusing inconsistent update" stderr
    		)
    	
    *   ok 56: push --porcelain
    *   ok 57: push --porcelain bad url
    *   ok 58: push --porcelain rejected
    *   ok 59: push --porcelain --dry-run rejected
    * failed 1 among 59 test(s)
    make[1]: *** [t5516-fetch-push.sh] Error 1
    make[1]: Leaving directory `/home/avar/g/git/t'
    make: *** [all] Error 2


And the bisect script/log from 1.7.0 to master:

    #!/bin/sh
    cd ~/g/git
    git reset --hard >/dev/null
    git clean -dxf >/dev/null

    make >/dev/null 2>&1
    cp -v /tmp/Makefile t/Makefile
    make test
    ret=$?

    git reset --hard >/dev/null
    git clean -dxf  >/dev/null

    exit $ret

log:

    $ git bisect log
    git bisect start
    # bad: [92a75a391e66cfc278cf59741e484efd80c02176] Merge branch 'maint'
    git bisect bad 92a75a391e66cfc278cf59741e484efd80c02176
    # good: [e923eaeb901ff056421b9007adcbbce271caa7b6] Git 1.7.0
    git bisect good e923eaeb901ff056421b9007adcbbce271caa7b6
    # good: [419fe5bc861517c789c8f028519e085fd8d1992f] fmt-merge-msg:
be quiet if nothing to merge
    git bisect good 419fe5bc861517c789c8f028519e085fd8d1992f
    # good: [b6b0afdc30e066788592ca07c9a6c6936c68cc11] test-lib: some
shells do not let $? propagate into an eval
    git bisect good b6b0afdc30e066788592ca07c9a6c6936c68cc11
    # good: [cd4ce1e8a81ef5c24af7b914fb72212273e7d489] Merge branch
'jc/status-show-ignored'
    git bisect good cd4ce1e8a81ef5c24af7b914fb72212273e7d489
    # bad: [71f1d729b39ce5c92df6d623151f88bbb5d4c774] Merge branch
'jn/gitweb-our-squelch'
    git bisect bad 71f1d729b39ce5c92df6d623151f88bbb5d4c774
    # good: [465ef577b59986d70c51ea72dd3f87759b2bcd4f] Merge branch
'jn/submodule-basic-test'
    git bisect good 465ef577b59986d70c51ea72dd3f87759b2bcd4f
    # bad: [9215f76fb6d938ae93889f46f27cff22723fe0e4] Merge branch
'js/maint-receive-pack-symref-alias'
    git bisect bad 9215f76fb6d938ae93889f46f27cff22723fe0e4
    # good: [9b0aa728705439ca4b4e7ec845f79f8487059320] Extract
verify_pack_index for reuse from verify_pack
    git bisect good 9b0aa728705439ca4b4e7ec845f79f8487059320
    # good: [90d05713575ea6ed21d05228bcda8461f7b28ccf]
http.c::new_http_pack_request: do away with the temp variable filename
    git bisect good 90d05713575ea6ed21d05228bcda8461f7b28ccf
    # bad: [da3efdb17bef25dedc753131462ee784d822132e] receive-pack:
detect aliased updates which can occur with symrefs
    git bisect bad da3efdb17bef25dedc753131462ee784d822132e
    # good: [5e1c71fd1488a33680c313f287b88d9f7a7d3e45] receive-pack:
switch global variable 'commands' to a parameter
    git bisect good 5e1c71fd1488a33680c313f287b88d9f7a7d3e45

What the t/Makefile patch does is this:

    diff --git a/t/Makefile b/t/Makefile
    index 25c559b..da0d5bc 100644
    --- a/t/Makefile
    +++ b/t/Makefile
    @@ -13,7 +13,7 @@ RM ?= rm -f
     # Shell quote;
     SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))

    -T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
    +T = $(wildcard t[5][5][1][6]-*.sh)
     TSVN = $(wildcard t91[0-9][0-9]-*.sh)

The reason is that I can't reproduce this when running just the
script, it has to be through make test, which suggests some sort of
evil heisenbug. Even running the exact same commands the Makefile
would run to execute the script doesn't work.

I also ran the tests under valgrind, here's the full output:
http://gist.github.com/433379
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help