Re: [PATCH 2/2] push: add an advice on unqualified <dst> push

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 26, 2018, 15:45 UTC
Message-ID: <87r2gclnjc.fsf@evledraar.gmail.com>
In-Reply-To: <xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com>

On Thu, Oct 11 2018, Junio C Hamano wrote:

Show 18 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Wed, Oct 10 2018, Jeff King wrote:
>>
>>> This is much better, and I love the customized behavior based on the
>>> object type.
>>>
>>> I wonder if we could reword the first paragraph to be a little less
>>> confusing, and spell out what we tried already. E.g., something like:
>>> ...
>>
>> Yeah that makes sense. I was trying to avoid touching the existing
>> wording to make this more surgical, but you came up with it, and since
>> you don't like it I'll just change that too.
>
> OK, for now I'll mark these two patches "read" in my inbox and
> forget about them, expecting that a reroll of 2/2 with improved
> messages would appear not in too distant future.

I was going to submit an update to this, as an additional improvement can anyone think of a reason not to always infer that we'd like a new branch if the LHS of the refspec starts with refs/remotes/* ?

    u git (push-advice-on-unqualified-src-2 $>) $ ./git-push avar refs/remotes/origin/master:newbranch -n
    To github.com:avar/git.git
     * [new branch]            origin/master -> newbranch
    u git (push-advice-on-unqualified-src-2 $>) $ git diff
    diff --git a/remote.c b/remote.c
    index 5b679df02d..949a9bd079 100644
    --- a/remote.c
    +++ b/remote.c
    @@ -969,7 +969,8 @@ static char *guess_ref(const char *name, struct ref *peer)
            if (!r)
                    return NULL;

    -       if (starts_with(r, "refs/heads/"))
    +       if (starts_with(r, "refs/heads/") ||
    +           starts_with(r, "refs/remotes/"))
                    strbuf_addstr(&buf, "refs/heads/");
            else if (starts_with(r, "refs/tags/"))
                    strbuf_addstr(&buf, "refs/tags/");

Maybe we need to be really paranoid here and also check if it's a "commit", i.e. you could setup a refspec like:

    fetch = +refs/tags/*:refs/remotes/origin-tags/*