git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Stefan Beller <sbeller@google.com>, git@vger.kernel.org
Subject: Re: [PATCH] git-submodule.sh: try harder to fetch a submodule
Date: Sat, 12 May 2018 09:03:29 +0900
Message-ID: <xmqqmux5g2pa.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180511232840.GB41222@aiede.svl.corp.google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

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

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



  parent reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-11 23:17 Stefan Beller
2018-05-11 23:28 ` Jonathan Nieder
2018-05-11 23:42   ` Stefan Beller
2018-05-12  0:03   ` Junio C Hamano [this message]
2018-05-15 19:07     ` Stefan Beller
2018-05-15 19:40       ` Stefan Beller
2018-05-15 20:04       ` Jonathan Nieder
2018-05-15 20:00 [PATCHv2 0/3] Reroll of sb/submodule-merge-in-merge-recursive Stefan Beller
2018-05-15 20:00 ` [PATCH] git-submodule.sh: try harder to fetch a submodule Stefan Beller

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmux5g2pa.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox