Re: [PATCH] git-submodule.sh: try harder to fetch a submodule

From: Junio C Hamano <gitster@pobox.com>
Date: May 12, 2018, 00:03 UTC
Message-ID: <xmqqmux5g2pa.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180511232840.GB41222@aiede.svl.corp.google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 5 quoted lines

>> HEAD is allowed by the protocol spec and would happen, if HEAD points at a
>> ref, that this user cannot see (due to ACLs for example).
>
> A more typical example would be if the ref simply doesn't exist (i.e.,
> is a branch yet to be born).

Indeed this is interesting. At first glance I thought this was about underlying "git clone" failing to grab things from a repository with unborn HEAD, but that part works perfectly OK. And if this failed clone were a full-repository clone that tried to grab even HEAD, then it is likely that we got the tip we need to populate the submodule's working tree (or the remote is useless for that in the first place). So the "allow to try even harder" is probably a good direction to go in.

Show 5 quoted lines

>>  				# is not reachable from a ref.
>>  				is_tip_reachable "$sm_path" "$sha1" ||
>>  				fetch_in_submodule "$sm_path" $depth ||
>
> Is keeping the '||' at the end of this line intended?

Good question.  It used to be

	guard1 || action1 || die
	guard2 || action2 || die

Even after a successful exit from "action1", the code used to try the second attempt, and the patch is removing the first die, making the whole thing into

	guard1 || action1 ||
	guard2 || action2 || die

which suggests a grave regression, doesn't it? "action1" (a whole repository fetch) may not pull down the needed commit even the fetch operation itself may succeed, in which case "guard2" notices that the tip is still not here and "action2" (an exact SHA-1 fetch) tries to pull down the exact thing as the last resort.

So it probably should be more like

	guard1 || action1 || warn
	guard2 || action2 || die

so that no matter what the outcome of the action1 is, the second set gets executed.