refactor ref status logic for pushing

Move the logic that detects up-to-date and non-fast-forward refs to a
new function in remote.[ch], set_ref_status_for_push().

Make transport_push() invoke set_ref_status_for_push() before invoking
the push_refs() implementation. (As a side-effect, the push_refs()
implementation in transport-helper.c now knows of non-fast-forward
pushes.)

Removed logic for detecting up-to-date refs from the push_refs()
implementation in transport-helper.c, as transport_push() has already
done so for it.

Make cmd_send_pack() invoke set_ref_status_for_push() before invoking
send_pack(), as transport_push() can't do it for send_pack() here.

Mark the test on the return status of non-fast-forward push to fail.
Git now exits with success, as transport.c::transport_push() does not
check for refs with status REF_STATUS_REJECT_NONFASTFORWARD nor does it
indicate rejected pushes with its return value.

Mark the test for ref status to succeed. As mentioned earlier, refs
might be marked as non-fast-forwards, triggering the push status
printing mechanism in transport.c.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8fffdbf20058e9970af4b5e4a14349ecb4ff455c..76c72065de73ea3f0da4665c0a47a64610e2ead2 100644 (file)
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -406,50 +406,20 @@ int send_pack(struct send_pack_args *args,
         */
        new_refs = 0;
        for (ref = remote_refs; ref; ref = ref->next) {
-
-               if (ref->peer_ref)
-                       hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
-               else if (!args->send_mirror)
+               if (!ref->peer_ref && !args->send_mirror)
                        continue;
 
-               ref->deletion = is_null_sha1(ref->new_sha1);
-               if (ref->deletion && !allow_deleting_refs) {
-                       ref->status = REF_STATUS_REJECT_NODELETE;
-                       continue;
-               }
-               if (!ref->deletion &&
-                   !hashcmp(ref->old_sha1, ref->new_sha1)) {
-                       ref->status = REF_STATUS_UPTODATE;
+               /* Check for statuses set by set_ref_status_for_push() */
+               switch (ref->status) {
+               case REF_STATUS_REJECT_NONFASTFORWARD:
+               case REF_STATUS_UPTODATE:
                        continue;
+               default:
+                       ; /* do nothing */
                }
 
-               /* This part determines what can overwrite what.
-                * The rules are:
-                *
-                * (0) you can always use --force or +A:B notation to
-                *     selectively force individual ref pairs.
-                *
-                * (1) if the old thing does not exist, it is OK.
-                *
-                * (2) 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
-                *     descendant of old, it is OK.
-                *
-                * (4) regardless of all of the above, removing :B is
-                *     always allowed.
-                */
-
-               ref->nonfastforward =
-                   !ref->deletion &&
-                   !is_null_sha1(ref->old_sha1) &&
-                   (!has_sha1_file(ref->old_sha1)
-                     || !ref_newer(ref->new_sha1, ref->old_sha1));
-
-               if (ref->nonfastforward && !ref->force && !args->force_update) {
-                       ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+               if (ref->deletion && !allow_deleting_refs) {
+                       ref->status = REF_STATUS_REJECT_NODELETE;
                        continue;
                }
 
@@ -673,6 +643,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
        if (match_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))
                return -1;
 
+       set_ref_status_for_push(remote_refs, args.send_mirror,
+               args.force_update);
+
        ret = send_pack(&args, fd, conn, remote_refs, &extra_have);
 
        if (helper_status)

diff --git a/remote.c b/remote.c
index e3afecdb1069a244493e906b5e16a96dc47535d3..c70181cdc621b27ed02aba17b3e4f7ab64518e9f 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -1247,6 +1247,56 @@ int match_refs(struct ref *src, struct ref **dst,
        return 0;
 }
 
+void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
+       int force_update)
+{
+       struct ref *ref;
+
+       for (ref = remote_refs; ref; ref = ref->next) {
+               if (ref->peer_ref)
+                       hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
+               else if (!send_mirror)
+                       continue;
+
+               ref->deletion = is_null_sha1(ref->new_sha1);
+               if (!ref->deletion &&
+                       !hashcmp(ref->old_sha1, ref->new_sha1)) {
+                       ref->status = REF_STATUS_UPTODATE;
+                       continue;
+               }
+
+               /* This part determines what can overwrite what.
+                * The rules are:
+                *
+                * (0) you can always use --force or +A:B notation to
+                *     selectively force individual ref pairs.
+                *
+                * (1) if the old thing does not exist, it is OK.
+                *
+                * (2) 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
+                *     descendant of old, it is OK.
+                *
+                * (4) regardless of all of the above, removing :B is
+                *     always allowed.
+                */
+
+               ref->nonfastforward =
+                       !ref->deletion &&
+                       !is_null_sha1(ref->old_sha1) &&
+                       (!has_sha1_file(ref->old_sha1)
+                         || !ref_newer(ref->new_sha1, ref->old_sha1));
+
+               if (ref->nonfastforward && !ref->force && !force_update) {
+                       ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+                       continue;
+               }
+       }
+}
+
 struct branch *branch_get(const char *name)
 {
        struct branch *ret;

diff --git a/remote.h b/remote.h
index 8b7ecf9197f2a9210299c9700cc88a92a796f714..6e13643cabb6fa9a5b619f53dd148345d9161ad4 100644 (file)
--- a/remote.h
+++ b/remote.h
@@ -98,6 +98,8 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
 
 int match_refs(struct ref *src, struct ref **dst,
               int nr_refspec, const char **refspec, int all);
+void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
+       int force_update);
 
 /*
  * Given a list of the remote refs and the specification of things to

diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index cc740fe124a4d656b588dc0a881396b068b961f8..6d92196d24a5553b746618b8f5e24ef2ac36096d 100755 (executable)
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -88,7 +88,7 @@ test_expect_success 'used receive-pack service' '
        test_cmp exp act
 '
 
-test_expect_success 'non-fast-forward push fails' '
+test_expect_failure 'non-fast-forward push fails' '
        cd "$ROOT_PATH"/test_repo_clone &&
        git checkout master &&
        echo "changed" > path2 &&
@@ -100,7 +100,7 @@ test_expect_success 'non-fast-forward push fails' '
         test $HEAD != $(git rev-parse --verify HEAD))
 '
 
-test_expect_failure 'non-fast-forward push show ref status' '
+test_expect_success 'non-fast-forward push show ref status' '
        grep "^ ! \[rejected\][ ]*master -> master (non-fast-forward)$" output
 '
 

diff --git a/transport-helper.c b/transport-helper.c
index 11f3d7ec52893dcdee127d8493f1897c6c091b31..7c9b569d9488aa9dc779a1587acf940f4ef0dc5a 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -329,16 +329,16 @@ static int push_refs(struct transport *transport,
                return 1;
 
        for (ref = remote_refs; ref; ref = ref->next) {
-               if (ref->peer_ref)
-                       hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
-               else if (!mirror)
+               if (!ref->peer_ref && !mirror)
                        continue;
 
-               ref->deletion = is_null_sha1(ref->new_sha1);
-               if (!ref->deletion &&
-                       !hashcmp(ref->old_sha1, ref->new_sha1)) {
-                       ref->status = REF_STATUS_UPTODATE;
+               /* Check for statuses set by set_ref_status_for_push() */
+               switch (ref->status) {
+               case REF_STATUS_REJECT_NONFASTFORWARD:
+               case REF_STATUS_UPTODATE:
                        continue;
+               default:
+                       ; /* do nothing */
                }
 
                if (force_all)

diff --git a/transport.c b/transport.c
index 3eea836a33a56aaa99eec78bb4850b02888e377b..12c4423f79f251a93675af7cadfc3bd1f38c6967 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -887,6 +887,10 @@ int transport_push(struct transport *transport,
                        return -1;
                }
 
+               set_ref_status_for_push(remote_refs,
+                       flags & TRANSPORT_PUSH_MIRROR,
+                       flags & TRANSPORT_PUSH_FORCE);
+
                ret = transport->push_refs(transport, remote_refs, flags);
 
                if (!quiet || push_had_errors(remote_refs))