References are allowed to update from one commit-ish to another if the
former is a 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 a
tag (lightweight and annotated) should be rejected unless the update is
forced.

To enable this functionality, the following checks have been added to
set_ref_status_for_push() for updating refs (i.e, not new or deletion)
to restrict fast-forwarding in pushes:

  1) The old and new references must be commits.  If this fails,
     it is not a valid update for a branch.

  2) The reference name cannot start with "refs/tags/".  This
     catches lightweight tags which (usually) point to commits
     and therefore would not be caught by (1).

If either of these checks fails, then it is flagged (by default) with a
status indicating the update is being rejected due to the reference
already existing in the remote.  This can be overridden by passing
--force to git push.

Signed-off-by: Chris Rorvick <ch...@rorvick.com>
---

Fix C99 comment.

 Documentation/git-push.txt | 10 +++++-----
 builtin/push.c             |  2 +-
 builtin/send-pack.c        |  5 +++++
 cache.h                    |  3 ++-
 remote.c                   | 24 ++++++++++++++++++++----
 send-pack.c                |  1 +
 t/t5516-fetch-push.sh      | 42 +++++++++++++++++++++++++++++++++++++++++-
 transport-helper.c         |  6 ++++++
 transport.c                |  8 ++++++--
 9 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c42..479e25f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,11 @@ 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 the update is
+a branch, and then only if it 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 branch or 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 0e13e11..cd7aa3f 100644
--- 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 a matching reference already exists 
in\n"
-          "the remote and the update is not a fast-forward.");
+          "the remote.");
 
 static void advise_pull_before_push(void)
 {
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index fda28bc..1eabf42 100644
--- 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 f410d94..127e504 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,13 +1004,14 @@ struct ref {
                requires_force:1,
                merge:1,
                nonfastforward:1,
-               is_a_tag:1,
+               forwardable:1,
                update:1,
                deletion:1;
        enum {
                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 44e72d6..a723f7a 100644
--- a/remote.c
+++ b/remote.c
@@ -1311,14 +1311,24 @@ 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.
                 *
-                * (3) if both new and old are commit-ish, and new is a
-                *     descendant of old, it is OK.
+                * (3) if new is commit-ish and old is a commit, new is a
+                *     descendant of old, and the reference is not of the
+                *     refs/tags/ hierarchy it is OK.
                 *
                 * (4) regardless of all of the above, removing :B is
                 *     always allowed.
                 */
 
-               ref->is_a_tag = !prefixcmp(ref->name, "refs/tags/");
+               if (prefixcmp(ref->name, "refs/tags/")) {
+                       /* Additionally, disallow fast-forwarding if
+                        * the old object is not a commit.  This kicks
+                        * out annotated tags that might pass the
+                        * is_newer() test but dangle if the reference
+                        * is updated.
+                        */
+                       struct object *obj = parse_object(ref->old_sha1);
+                       ref->forwardable = !obj || obj->type == OBJ_COMMIT;
+               }
 
                ref->update =
                        !ref->deletion &&
@@ -1329,7 +1339,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->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 f50dfd9..1c375f0 100644
--- 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 b5417cc..ca800b2 100755
--- 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,46 @@ 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 &&
+               >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 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 &&
+               >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" &&
diff --git a/transport-helper.c b/transport-helper.c
index 4713b69..965b778 100644
--- 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 60a7421..a380ad7 100644
--- 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->is_a_tag)
-                               *reject_mask |= REJECT_ALREADY_EXISTS;
                        if (!strcmp(head, ref->name))
                                *reject_mask |= REJECT_NON_FF_HEAD;
                        else
                                *reject_mask |= REJECT_NON_FF_OTHER;
+               } else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) {
+                       *reject_mask |= REJECT_ALREADY_EXISTS;
                }
        }
 }
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to