On Fri, May 11, 2018 at 4:28 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Hi, > > Stefan Beller wrote: > >> This is the logical continuum of fb43e31f2b4 (submodule: try harder to >> fetch needed sha1 by direct fetching sha1, 2016-02-23) and fixes it as >> some assumptions were not correct. > > Interesting. > > I think what would help most is an example set of commands I can use > to reproduce this (bonus points if in the form of a test). I tried coming up with a test in git-core that produces a remote similar to Gerrit, and let me tell you, it's not Git that is weird here. ;) >> > If $sha1 was not part of the default fetch ... fail ourselves here >> assumes that the fetch_in_submodule only fails when the serverside does > > I'm having some trouble with the formatting here. Is the part > preceded by '>' a quote, and if so a quote from what? The quote is from fb43e31f2b4. >> There are other failures, why such a fetch may fail, such as >> fatal: Couldn't find remote ref HEAD >> which can happen if the remote side doesn't advertise HEAD. Not advertising > > nit: it can be useful to have a blank line before and after such > example output to help both my eyes and tools like "git log > --format='%w(100)%b'" to understand the formatting. Why would you use this formatting? > >> 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). Oh, I checked that but not for the submodule case, let me check that. >> diff --git a/git-submodule.sh b/git-submodule.sh >> index 24914963ca2..13b378a6c8f 100755 >> --- a/git-submodule.sh >> +++ b/git-submodule.sh >> @@ -614,7 +614,6 @@ cmd_update() >> # 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? yes, as we only want to run the code below if there was some error. >> - die "$(eval_gettext "Unable to fetch in submodule path '\$displaypath'")" >> >> # Now we tried the usual fetch, but $sha1 may >> # not be reachable from any of the refs >> is_tip_reachable "$sm_path" "$sha1" || >> fetch_in_submodule "$sm_path" $depth "$sha1" || >> die "$(eval_gettext "Fetched in submodule path '\$displaypath', but it did not contain \$sha1. Direct fetching of that commit failed.")" > > Should this error message be changed? I don't think so?
