push: avoid showing false negotiation errors

When "git push" is configured to use the push negotiation, a push of
deletion of a branch (without pushing anything else) may end up not
having anything to negotiate for the common ancestor discovery.

In such a case, we end up making an internal invocation of "git
fetch --negotiate-only" without any "--negotiate-tip" parameters
that stops the negotiate-only fetch from being run, which by itself
is not a bad thing (one fewer round-trip), but the end-user sees a
"fatal: --negotiate-only needs one or more --negotiation-tip=*"
message that the user cannot act upon.

Teach "git push" to notice the situation and omit performing the
negotiate-only fetch to begin with.  One fewer process spawned, one
fewer "alarming" message given the user.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/send-pack.c b/send-pack.c
index 37f59d4f66bbc2b21bf08288a75de4a159b24625..72aa2dd9cd9f771ae90caeeb27d0a9bc6f5864b0 100644 (file)
--- a/send-pack.c
+++ b/send-pack.c
@@ -425,17 +425,26 @@ static void get_commons_through_negotiation(const char *url,
        struct child_process child = CHILD_PROCESS_INIT;
        const struct ref *ref;
        int len = the_hash_algo->hexsz + 1; /* hash + NL */
+       int nr_negotiation_tip = 0;
 
        child.git_cmd = 1;
        child.no_stdin = 1;
        child.out = -1;
        strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
        for (ref = remote_refs; ref; ref = ref->next) {
-               if (!is_null_oid(&ref->new_oid))
-                       strvec_pushf(&child.args, "--negotiation-tip=%s", oid_to_hex(&ref->new_oid));
+               if (!is_null_oid(&ref->new_oid)) {
+                       strvec_pushf(&child.args, "--negotiation-tip=%s",
+                                    oid_to_hex(&ref->new_oid));
+                       nr_negotiation_tip++;
+               }
        }
        strvec_push(&child.args, url);
 
+       if (!nr_negotiation_tip) {
+               child_process_clear(&child);
+               return;
+       }
+
        if (start_command(&child))
                die(_("send-pack: unable to fork off fetch subprocess"));
 

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2e7c0e1648f7aa4b879edcbe7cf15cde7583ae65..9d693eb57f7790ddb81cee0b905a101719069562 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -230,6 +230,16 @@ test_expect_success 'push with negotiation proceeds anyway even if negotiation f
        test_grep "push negotiation failed" err
 '
 
+test_expect_success 'push deletion with negotiation' '
+       mk_empty testrepo &&
+       git push testrepo $the_first_commit:refs/heads/master &&
+       git -c push.negotiate=1 push testrepo \
+               :master $the_first_commit:refs/heads/next 2>errors-2 &&
+       test_grep ! "negotiate-only needs one or " errors-2 &&
+       git -c push.negotiate=1 push testrepo :next 2>errors-1 &&
+       test_grep ! "negotiate-only needs one or " errors-1
+'
+
 test_expect_success 'push with negotiation does not attempt to fetch submodules' '
        mk_empty submodule_upstream &&
        test_commit -C submodule_upstream submodule_commit &&