Re: [PATCH] transport submodules: correct error message

[Stefan Beller <sbeller@google.com>]

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.

>
> Perhaps it may be an improvement.
>
>>     2) advertise pushing without recursing into submodules. ("Use this
>>        command to make the error message go away")
>
> Not mentioning "on-demand" may be an improvement for those who do
> use set-up like Dave has,

Daves setup is not a special setup; it is just git@next (later than
1863e05af5) in a repo that happens to have submodules; using
Gerrit as the code review system.

> 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/ ?

Yes. I think this is a problem in general with Git and Gerrit, as they have
different assumptions of the refs/ namespace.
(You can obtain all Gerrit changes in flight via fetching the refs/changes/*
similar to you pushing all git.git working branches not to all repositories,
but only https://github.com/gitster/git/)

> But would it be an improvement to suggest --no-recurse-submodules?

Well it is certainly better than the current situation, the user DID
use --recurse-submodules but still git refuses and advises to use
--recurse-submodules, which is understandable with background
knowledge, but very confusing at least.

The hint for --no-recurse-submodules can be understood as:
  "I told you there is something fishy with submodules, now you
  can just ignore this warning and murk up your history as you like."
which let's me question the correctness of assumptions in
1863e05af5^2.

> This issue seems like a property of the particular set-up, as
> opposed to being a one-off issue.  The next, subsequent

until Dave got the changes in the submodules reviewed and they
appear on a target branch, then fetching them, then the warning will
be gone.

> and probably
> all future pushes from that repository will have the same issue
> because the root cause is not due to the relative position of
> commits we have locally vs the tips of remote, but due to the way
> remote tracking is set-up, no?

Yes; ideally we'd have a remote-tracking ref for each change
pushed to Gerrit.

>
> 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.

If such a thing exists, then the check is the safest-but-inconvenient
option (in all setups I'd think), and users can switch to either
sameRefSpecButNoCheck (Gerrit) or on-demand (github et al)