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>
Pang Yan Han committed
Sep 28, 2011 at 23:39 UTC
160b81ed819d870b7d098bf61b51c4bc959507f3
4 files changed
+211
-8
builtin/receive-pack.c
+12
-6
@@ -147,7 +147,8 @@ static void write_head_info(void)
147
struct command {
148
struct command *next;
149
const char *error_string;
150
- unsigned int skip_update;
150
+ unsigned int skip_update:1,
151
+ did_not_exist:1;
152
unsigned char old_sha1[20];
153
unsigned char new_sha1[20];
154
char ref_name[FLEX_ARRAY]; /* more */
@@ -215,7 +216,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
216
int have_input = 0, code;
217
218
for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
218
- if (!cmd->error_string)
219
+ if (!cmd->error_string && !cmd->did_not_exist)
220
have_input = 1;
221
}
222
@@ -248,7 +249,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
249
}
250
251
for (cmd = commands; cmd; cmd = cmd->next) {
251
- if (!cmd->error_string) {
252
+ if (!cmd->error_string && !cmd->did_not_exist) {
253
size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
254
sha1_to_hex(cmd->old_sha1),
255
sha1_to_hex(cmd->new_sha1),
@@ -445,8 +446,13 @@ static const char *update(struct command *cmd)
446
447
if (is_null_sha1(new_sha1)) {
448
if (!parse_object(old_sha1)) {
448
- rp_warning("Allowing deletion of corrupt ref.");
449
old_sha1 = NULL;
450
+ if (ref_exists(name)) {
451
+ rp_warning("Allowing deletion of corrupt ref.");
452
+ } else {
453
+ rp_warning("Deleting a non-existent ref.");
454
+ cmd->did_not_exist = 1;
455
+ }
456
}
457
if (delete_ref(namespaced_name, old_sha1, 0)) {
458
rp_error("failed to delete %s", name);
@@ -477,7 +483,7 @@ static void run_update_post_hook(struct command *commands)
483
struct child_process proc;
484
485
for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
480
- if (cmd->error_string)
486
+ if (cmd->error_string || cmd->did_not_exist)
487
continue;
488
argc++;
489
}
@@ -488,7 +494,7 @@ static void run_update_post_hook(struct command *commands)
494
495
for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
496
char *p;
491
- if (cmd->error_string)
497
+ if (cmd->error_string || cmd->did_not_exist)
498
continue;
499
p = xmalloc(strlen(cmd->ref_name) + 1);
500
strcpy(p, cmd->ref_name);
refs.c
+1
-1
@@ -1851,7 +1851,7 @@ int update_ref(const char *action, const char *refname,
1851
return 0;
1852
}
1853
1854
-int ref_exists(char *refname)
1854
+int ref_exists(const char *refname)
1855
{
1856
unsigned char sha1[20];
1857
return !!resolve_ref(refname, sha1, 1, NULL);
refs.h
+1
-1
@@ -57,7 +57,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
57
*/
58
extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
59
extern void clear_extra_refs(void);
60
-extern int ref_exists(char *);
60
+extern int ref_exists(const char *);
61
62
extern int peel_ref(const char *, unsigned char *);
63
t/t5516-fetch-push.sh
+197
@@ -40,6 +40,40 @@ mk_test () {
40
)
41
}
42
43
+mk_test_with_hooks() {
44
+ mk_test "$@" &&
45
+ (
46
+ cd testrepo &&
47
+ mkdir .git/hooks &&
48
+ cd .git/hooks &&
49
+
50
+ cat >pre-receive <<-'EOF' &&
51
+ #!/bin/sh
52
+ cat - >>pre-receive.actual
53
+ EOF
54
+
55
+ cat >update <<-'EOF' &&
56
+ #!/bin/sh
57
+ printf "%s %s %s\n" "$@" >>update.actual
58
+ EOF
59
+
60
+ cat >post-receive <<-'EOF' &&
61
+ #!/bin/sh
62
+ cat - >>post-receive.actual
63
+ EOF
64
+
65
+ cat >post-update <<-'EOF' &&
66
+ #!/bin/sh
67
+ for ref in "$@"
68
+ do
69
+ printf "%s\n" "$ref" >>post-update.actual
70
+ done
71
+ EOF
72
+
73
+ chmod +x pre-receive update post-receive post-update
74
+ )
75
+}
76
+
77
mk_child() {
78
rm -rf "$1" &&
79
git clone testrepo "$1"
@@ -559,6 +593,169 @@ test_expect_success 'allow deleting an invalid remote ref' '
593
594
'
595
596
+test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' '
597
+ mk_test_with_hooks heads/master heads/next &&
598
+ orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&
599
+ newmaster=$(git show-ref -s --verify refs/heads/master) &&
600
+ orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&
601
+ newnext=$_z40 &&
602
+ git push testrepo refs/heads/master:refs/heads/master :refs/heads/next &&
603
+ (
604
+ cd testrepo/.git &&
605
+ cat >pre-receive.expect <<-EOF &&
606
+ $orgmaster $newmaster refs/heads/master
607
+ $orgnext $newnext refs/heads/next
608
+ EOF
609
+
610
+ cat >update.expect <<-EOF &&
611
+ refs/heads/master $orgmaster $newmaster
612
+ refs/heads/next $orgnext $newnext
613
+ EOF
614
+
615
+ cat >post-receive.expect <<-EOF &&
616
+ $orgmaster $newmaster refs/heads/master
617
+ $orgnext $newnext refs/heads/next
618
+ EOF
619
+
620
+ cat >post-update.expect <<-EOF &&
621
+ refs/heads/master
622
+ refs/heads/next
623
+ EOF
624
+
625
+ test_cmp pre-receive.expect pre-receive.actual &&
626
+ test_cmp update.expect update.actual &&
627
+ test_cmp post-receive.expect post-receive.actual &&
628
+ test_cmp post-update.expect post-update.actual
629
+ )
630
+'
631
+
632
+test_expect_success 'deleting dangling ref triggers hooks with correct args' '
633
+ mk_test_with_hooks heads/master &&
634
+ rm -f testrepo/.git/objects/??/* &&
635
+ git push testrepo :refs/heads/master &&
636
+ (
637
+ cd testrepo/.git &&
638
+ cat >pre-receive.expect <<-EOF &&
639
+ $_z40 $_z40 refs/heads/master
640
+ EOF
641
+
642
+ cat >update.expect <<-EOF &&
643
+ refs/heads/master $_z40 $_z40
644
+ EOF
645
+
646
+ cat >post-receive.expect <<-EOF &&
647
+ $_z40 $_z40 refs/heads/master
648
+ EOF
649
+
650
+ cat >post-update.expect <<-EOF &&
651
+ refs/heads/master
652
+ EOF
653
+
654
+ test_cmp pre-receive.expect pre-receive.actual &&
655
+ test_cmp update.expect update.actual &&
656
+ test_cmp post-receive.expect post-receive.actual &&
657
+ test_cmp post-update.expect post-update.actual
658
+ )
659
+'
660
+
661
+test_expect_success 'deletion of a non-existent ref is not fed to post-receive and post-update hooks' '
662
+ mk_test_with_hooks heads/master &&
663
+ orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&
664
+ newmaster=$(git show-ref -s --verify refs/heads/master) &&
665
+ git push testrepo master :refs/heads/nonexistent &&
666
+ (
667
+ cd testrepo/.git &&
668
+ cat >pre-receive.expect <<-EOF &&
669
+ $orgmaster $newmaster refs/heads/master
670
+ $_z40 $_z40 refs/heads/nonexistent
671
+ EOF
672
+
673
+ cat >update.expect <<-EOF &&
674
+ refs/heads/master $orgmaster $newmaster
675
+ refs/heads/nonexistent $_z40 $_z40
676
+ EOF
677
+
678
+ cat >post-receive.expect <<-EOF &&
679
+ $orgmaster $newmaster refs/heads/master
680
+ EOF
681
+
682
+ cat >post-update.expect <<-EOF &&
683
+ refs/heads/master
684
+ EOF
685
+
686
+ test_cmp pre-receive.expect pre-receive.actual &&
687
+ test_cmp update.expect update.actual &&
688
+ test_cmp post-receive.expect post-receive.actual &&
689
+ test_cmp post-update.expect post-update.actual
690
+ )
691
+'
692
+
693
+test_expect_success 'deletion of a non-existent ref alone does trigger post-receive and post-update hooks' '
694
+ mk_test_with_hooks heads/master &&
695
+ git push testrepo :refs/heads/nonexistent &&
696
+ (
697
+ cd testrepo/.git &&
698
+ cat >pre-receive.expect <<-EOF &&
699
+ $_z40 $_z40 refs/heads/nonexistent
700
+ EOF
701
+
702
+ cat >update.expect <<-EOF &&
703
+ refs/heads/nonexistent $_z40 $_z40
704
+ EOF
705
+
706
+ test_cmp pre-receive.expect pre-receive.actual &&
707
+ test_cmp update.expect update.actual &&
708
+ test_path_is_missing post-receive.actual &&
709
+ test_path_is_missing post-update.actual
710
+ )
711
+'
712
+
713
+test_expect_success 'mixed ref updates, deletes, invalid deletes trigger hooks with correct input' '
714
+ mk_test_with_hooks heads/master heads/next heads/pu &&
715
+ orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&
716
+ newmaster=$(git show-ref -s --verify refs/heads/master) &&
717
+ orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&
718
+ newnext=$_z40 &&
719
+ orgpu=$(cd testrepo && git show-ref -s --verify refs/heads/pu) &&
720
+ newpu=$(git show-ref -s --verify refs/heads/master) &&
721
+ git push testrepo refs/heads/master:refs/heads/master \
722
+ refs/heads/master:refs/heads/pu :refs/heads/next \
723
+ :refs/heads/nonexistent &&
724
+ (
725
+ cd testrepo/.git &&
726
+ cat >pre-receive.expect <<-EOF &&
727
+ $orgmaster $newmaster refs/heads/master
728
+ $orgnext $newnext refs/heads/next
729
+ $orgpu $newpu refs/heads/pu
730
+ $_z40 $_z40 refs/heads/nonexistent
731
+ EOF
732
+
733
+ cat >update.expect <<-EOF &&
734
+ refs/heads/master $orgmaster $newmaster
735
+ refs/heads/next $orgnext $newnext
736
+ refs/heads/pu $orgpu $newpu
737
+ refs/heads/nonexistent $_z40 $_z40
738
+ EOF
739
+
740
+ cat >post-receive.expect <<-EOF &&
741
+ $orgmaster $newmaster refs/heads/master
742
+ $orgnext $newnext refs/heads/next
743
+ $orgpu $newpu refs/heads/pu
744
+ EOF
745
+
746
+ cat >post-update.expect <<-EOF &&
747
+ refs/heads/master
748
+ refs/heads/next
749
+ refs/heads/pu
750
+ EOF
751
+
752
+ test_cmp pre-receive.expect pre-receive.actual &&
753
+ test_cmp update.expect update.actual &&
754
+ test_cmp post-receive.expect post-receive.actual &&
755
+ test_cmp post-update.expect post-update.actual
756
+ )
757
+'
758
+
759
test_expect_success 'allow deleting a ref using --delete' '
760
mk_test heads/master &&
761
(cd testrepo && git config receive.denyDeleteCurrent warn) &&