On 9 August 2018 at 00:17, Stefan Beller <[email protected]> wrote: > Currently when git-fetch is asked to recurse into submodules, it dispatches > a plain "git-fetch -C <submodule-dir>" (and some submodule related options > such as prefix and recusing strategy, but) without any information of the > remote or the tip that should be fetched. > > This works surprisingly well in some workflows (such as using submodules > as a third party library), while not so well in other scenarios, such > as in a Gerrit topic-based workflow, that can tie together changes > (potentially across repositories) on the server side. One of the parts > of such a Gerrit workflow is to download a change when wanting to examine > it, and you'd want to have its submodule changes that are in the same > topic downloaded as well. However these submodule changes reside in their > own repository in their on ref (refs/changes/<int>).
s/on/own/ > Retry fetching a submodule if the object id that the superproject points > to, cannot be found. > > Note: This is an RFC and doesn't support fetching to FETCH_HEAD yet, but > only into a local branch. To make fetching into FETCH_HEAD work, we need > some refactoring in builtin/fetch.c to adjust the calls to > 'check_for_new_submodule_commits'. > > Signed-off-by: Stefan Beller <[email protected]> > --- > diff --git a/submodule.c b/submodule.c > index ec7ea6f8c2d..6cbd0b1a470 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -1127,6 +1127,7 @@ struct submodule_parallel_fetch { > int result; > > struct string_list changed_submodule_names; > + struct string_list retry; > }; > #define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0, 0, > STRING_LIST_INIT_DUP } `retry` will effectively be `STRING_LIST_INIT_NODUP`, but making that explicit would be better and the next addition to the struct would be easier to get right. > +retry_next: > + retry_it = string_list_pop(&spf->retry); > + if (retry_it) { > + struct strbuf submodule_prefix = STRBUF_INIT; > + const struct submodule *sub = > + submodule_from_name(spf->r, > + &null_oid, > + retry_it->string); > + > + child_process_init(cp); > + cp->dir = get_submodule_git_dir(spf->r, sub->path); > + if (!cp->dir) > + goto retry_next; So here you just drop the string list item. Since it's NODUP, and since the `util` pointers are owned elsewhere(?), this seems fine. Other uses of `string_list_pop()` might not be so straightforward. Just a thought, but rather than pop+if+goto, maybe while ((retry_it = )) { ... if (!cp->dir) continue; ... return 1; } I haven't commented on any of the submodule stuff, which is probably where you'd be most interested in comments. I don't use submodules, nor do I know the code that runs them.. I guess my comments are more "if those who know something about submodules find this series worthwhile, you might want to consider my comments as well". Martin
