Merge branch 'jc/push-reject-reasons'

Improve error and advice messages given locally when "git push"
refuses when it cannot compute fast-forwardness by separating these
cases from the normal "not a fast-forward; merge first and push
again" case.

* jc/push-reject-reasons:
push: finishing touches to explain REJECT_ALREADY_EXISTS better
push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE
push: further simplify the logic to assign rejection reason
push: further clean up fields of "struct ref"

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 10225cfaf745524ddd01dd7957135bca1c435d91..c8abe86ed5cf6679914f7726b4852b679f6a36ba 100644 (file)
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -143,7 +143,8 @@ advice.*::
        pushUpdateRejected::
                Set this variable to 'false' if you want to disable
                'pushNonFFCurrent', 'pushNonFFDefault',
-               'pushNonFFMatching', and 'pushAlreadyExists'
+               'pushNonFFMatching', 'pushAlreadyExists',
+               'pushFetchFirst', and 'pushNeedsForce'
                simultaneously.
        pushNonFFCurrent::
                Advice shown when linkgit:git-push[1] fails due to a
@@ -162,6 +163,15 @@ advice.*::
        pushAlreadyExists::
                Shown when linkgit:git-push[1] rejects an update that
                does not qualify for fast-forwarding (e.g., a tag.)
+       pushFetchFirst::
+               Shown when linkgit:git-push[1] rejects an update that
+               tries to overwrite a remote ref that points at an
+               object we do not have.
+       pushNeedsForce::
+               Shown when linkgit:git-push[1] rejects an update that
+               tries to overwrite a remote ref that points at an
+               object that is not a committish, or make the remote
+               ref point at an object that is not a committish.
        statusHints::
                Show directions on how to proceed from the current
                state in the output of linkgit:git-status[1], in

diff --git a/advice.c b/advice.c
index d2879272802cb3477dd6b47a78098ffdc26dcb37..780f58da0f5b508eaa278c98dd15bdf6f919c468 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -5,6 +5,8 @@ int advice_push_non_ff_current = 1;
 int advice_push_non_ff_default = 1;
 int advice_push_non_ff_matching = 1;
 int advice_push_already_exists = 1;
+int advice_push_fetch_first = 1;
+int advice_push_needs_force = 1;
 int advice_status_hints = 1;
 int advice_commit_before_merge = 1;
 int advice_resolve_conflict = 1;
@@ -20,6 +22,8 @@ static struct {
        { "pushnonffdefault", &advice_push_non_ff_default },
        { "pushnonffmatching", &advice_push_non_ff_matching },
        { "pushalreadyexists", &advice_push_already_exists },
+       { "pushfetchfirst", &advice_push_fetch_first },
+       { "pushneedsforce", &advice_push_needs_force },
        { "statushints", &advice_status_hints },
        { "commitbeforemerge", &advice_commit_before_merge },
        { "resolveconflict", &advice_resolve_conflict },

diff --git a/advice.h b/advice.h
index 8bf63563a5cf1573e0e6bf7ffcd5c55643d3f439..fad36df467f065bf7ff7a82d9d447ee4ac5cecb9 100644 (file)
--- a/advice.h
+++ b/advice.h
@@ -8,6 +8,8 @@ extern int advice_push_non_ff_current;
 extern int advice_push_non_ff_default;
 extern int advice_push_non_ff_matching;
 extern int advice_push_already_exists;
+extern int advice_push_fetch_first;
+extern int advice_push_needs_force;
 extern int advice_status_hints;
 extern int advice_commit_before_merge;
 extern int advice_resolve_conflict;

diff --git a/builtin/push.c b/builtin/push.c
index b158028be81e38c37ec8dd69506cdc662ff1379b..42b129d36cf615ed264be0f1bff523a7fc327b12 100644 (file)
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -220,9 +220,20 @@ static const char message_advice_checkout_pull_push[] =
           "(e.g. 'git pull') before pushing again.\n"
           "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
+static const char message_advice_ref_fetch_first[] =
+       N_("Updates were rejected because the remote contains work that you do\n"
+          "not have locally. This is usually caused by another repository pushing\n"
+          "to the same ref. You may want to first merge the remote changes (e.g.,\n"
+          "'git pull') before pushing again.\n"
+          "See the 'Note about fast-forwards' in 'git push --help' for details.");
+
 static const char message_advice_ref_already_exists[] =
-       N_("Updates were rejected because the destination reference already exists\n"
-          "in the remote.");
+       N_("Updates were rejected because the tag already exists in the remote.");
+
+static const char message_advice_ref_needs_force[] =
+       N_("You cannot update a remote ref that points at a non-commit object,\n"
+          "or update a remote ref to make it point at a non-commit object,\n"
+          "without using the '--force' option.\n");
 
 static void advise_pull_before_push(void)
 {
@@ -252,6 +263,20 @@ static void advise_ref_already_exists(void)
        advise(_(message_advice_ref_already_exists));
 }
 
+static void advise_ref_fetch_first(void)
+{
+       if (!advice_push_fetch_first || !advice_push_update_rejected)
+               return;
+       advise(_(message_advice_ref_fetch_first));
+}
+
+static void advise_ref_needs_force(void)
+{
+       if (!advice_push_needs_force || !advice_push_update_rejected)
+               return;
+       advise(_(message_advice_ref_needs_force));
+}
+
 static int push_with_options(struct transport *transport, int flags)
 {
        int err;
@@ -285,6 +310,10 @@ static int push_with_options(struct transport *transport, int flags)
                        advise_checkout_pull_push();
        } else if (reject_reasons & REJECT_ALREADY_EXISTS) {
                advise_ref_already_exists();
+       } else if (reject_reasons & REJECT_FETCH_FIRST) {
+               advise_ref_fetch_first();
+       } else if (reject_reasons & REJECT_NEEDS_FORCE) {
+               advise_ref_needs_force();
        }
 
        return 1;

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index f849e0a4a041610dc258a9dc56cc8a357cff31b6..57a46b2654aa82b154d32eb77addd6a4f89c8e8d 100644 (file)
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -44,6 +44,16 @@ static void print_helper_status(struct ref *ref)
                        msg = "non-fast forward";
                        break;
 
+               case REF_STATUS_REJECT_FETCH_FIRST:
+                       res = "error";
+                       msg = "fetch first";
+                       break;
+
+               case REF_STATUS_REJECT_NEEDS_FORCE:
+                       res = "error";
+                       msg = "needs force";
+                       break;
+
                case REF_STATUS_REJECT_ALREADY_EXISTS:
                        res = "error";
                        msg = "already exists";

diff --git a/cache.h b/cache.h
index 9038fb500e45e926a696598b91a3759c22e3b306..e493563f4c07e6adcd00a1b2476926d69a4a67f8 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1015,10 +1015,8 @@ struct ref {
        char *symref;
        unsigned int
                force:1,
-               requires_force:1,
+               forced_update:1,
                merge:1,
-               nonfastforward:1,
-               update:1,
                deletion:1;
        enum {
                REF_STATUS_NONE = 0,
@@ -1026,6 +1024,8 @@ struct ref {
                REF_STATUS_REJECT_NONFASTFORWARD,
                REF_STATUS_REJECT_ALREADY_EXISTS,
                REF_STATUS_REJECT_NODELETE,
+               REF_STATUS_REJECT_FETCH_FIRST,
+               REF_STATUS_REJECT_NEEDS_FORCE,
                REF_STATUS_UPTODATE,
                REF_STATUS_REMOTE_REJECT,
                REF_STATUS_EXPECTING_REPORT

diff --git a/remote.c b/remote.c
index 9e21b1d78755cec7b30224f10c7f8ecbef3c9a49..e53a6eb7769e2884f77819c73423c31e49b0114e 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -1317,28 +1317,23 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
                 *     passing the --force argument
                 */
 
-               ref->update =
-                       !ref->deletion &&
-                       !is_null_sha1(ref->old_sha1);
-
-               if (ref->update) {
-                       ref->nonfastforward =
-                               !has_sha1_file(ref->old_sha1)
-                                 || !ref_newer(ref->new_sha1, ref->old_sha1);
-
-                       if (!prefixcmp(ref->name, "refs/tags/")) {
-                               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;
-                                       continue;
-                               }
-                       }
+               if (!ref->deletion && !is_null_sha1(ref->old_sha1)) {
+                       int why = 0; /* why would this push require --force? */
+
+                       if (!prefixcmp(ref->name, "refs/tags/"))
+                               why = REF_STATUS_REJECT_ALREADY_EXISTS;
+                       else if (!has_sha1_file(ref->old_sha1))
+                               why = REF_STATUS_REJECT_FETCH_FIRST;
+                       else if (!lookup_commit_reference_gently(ref->old_sha1, 1) ||
+                                !lookup_commit_reference_gently(ref->new_sha1, 1))
+                               why = REF_STATUS_REJECT_NEEDS_FORCE;
+                       else if (!ref_newer(ref->new_sha1, ref->old_sha1))
+                               why = REF_STATUS_REJECT_NONFASTFORWARD;
+
+                       if (!force_ref_update)
+                               ref->status = why;
+                       else if (why)
+                               ref->forced_update = 1;
                }
        }
 }
@@ -1532,7 +1527,8 @@ int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1)
        struct commit_list *list, *used;
        int found = 0;
 
-       /* Both new and old must be commit-ish and new is descendant of
+       /*
+        * Both new and old must be commit-ish and new is descendant of
         * old.  Otherwise we require --force.
         */
        o = deref_tag(parse_object(old_sha1), NULL, 0);

diff --git a/send-pack.c b/send-pack.c
index 1c375f0a28c6417f1f6cb48afbde681f458c544d..97ab336097bfb45c63aec739a6eef2df26cc4790 100644 (file)
--- a/send-pack.c
+++ b/send-pack.c
@@ -230,6 +230,8 @@ int send_pack(struct send_pack_args *args,
                switch (ref->status) {
                case REF_STATUS_REJECT_NONFASTFORWARD:
                case REF_STATUS_REJECT_ALREADY_EXISTS:
+               case REF_STATUS_REJECT_FETCH_FIRST:
+               case REF_STATUS_REJECT_NEEDS_FORCE:
                case REF_STATUS_UPTODATE:
                        continue;
                default:

diff --git a/transport-helper.c b/transport-helper.c
index 965b778cb3bb6fdd8068dbc7f9603cf9815c028d..cb3ef7d38e475480a5d81ce6077c4c87ea09a318 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -666,6 +666,16 @@ static void push_update_ref_status(struct strbuf *buf,
                        free(msg);
                        msg = NULL;
                }
+               else if (!strcmp(msg, "fetch first")) {
+                       status = REF_STATUS_REJECT_FETCH_FIRST;
+                       free(msg);
+                       msg = NULL;
+               }
+               else if (!strcmp(msg, "needs force")) {
+                       status = REF_STATUS_REJECT_NEEDS_FORCE;
+                       free(msg);
+                       msg = NULL;
+               }
        }
 
        if (*ref)

diff --git a/transport.c b/transport.c
index 0750a5fbbee7f192ce90d3d800921435cbb6208a..384ff9acf1e1c6e192f1f3f75d5fb20c431053ce 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -659,7 +659,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
                const char *msg;
 
                strcpy(quickref, status_abbrev(ref->old_sha1));
-               if (ref->requires_force) {
+               if (ref->forced_update) {
                        strcat(quickref, "...");
                        type = '+';
                        msg = "forced update";
@@ -699,6 +699,14 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
                print_ref_status('!', "[rejected]", ref, ref->peer_ref,
                                                 "already exists", porcelain);
                break;
+       case REF_STATUS_REJECT_FETCH_FIRST:
+               print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+                                                "fetch first", porcelain);
+               break;
+       case REF_STATUS_REJECT_NEEDS_FORCE:
+               print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+                                                "needs force", porcelain);
+               break;
        case REF_STATUS_REMOTE_REJECT:
                print_ref_status('!', "[remote rejected]", ref,
                                                 ref->deletion ? NULL : ref->peer_ref,
@@ -750,6 +758,10 @@ void transport_print_push_status(const char *dest, struct ref *refs,
                                *reject_reasons |= REJECT_NON_FF_OTHER;
                } else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) {
                        *reject_reasons |= REJECT_ALREADY_EXISTS;
+               } else if (ref->status == REF_STATUS_REJECT_FETCH_FIRST) {
+                       *reject_reasons |= REJECT_FETCH_FIRST;
+               } else if (ref->status == REF_STATUS_REJECT_NEEDS_FORCE) {
+                       *reject_reasons |= REJECT_NEEDS_FORCE;
                }
        }
 }

diff --git a/transport.h b/transport.h
index ac5a9f57d1051ba74fbf641f11d8c965faf8f078..a3450e97c0d0d94778a4b635efdd53b724eb70c2 100644 (file)
--- a/transport.h
+++ b/transport.h
@@ -144,6 +144,8 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 #define REJECT_NON_FF_HEAD     0x01
 #define REJECT_NON_FF_OTHER    0x02
 #define REJECT_ALREADY_EXISTS  0x04
+#define REJECT_FETCH_FIRST     0x08
+#define REJECT_NEEDS_FORCE     0x10
 
 int transport_push(struct transport *connection,
                   int refspec_nr, const char **refspec, int flags,