Re: [PATCH 3/2] batch check whether submodule needs pushing into one call

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2016, 18:13 UTC
Message-ID: <xmqq1t0jlnm2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160916123155.GA40725@book.hvoigt.net>

Heiko Voigt <hvoigt@hvoigt.net> writes:

Show 14 quoted lines

> On Fri, Sep 16, 2016 at 11:40:19AM +0200, Heiko Voigt wrote:
>> > By the way, with the two new patches, 'pu' seems to start failing
>> > some tests, e.g. 5533 5404 5405.
>> 
>> Ah ok I did only test on master, will look into those.
>
> Ok I had a look into these and the reason t5533 fails is because on pu
> --recurse-submodules is enabled by default and I missed the case when
> overwriting a ref. In that case we get the sha1 from the remote side as
> old. So we could catch that and fall back to all revisions there, but...
>
> ... tl;dr: The solution to use the old revisions from the remote side
> was too simple and does not make matters better but actually worse for
> some typical usecases. Its only in the last patch.

You may not even have the old one in your copy of the remote repository if you haven't fetched from them and you are forcing your push. "rev-list <new ones> --not <old ones>" may fail in such a case, not producing the list of new commits. You'd need to exclude old ones you learned over the wire that you do not yet have locally.

> The most exact solution would be to use all actual remote refs available
> (not sure if we have them at this point in the process?) another
> solution would be to still append the --remotes=<remotename> option as a
> fallback to reduce the revisions.

I'd say --remotes=<remotename> is the least problematic thing to do.