Re: What's cooking in git.git (Mar 2009, #06; Sat, 21)

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 25, 2009, 18:06 UTC
Message-ID: <7vzlf94gv8.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20090324111645.GA6084@pvv.org>

Finn Arne Gangstad <finnag@pvv.org> writes:

Show 28 quoted lines

> On Tue, Mar 24, 2009 at 02:13:18AM -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> >   $ git push --dry-run sf.net
>> >   warning: ...
>> >   warning: You can specify what action you want to take in this case, and
>> >   warning: avoid seeing this message again, by configuring 'push.default' to:
>> >...
>> >   warning:   'current'  : Push the current branch
>> >   fatal: 'sf.net' does not appear to be a git repository
>> >   fatal: The remote end hung up unexpectedly
>> >
>> > The final, most important error messages are dwarfed out by the warning
>> > that talks about setting configuration on the remote that does not even
>> > exist.
>
> I actually agree, but that final error is printed in a very ugly
> place! It would require some surgery to pause the warning until we
> figure out that sf.net is not a valid remote in the current setup.
>
> get_refs_via_connect->connect_setup->git_connect-> .. fork()s .., runs
>    "sh -c git-receive-pack 'sf.net'" (which prints the first error)
> and get_remote_heads->packet_read_line->safe_read() dumps the final error
> and dies.
>
> Is there any reason why remote_get needs to return a valid remote for
> a value like "sf.net"?  If it didn't, the error message would be even
> better, and not complain about a "remote end".

Yeah, you could work around by special casing colon-less ones as you suggested, but I do not think it is worth it.

Instead of "sf.net", the example could have been

	git-core.git.sourceforge.net:/gitroot/gut-core

which looks like a perfectly valid push destination, but it has a typo in the pathname, and the remote end will hang up unexpectedly in such a case, too. I do not think remote_get() is a wrong thing to blame; in that codepath you simply do not know.

If you want to remove warning when we will eventually go "fatal", the way to do so is to remember you need to issue a warning but defer it until you actually start telling the other end in do_send_pack(), or something like that.

I do not think we would want nor need to go that route. This warning is about a local configuration that applies to _all_ remotes, and setting the configuration once will squelch it for future push to any remotes. Giving the warning even for an _incorrect_ remote is not wrong per-se.

Issuing the warning at first looked incorrect to me, but it warns exactly about what the patch that added the warning messages wanted to warn about:

    If you did not misspell the name of the remote or the URL, you would
    have triggered the default "matching refs" semantics, but you may not
    have meant to do so, hence we are warning.  If you want to change the
    default, here is how.  If you find this message irritating, you can
    squelch it this way.

In this particular case, I _did_ want the default matching refs semantics, so it was irritating whether I had the last two "fatal" lines or not. But the point of your patch was to help new people for the cost of irritating people like me (once), so...