Skip to content

Commit

Permalink
receive-pack: don't pass non-existent refs to post-{receive,update} h…
Browse files Browse the repository at this point in the history 
…ooks

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>
  • Loading branch information
Pang Yan Han authored and Junio C Hamano committed Sep 30, 2011
1 parent 85e9c7e commit 160b81e
Show file tree
Hide file tree
Showing 4 changed files with 211 additions and 8 deletions.
18 changes: 12 additions & 6 deletions builtin/receive-pack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ static void write_head_info(void)
struct command {
struct command *next;
const char *error_string;
unsigned int skip_update;
unsigned int skip_update:1,
did_not_exist:1;
unsigned char old_sha1[20];
unsigned char new_sha1[20];
char ref_name[FLEX_ARRAY]; /* more */
Expand Down Expand Up @@ -215,7 +216,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
int have_input = 0, code;

for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
if (!cmd->error_string)
if (!cmd->error_string && !cmd->did_not_exist)
have_input = 1;
}

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

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

if (is_null_sha1(new_sha1)) {
if (!parse_object(old_sha1)) {
rp_warning("Allowing deletion of corrupt ref.");
old_sha1 = NULL;
if (ref_exists(name)) {
rp_warning("Allowing deletion of corrupt ref.");
} else {
rp_warning("Deleting a non-existent ref.");
cmd->did_not_exist = 1;
}
}
if (delete_ref(namespaced_name, old_sha1, 0)) {
rp_error("failed to delete %s", name);
Expand Down Expand Up @@ -477,7 +483,7 @@ static void run_update_post_hook(struct command *commands)
struct child_process proc;

for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
if (cmd->error_string)
if (cmd->error_string || cmd->did_not_exist)
continue;
argc++;
}
Expand All @@ -488,7 +494,7 @@ static void run_update_post_hook(struct command *commands)

for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
char *p;
if (cmd->error_string)
if (cmd->error_string || cmd->did_not_exist)
continue;
p = xmalloc(strlen(cmd->ref_name) + 1);
strcpy(p, cmd->ref_name);
Expand Down
2 changes: 1 addition & 1 deletion refs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1851,7 +1851,7 @@ int update_ref(const char *action, const char *refname,
return 0;
}

int ref_exists(char *refname)
int ref_exists(const char *refname)
{
unsigned char sha1[20];
return !!resolve_ref(refname, sha1, 1, NULL);
Expand Down
2 changes: 1 addition & 1 deletion refs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
*/
extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
extern void clear_extra_refs(void);
extern int ref_exists(char *);
extern int ref_exists(const char *);

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

Expand Down
197 changes: 197 additions & 0 deletions t/t5516-fetch-push.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) &&
Expand Down

0 comments on commit 160b81e

Please sign in to comment.