receive-pack: don't pass non-existent refs to post-{receive,update} hooks

When a push specifies deletion of non-existent refs, the post post-receive and
post-update hooks receive them as input/arguments.

For instance, for the following push, where refs/heads/nonexistent is a ref
which does not exist on the remote side:

	git push origin :refs/heads/nonexistent

the post-receive hook receives from standard input:

	<null-sha1> SP <null-sha1> SP refs/heads/nonexistent

and the post-update hook receives as arguments:

	refs/heads/nonexistent

which does not make sense since it is a no-op.

Teach receive-pack not to pass non-existent refs to the post-receive and
post-update hooks. If the push only attempts to delete non-existent refs,
these hooks are not even called.

The update and pre-receive hooks are still notified about attempted
deletion of non-existent refs to give them a chance to inspect the
situation and act on it.

[jc: mild fix-ups to avoid introducing an extra list; also added fixes to
some tests]

Signed-off-by: Pang Yan Han <pangyanhan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 197 insertions, 0 deletions

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 3abb2907e..b69cf574d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -40,6 +40,40 @@ mk_test () {
 	)
 }
 
+mk_test_with_hooks() {
+	mk_test "$@" &&
+	(
+		cd testrepo &&
+		mkdir .git/hooks &&
+		cd .git/hooks &&
+
+		cat >pre-receive <<-'EOF' &&
+		#!/bin/sh
+		cat - >>pre-receive.actual
+		EOF
+
+		cat >update <<-'EOF' &&
+		#!/bin/sh
+		printf "%s %s %s\n" "$@" >>update.actual
+		EOF
+
+		cat >post-receive <<-'EOF' &&
+		#!/bin/sh
+		cat - >>post-receive.actual
+		EOF
+
+		cat >post-update <<-'EOF' &&
+		#!/bin/sh
+		for ref in "$@"
+		do
+			printf "%s\n" "$ref" >>post-update.actual
+		done
+		EOF
+
+		chmod +x pre-receive update post-receive post-update
+	)
+}
+
 mk_child() {
 	rm -rf "$1" &&
 	git clone testrepo "$1"
@@ -559,6 +593,169 @@ test_expect_success 'allow deleting an invalid remote ref' '
 
 '
 
+test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' '
+	mk_test_with_hooks heads/master heads/next &&
+	orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&
+	newmaster=$(git show-ref -s --verify refs/heads/master) &&
+	orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&
+	newnext=$_z40 &&
+	git push testrepo refs/heads/master:refs/heads/master :refs/heads/next &&
+	(
+		cd testrepo/.git &&
+		cat >pre-receive.expect <<-EOF &&
+		$orgmaster $newmaster refs/heads/master
+		$orgnext $newnext refs/heads/next
+		EOF
+
+		cat >update.expect <<-EOF &&
+		refs/heads/master $orgmaster $newmaster
+		refs/heads/next $orgnext $newnext
+		EOF
+
+		cat >post-receive.expect <<-EOF &&
+		$orgmaster $newmaster refs/heads/master
+		$orgnext $newnext refs/heads/next
+		EOF
+
+		cat >post-update.expect <<-EOF &&
+		refs/heads/master
+		refs/heads/next
+		EOF
+
+		test_cmp pre-receive.expect pre-receive.actual &&
+		test_cmp update.expect update.actual &&
+		test_cmp post-receive.expect post-receive.actual &&
+		test_cmp post-update.expect post-update.actual
+	)
+'
+
+test_expect_success 'deleting dangling ref triggers hooks with correct args' '
+	mk_test_with_hooks heads/master &&
+	rm -f testrepo/.git/objects/??/* &&
+	git push testrepo :refs/heads/master &&
+	(
+		cd testrepo/.git &&
+		cat >pre-receive.expect <<-EOF &&
+		$_z40 $_z40 refs/heads/master
+		EOF
+
+		cat >update.expect <<-EOF &&
+		refs/heads/master $_z40 $_z40
+		EOF
+
+		cat >post-receive.expect <<-EOF &&
+		$_z40 $_z40 refs/heads/master
+		EOF
+
+		cat >post-update.expect <<-EOF &&
+		refs/heads/master
+		EOF
+
+		test_cmp pre-receive.expect pre-receive.actual &&
+		test_cmp update.expect update.actual &&
+		test_cmp post-receive.expect post-receive.actual &&
+		test_cmp post-update.expect post-update.actual
+	)
+'
+
+test_expect_success 'deletion of a non-existent ref is not fed to post-receive and post-update hooks' '
+	mk_test_with_hooks heads/master &&
+	orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&
+	newmaster=$(git show-ref -s --verify refs/heads/master) &&
+	git push testrepo master :refs/heads/nonexistent &&
+	(
+		cd testrepo/.git &&
+		cat >pre-receive.expect <<-EOF &&
+		$orgmaster $newmaster refs/heads/master
+		$_z40 $_z40 refs/heads/nonexistent
+		EOF
+
+		cat >update.expect <<-EOF &&
+		refs/heads/master $orgmaster $newmaster
+		refs/heads/nonexistent $_z40 $_z40
+		EOF
+
+		cat >post-receive.expect <<-EOF &&
+		$orgmaster $newmaster refs/heads/master
+		EOF
+
+		cat >post-update.expect <<-EOF &&
+		refs/heads/master
+		EOF
+
+		test_cmp pre-receive.expect pre-receive.actual &&
+		test_cmp update.expect update.actual &&
+		test_cmp post-receive.expect post-receive.actual &&
+		test_cmp post-update.expect post-update.actual
+	)
+'
+
+test_expect_success 'deletion of a non-existent ref alone does trigger post-receive and post-update hooks' '
+	mk_test_with_hooks heads/master &&
+	git push testrepo :refs/heads/nonexistent &&
+	(
+		cd testrepo/.git &&
+		cat >pre-receive.expect <<-EOF &&
+		$_z40 $_z40 refs/heads/nonexistent
+		EOF
+
+		cat >update.expect <<-EOF &&
+		refs/heads/nonexistent $_z40 $_z40
+		EOF
+
+		test_cmp pre-receive.expect pre-receive.actual &&
+		test_cmp update.expect update.actual &&
+		test_path_is_missing post-receive.actual &&
+		test_path_is_missing post-update.actual
+	)
+'
+
+test_expect_success 'mixed ref updates, deletes, invalid deletes trigger hooks with correct input' '
+	mk_test_with_hooks heads/master heads/next heads/pu &&
+	orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&
+	newmaster=$(git show-ref -s --verify refs/heads/master) &&
+	orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&
+	newnext=$_z40 &&
+	orgpu=$(cd testrepo && git show-ref -s --verify refs/heads/pu) &&
+	newpu=$(git show-ref -s --verify refs/heads/master) &&
+	git push testrepo refs/heads/master:refs/heads/master \
+	    refs/heads/master:refs/heads/pu :refs/heads/next \
+	    :refs/heads/nonexistent &&
+	(
+		cd testrepo/.git &&
+		cat >pre-receive.expect <<-EOF &&
+		$orgmaster $newmaster refs/heads/master
+		$orgnext $newnext refs/heads/next
+		$orgpu $newpu refs/heads/pu
+		$_z40 $_z40 refs/heads/nonexistent
+		EOF
+
+		cat >update.expect <<-EOF &&
+		refs/heads/master $orgmaster $newmaster
+		refs/heads/next $orgnext $newnext
+		refs/heads/pu $orgpu $newpu
+		refs/heads/nonexistent $_z40 $_z40
+		EOF
+
+		cat >post-receive.expect <<-EOF &&
+		$orgmaster $newmaster refs/heads/master
+		$orgnext $newnext refs/heads/next
+		$orgpu $newpu refs/heads/pu
+		EOF
+
+		cat >post-update.expect <<-EOF &&
+		refs/heads/master
+		refs/heads/next
+		refs/heads/pu
+		EOF
+
+		test_cmp pre-receive.expect pre-receive.actual &&
+		test_cmp update.expect update.actual &&
+		test_cmp post-receive.expect post-receive.actual &&
+		test_cmp post-update.expect post-update.actual
+	)
+'
+
 test_expect_success 'allow deleting a ref using --delete' '
 	mk_test heads/master &&
 	(cd testrepo && git config receive.denyDeleteCurrent warn) &&