Skip to content

Commit 160b81e

Browse files
yanhangitster
yanhan
authored and
gitster
committed
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 85e9c7e commit 160b81e

File tree

4 files changed

+211
-8
lines changed

4 files changed

+211
-8
lines changed

‎builtin/receive-pack.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ static void write_head_info(void)
147147
struct command {
148148
struct command *next;
149149
const char *error_string;
150-
unsigned int skip_update;
150+
unsigned int skip_update:1,
151+
did_not_exist:1;
151152
unsigned char old_sha1[20];
152153
unsigned char new_sha1[20];
153154
char ref_name[FLEX_ARRAY]; /* more */
@@ -215,7 +216,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
215216
int have_input = 0, code;
216217

217218
for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
218-
if (!cmd->error_string)
219+
if (!cmd->error_string && !cmd->did_not_exist)
219220
have_input = 1;
220221
}
221222

@@ -248,7 +249,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
248249
}
249250

250251
for (cmd = commands; cmd; cmd = cmd->next) {
251-
if (!cmd->error_string) {
252+
if (!cmd->error_string && !cmd->did_not_exist) {
252253
size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
253254
sha1_to_hex(cmd->old_sha1),
254255
sha1_to_hex(cmd->new_sha1),
@@ -445,8 +446,13 @@ static const char *update(struct command *cmd)
445446

446447
if (is_null_sha1(new_sha1)) {
447448
if (!parse_object(old_sha1)) {
448-
rp_warning("Allowing deletion of corrupt ref.");
449449
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+
}
450456
}
451457
if (delete_ref(namespaced_name, old_sha1, 0)) {
452458
rp_error("failed to delete %s", name);
@@ -477,7 +483,7 @@ static void run_update_post_hook(struct command *commands)
477483
struct child_process proc;
478484

479485
for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
480-
if (cmd->error_string)
486+
if (cmd->error_string || cmd->did_not_exist)
481487
continue;
482488
argc++;
483489
}
@@ -488,7 +494,7 @@ static void run_update_post_hook(struct command *commands)
488494

489495
for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
490496
char *p;
491-
if (cmd->error_string)
497+
if (cmd->error_string || cmd->did_not_exist)
492498
continue;
493499
p = xmalloc(strlen(cmd->ref_name) + 1);
494500
strcpy(p, cmd->ref_name);

‎refs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1851,7 +1851,7 @@ int update_ref(const char *action, const char *refname,
18511851
return 0;
18521852
}
18531853

1854-
int ref_exists(char *refname)
1854+
int ref_exists(const char *refname)
18551855
{
18561856
unsigned char sha1[20];
18571857
return !!resolve_ref(refname, sha1, 1, NULL);

‎refs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
5757
*/
5858
extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
5959
extern void clear_extra_refs(void);
60-
extern int ref_exists(char *);
60+
extern int ref_exists(const char *);
6161

6262
extern int peel_ref(const char *, unsigned char *);
6363

‎t/t5516-fetch-push.sh

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,40 @@ mk_test () {
4040
)
4141
}
4242

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+
4377
mk_child() {
4478
rm -rf "$1" &&
4579
git clone testrepo "$1"
@@ -559,6 +593,169 @@ test_expect_success 'allow deleting an invalid remote ref' '
559593
560594
'
561595

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+
562759
test_expect_success 'allow deleting a ref using --delete' '
563760
mk_test heads/master &&
564761
(cd testrepo && git config receive.denyDeleteCurrent warn) &&

0 commit comments

Comments
 (0)