push: detect local refspec errors early

When pushing, we do not even look at our push refspecs until
after we have made contact with the remote receive-pack and
gotten its list of refs. This means that we may go to some
work, including asking the user to log in, before realizing
we have simple errors like "git push origin matser".

We cannot catch all refspec problems, since fully evaluating
the refspecs requires knowing what the remote side has. But
we can do a quick sanity check of the local side and catch a
few simple error cases.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/remote.c b/remote.c
index b6586c089ff7d44321a08ec04e18d169fbe425cc..9bf498af1b4c266c92585cbf3e95ad09db64fd0d 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -1373,6 +1373,31 @@ static void prepare_ref_index(struct string_list *ref_index, struct ref *ref)
        sort_string_list(ref_index);
 }
 
+/*
+ * Given only the set of local refs, sanity-check the set of push
+ * refspecs. We can't catch all errors that match_push_refs would,
+ * but we can catch some errors early before even talking to the
+ * remote side.
+ */
+int check_push_refs(struct ref *src, int nr_refspec, const char **refspec_names)
+{
+       struct refspec *refspec = parse_push_refspec(nr_refspec, refspec_names);
+       int ret = 0;
+       int i;
+
+       for (i = 0; i < nr_refspec; i++) {
+               struct refspec *rs = refspec + i;
+
+               if (rs->pattern || rs->matching)
+                       continue;
+
+               ret |= match_explicit_lhs(src, rs, NULL, NULL);
+       }
+
+       free_refspec(nr_refspec, refspec);
+       return ret;
+}
+
 /*
  * Given the set of refs the local repository has, the set of refs the
  * remote repository has, and the refspec used for push, determine

diff --git a/remote.h b/remote.h
index fb7647fab92aef651fbf2eb4118232a550450a55..917d383a80ddbfa377f10c3d1a4e132336065c2c 100644 (file)
--- a/remote.h
+++ b/remote.h
@@ -166,6 +166,7 @@ extern int query_refspecs(struct refspec *specs, int nr, struct refspec *query);
 char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
                     const char *name);
 
+int check_push_refs(struct ref *src, int nr_refspec, const char **refspec);
 int match_push_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,

diff --git a/t/t5529-push-errors.sh b/t/t5529-push-errors.sh
new file mode 100755 (executable)
index 0000000..9871307
--- /dev/null
+++ b/t/t5529-push-errors.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='detect some push errors early (before contacting remote)'
+. ./test-lib.sh
+
+test_expect_success 'setup commits' '
+       test_commit one
+'
+
+test_expect_success 'setup remote' '
+       git init --bare remote.git &&
+       git remote add origin remote.git
+'
+
+test_expect_success 'setup fake receive-pack' '
+       FAKE_RP_ROOT=$(pwd) &&
+       export FAKE_RP_ROOT &&
+       write_script fake-rp <<-\EOF &&
+       echo yes >"$FAKE_RP_ROOT"/rp-ran
+       exit 1
+       EOF
+       git config remote.origin.receivepack "\"\$FAKE_RP_ROOT/fake-rp\""
+'
+
+test_expect_success 'detect missing branches early' '
+       echo no >rp-ran &&
+       echo no >expect &&
+       test_must_fail git push origin missing &&
+       test_cmp expect rp-ran
+'
+
+test_expect_success 'detect missing sha1 expressions early' '
+       echo no >rp-ran &&
+       echo no >expect &&
+       test_must_fail git push origin master~2:master &&
+       test_cmp expect rp-ran
+'
+
+test_expect_success 'detect ambiguous refs early' '
+       git branch foo &&
+       git tag foo &&
+       echo no >rp-ran &&
+       echo no >expect &&
+       test_must_fail git push origin foo &&
+       test_cmp expect rp-ran
+'
+
+test_done

diff --git a/transport.c b/transport.c
index ca7bb441bf503c598d9679b338d7f7c0d8103cb0..325f03e1eef97df296bb1cc9f7d2d33c218dde26 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -1132,8 +1132,7 @@ int transport_push(struct transport *transport,
 
                return transport->push(transport, refspec_nr, refspec, flags);
        } else if (transport->push_refs) {
-               struct ref *remote_refs =
-                       transport->get_refs_list(transport, 1);
+               struct ref *remote_refs;
                struct ref *local_refs = get_local_heads();
                int match_flags = MATCH_REFS_NONE;
                int verbose = (transport->verbose > 0);
@@ -1142,6 +1141,11 @@ int transport_push(struct transport *transport,
                int pretend = flags & TRANSPORT_PUSH_DRY_RUN;
                int push_ret, ret, err;
 
+               if (check_push_refs(local_refs, refspec_nr, refspec) < 0)
+                       return -1;
+
+               remote_refs = transport->get_refs_list(transport, 1);
+
                if (flags & TRANSPORT_PUSH_ALL)
                        match_flags |= MATCH_REFS_ALL;
                if (flags & TRANSPORT_PUSH_MIRROR)