Skip to content

Commit cdc2b2f

Browse files
gitstergitster
committed
Merge branch 'ph/push-to-delete-nothing'
* ph/push-to-delete-nothing: receive-pack: don't pass non-existent refs to post-{receive,update} hooks Conflicts: builtin/receive-pack.c
2 parents 66d2c22 + 160b81e commit cdc2b2f

File tree

2 files changed

+215
-8
lines changed

2 files changed

+215
-8
lines changed

‎builtin/receive-pack.c

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ static void write_head_info(void)
154154
struct command {
155155
struct command *next;
156156
const char *error_string;
157-
unsigned int skip_update;
157+
unsigned int skip_update:1,
158+
did_not_exist:1;
158159
unsigned char old_sha1[20];
159160
unsigned char new_sha1[20];
160161
char ref_name[FLEX_ARRAY]; /* more */
@@ -264,6 +265,7 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta
264265

265266
struct receive_hook_feed_state {
266267
struct command *cmd;
268+
int skip_broken;
267269
struct strbuf buf;
268270
};
269271

@@ -272,7 +274,8 @@ static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
272274
struct receive_hook_feed_state *state = state_;
273275
struct command *cmd = state->cmd;
274276

275-
while (cmd && cmd->error_string)
277+
while (cmd &&
278+
state->skip_broken && (cmd->error_string || cmd->did_not_exist))
276279
cmd = cmd->next;
277280
if (!cmd)
278281
return -1; /* EOF */
@@ -288,13 +291,15 @@ static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
288291
return 0;
289292
}
290293

291-
static int run_receive_hook(struct command *commands, const char *hook_name)
294+
static int run_receive_hook(struct command *commands, const char *hook_name,
295+
int skip_broken)
292296
{
293297
struct receive_hook_feed_state state;
294298
int status;
295299

296300
strbuf_init(&state.buf, 0);
297301
state.cmd = commands;
302+
state.skip_broken = skip_broken;
298303
if (feed_receive_hook(&state, NULL, NULL))
299304
return 0;
300305
state.cmd = commands;
@@ -485,8 +490,13 @@ static const char *update(struct command *cmd)
485490

486491
if (is_null_sha1(new_sha1)) {
487492
if (!parse_object(old_sha1)) {
488-
rp_warning("Allowing deletion of corrupt ref.");
489493
old_sha1 = NULL;
494+
if (ref_exists(name)) {
495+
rp_warning("Allowing deletion of corrupt ref.");
496+
} else {
497+
rp_warning("Deleting a non-existent ref.");
498+
cmd->did_not_exist = 1;
499+
}
490500
}
491501
if (delete_ref(namespaced_name, old_sha1, 0)) {
492502
rp_error("failed to delete %s", name);
@@ -517,7 +527,7 @@ static void run_update_post_hook(struct command *commands)
517527
struct child_process proc;
518528

519529
for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
520-
if (cmd->error_string)
530+
if (cmd->error_string || cmd->did_not_exist)
521531
continue;
522532
argc++;
523533
}
@@ -528,7 +538,7 @@ static void run_update_post_hook(struct command *commands)
528538

529539
for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
530540
char *p;
531-
if (cmd->error_string)
541+
if (cmd->error_string || cmd->did_not_exist)
532542
continue;
533543
p = xmalloc(strlen(cmd->ref_name) + 1);
534544
strcpy(p, cmd->ref_name);
@@ -672,7 +682,7 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
672682
0, &cmd))
673683
set_connectivity_errors(commands);
674684

675-
if (run_receive_hook(commands, pre_receive_hook)) {
685+
if (run_receive_hook(commands, pre_receive_hook, 0)) {
676686
for (cmd = commands; cmd; cmd = cmd->next)
677687
cmd->error_string = "pre-receive hook declined";
678688
return;
@@ -940,7 +950,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
940950
unlink_or_warn(pack_lockfile);
941951
if (report_status)
942952
report(commands, unpack_status);
943-
run_receive_hook(commands, post_receive_hook);
953+
run_receive_hook(commands, post_receive_hook, 1);
944954
run_update_post_hook(commands);
945955
if (auto_gc) {
946956
const char *argv_gc_auto[] = {

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