push: require force for refs under refs/tags/

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>

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c4258a9c8a9dbab0b789215716065bef00ad..09bdec75bc4f2895b81d1a288ed9f2a0c9ea6e08 100644 (file)
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -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>`.
 +

diff --git a/builtin/push.c b/builtin/push.c
index e08485d1239fae1e1409864007b30b9c458a02e7..83a3cc80c332a6b53c3f18365eaef2a406a531ef 100644 (file)
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -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)
 {

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 9f986077aaaadaf1c858cd8e4a632eb8575e7260..f849e0a4a041610dc258a9dc56cc8a357cff31b6 100644 (file)
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -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";

diff --git a/cache.h b/cache.h
index b7ab4ac62428a3bc50224801bfee0277c6139e16..a32a0ea91c11f747a4612cc5c0b773635ec426c9 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -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,

diff --git a/remote.c b/remote.c
index 4a6f822089c076c10417cee6afc7a09de9777120..012b52f6fd5aa892894b09bc38109155ad0d6810 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -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.
                 */
 
@@ -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;

diff --git a/send-pack.c b/send-pack.c
index f50dfd9f488821e6b052affb19c0048afd11109c..1c375f0a28c6417f1f6cb48afbde681f458c544d 100644 (file)
--- a/send-pack.c
+++ b/send-pack.c
@@ -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:

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc951b1cecbbde613fa572387f362e889d1..8f024a08f0b175c12747c33ac0a07cacb73937d9 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -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
 
@@ -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" &&

diff --git a/transport-helper.c b/transport-helper.c
index 4713b6930248d961824c9f867b10340e6278132f..965b778cb3bb6fdd8068dbc7f9603cf9815c028d 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -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)
@@ -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:

diff --git a/transport.c b/transport.c
index f3160b142f7f5142a0935168e65461d34a5b2b7a..2673d273ff3aa5530ee57e8a3916bbbbc802466d 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -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,
@@ -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;
                }
        }
 }