Skip to content

Commit

Permalink
push: require force for refs under refs/tags/
Browse files Browse the repository at this point in the history
References are allowed to update from one commit-ish to another if the
former is an ancestor of the latter.  This behavior is oriented to
branches which are expected to move with commits.  Tag references are
expected to be static in a repository, though, thus an update to
something under refs/tags/ should be rejected unless the update is
forced.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Chris Rorvick authored and Junio C Hamano committed Dec 2, 2012
1 parent 8c5f6f7 commit dbfeddb
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 13 deletions.
11 changes: 6 additions & 5 deletions Documentation/git-push.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ be named. If `:`<dst> is omitted, the same ref as <src> will be
updated.
+
The object referenced by <src> is used to update the <dst> reference
on the remote side, but by default this is only allowed if the
update can fast-forward <dst>. By having the optional leading `+`,
you can tell git to update the <dst> ref even when the update is not a
fast-forward. This does *not* attempt to merge <src> into <dst>. See
EXAMPLES below for details.
on the remote side. By default this is only allowed if <dst> is not
under refs/tags/, and then only if it can fast-forward <dst>. By having
the optional leading `+`, you can tell git to update the <dst> ref even
if it is not allowed by default (e.g., it is not a fast-forward.) This
does *not* attempt to merge <src> into <dst>. See EXAMPLES below for
details.
+
`tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
+
Expand Down
2 changes: 1 addition & 1 deletion builtin/push.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] =

static const char message_advice_ref_already_exists[] =
N_("Updates were rejected because the destination reference already exists\n"
"in the remote and the update is not a fast-forward.");
"in the remote.");

static void advise_pull_before_push(void)
{
Expand Down
5 changes: 5 additions & 0 deletions builtin/send-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref)
msg = "non-fast forward";
break;

case REF_STATUS_REJECT_ALREADY_EXISTS:
res = "error";
msg = "already exists";
break;

case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = "error";
Expand Down
1 change: 1 addition & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,7 @@ struct ref {
REF_STATUS_NONE = 0,
REF_STATUS_OK,
REF_STATUS_REJECT_NONFASTFORWARD,
REF_STATUS_REJECT_ALREADY_EXISTS,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
Expand Down
18 changes: 14 additions & 4 deletions remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -1315,14 +1315,18 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
*
* (1) if the old thing does not exist, it is OK.
*
* (2) if you do not have the old thing, you are not allowed
* (2) if the destination is under refs/tags/ you are
* not allowed to overwrite it; tags are expected
* to be static once created
*
* (3) if you do not have the old thing, you are not allowed
* to overwrite it; you would not know what you are losing
* otherwise.
*
* (3) if both new and old are commit-ish, and new is a
* (4) if both new and old are commit-ish, and new is a
* descendant of old, it is OK.
*
* (4) regardless of all of the above, removing :B is
* (5) regardless of all of the above, removing :B is
* always allowed.
*/

Expand All @@ -1337,7 +1341,13 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
!has_sha1_file(ref->old_sha1)
|| !ref_newer(ref->new_sha1, ref->old_sha1);

if (ref->nonfastforward) {
if (ref->not_forwardable) {
ref->requires_force = 1;
if (!force_ref_update) {
ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
continue;
}
} else if (ref->nonfastforward) {
ref->requires_force = 1;
if (!force_ref_update) {
ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
Expand Down
1 change: 1 addition & 0 deletions send-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ int send_pack(struct send_pack_args *args,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
Expand Down
23 changes: 22 additions & 1 deletion t/t5516-fetch-push.sh
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' '
git branch -D frotz
fi &&
git tag -f frotz &&
git push testrepo frotz &&
git push -f testrepo frotz &&
check_push_result $the_commit tags/frotz &&
check_push_result $the_first_commit heads/frotz
Expand Down Expand Up @@ -929,6 +929,27 @@ test_expect_success 'push into aliased refs (inconsistent)' '
)
'

test_expect_success 'push requires --force to update lightweight tag' '
mk_test heads/master &&
mk_child child1 &&
mk_child child2 &&
(
cd child1 &&
git tag Tag &&
git push ../child2 Tag &&
git push ../child2 Tag &&
>file1 &&
git add file1 &&
git commit -m "file1" &&
git tag -f Tag &&
test_must_fail git push ../child2 Tag &&
git push --force ../child2 Tag &&
git tag -f Tag &&
test_must_fail git push ../child2 Tag HEAD~ &&
git push --force ../child2 Tag
)
'

test_expect_success 'push --porcelain' '
mk_empty &&
echo >.git/foo "To testrepo" &&
Expand Down
6 changes: 6 additions & 0 deletions transport-helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,11 @@ static void push_update_ref_status(struct strbuf *buf,
free(msg);
msg = NULL;
}
else if (!strcmp(msg, "already exists")) {
status = REF_STATUS_REJECT_ALREADY_EXISTS;
free(msg);
msg = NULL;
}
}

if (*ref)
Expand Down Expand Up @@ -720,6 +725,7 @@ static int push_refs_with_push(struct transport *transport,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
Expand Down
8 changes: 6 additions & 2 deletions transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
"non-fast-forward", porcelain);
break;
case REF_STATUS_REJECT_ALREADY_EXISTS:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
"already exists", porcelain);
break;
case REF_STATUS_REMOTE_REJECT:
print_ref_status('!', "[remote rejected]", ref,
ref->deletion ? NULL : ref->peer_ref,
Expand Down Expand Up @@ -740,12 +744,12 @@ void transport_print_push_status(const char *dest, struct ref *refs,
ref->status != REF_STATUS_OK)
n += print_one_push_status(ref, dest, n, porcelain);
if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
if (ref->not_forwardable)
*reject_reasons |= REJECT_ALREADY_EXISTS;
if (!strcmp(head, ref->name))
*reject_reasons |= REJECT_NON_FF_HEAD;
else
*reject_reasons |= REJECT_NON_FF_OTHER;
} else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) {
*reject_reasons |= REJECT_ALREADY_EXISTS;
}
}
}
Expand Down

0 comments on commit dbfeddb

Please sign in to comment.