push: require force for annotated tags

Do not allow fast-forwarding of references that point to a tag object.
Updating from a tag is potentially destructive since it would likely
leave the tag dangling.  Disallowing updates to a tag also makes sense
semantically and is consistent with the behavior of lightweight tags.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 09bdec75bc4f2895b81d1a288ed9f2a0c9ea6e08..7a04ce5f21429af05b95dcd323245369b8ec33a3 100644 (file)
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -52,11 +52,11 @@ updated.
 +
 The object referenced by <src> is used to update the <dst> reference
 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.
+a tag (annotated or lightweight), 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>`.
 +

diff --git a/remote.c b/remote.c
index 012b52f6fd5aa892894b09bc38109155ad0d6810..f5bc4e7260850d9b870d3e62e21c8a3a5380a130 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -1281,9 +1281,16 @@ int match_push_refs(struct ref *src, struct ref **dst,
 
 static inline int is_forwardable(struct ref* ref)
 {
+       struct object *o;
+
        if (!prefixcmp(ref->name, "refs/tags/"))
                return 0;
 
+       /* old object must be a commit */
+       o = parse_object(ref->old_sha1);
+       if (!o || o->type != OBJ_COMMIT)
+               return 0;
+
        return 1;
 }
 
@@ -1323,8 +1330,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
                 *     to overwrite it; you would not know what you are losing
                 *     otherwise.
                 *
-                * (4) if both new and old are commit-ish, and new is a
-                *     descendant of old, it is OK.
+                * (4) if old is a commit and new is a descendant of old
+                *     (implying new is commit-ish), it is OK.
                 *
                 * (5) regardless of all of the above, removing :B is
                 *     always allowed.

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8f024a08f0b175c12747c33ac0a07cacb73937d9..60093728fecd626ae5e005c4d41cee330ac1112c 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -950,6 +950,27 @@ test_expect_success 'push requires --force to update lightweight tag' '
        )
 '
 
+test_expect_success 'push requires --force to update annotated tag' '
+       mk_test heads/master &&
+       mk_child child1 &&
+       mk_child child2 &&
+       (
+               cd child1 &&
+               git tag -a -m "message 1" Tag &&
+               git push ../child2 Tag:refs/tmp/Tag &&
+               git push ../child2 Tag:refs/tmp/Tag &&
+               >file1 &&
+               git add file1 &&
+               git commit -m "file1" &&
+               git tag -f -a -m "message 2" Tag &&
+               test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
+               git push --force ../child2 Tag:refs/tmp/Tag &&
+               git tag -f -a -m "message 3" Tag HEAD~ &&
+               test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
+               git push --force ../child2 Tag:refs/tmp/Tag
+       )
+'
+
 test_expect_success 'push --porcelain' '
        mk_empty &&
        echo >.git/foo  "To testrepo" &&