Re: [PATCH] transport submodules: correct error message

[Stefan Beller <sbeller@google.com>]

On Tue, Jan 17, 2017 at 4:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Tue, Jan 17, 2017 at 3:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Stefan Beller <sbeller@google.com> writes:
>>>
>>>> Trying to push with --recurse-submodules=on-demand would run into
>>>> the same problem. To fix this issue
>>>>     1) specifically mention that we looked for branches on the remote.
>>>
>>> That makes an incorrect statement ("not found on any remote"---we
>>> did not inspect all of the said remote, only heads and tags) into an
>>> irrelevant statement ("not found on any remote branch"---the end
>>> user would say "so what?  I know it exists there, it's just that not
>>> all remote refs have corresponding tracking ref locally on our side").
>>
>> eh. So to be correct we need to tell the user we did not find any match on
>> a "remote-tracking branch" as the gitglossary puts it.
>
> I think the updated text is already "correct".  I am pointing out
> that it may be correct but not very helpful to the users.
>
>>> where remote tracking information is
>>> incomplete if you only look at heads and refs, in the sense that we
>>> no longer suggest ineffective workaround.
>>
>> s/ineffective/an effective/ ?
>
> Even though I make many typoes, I meant ineffective in this case.
> "The old message suggested workaround that would not help.  You no
> longer give that workaround that does not work."

Oh, I misunderstood the original as I lumped on-demand and check
together mentally, because in the Gerrit world they behave similar.
(Both error out; in the on-demand case the server produces the failure
message, though)

>
>>> If that is the case, perhaps configuring push.recurseSubmodules to
>>> turn this off (especially because you plan to turn the defaul to
>>> "check") and not giving the command line option would give a more
>>> pleasant end-user experience, I suspect.
>>
>> I though about going another way and adding another new value
>> to the enum, such that
>>
>>     git push --recurse-submodules=sameRefSpecButNoCheck \
>>         origin HEAD:refs/for/master
>>
>> works for Gerrit users.
>
> It is unclear what that enum tells Git to do.  Care to explain?  How
> is it different from "no"?

In those submodules, that are checked positively, blindly run
git -C ${sub} push ${ref_spec} and do not double check again,
which we currently do in the on-demand case.