> We used to recurse into submodules, even if they were broken having > only an objects directory. The child process executed in the submodule > would fail though if the submodule was broken. This is tested via > "fetching submodule into a broken repository" in t5526. > > This patch tightens the check upfront, such that we do not need > to spawn a child process to find out if the submodule is broken. Thanks, patches 4-7 look good to me - I see that you have addressed all my comments. Not sending one email each for patches 4, 5, and 6 - although I have commented on all of them, my comments were minor. My more in-depth review was done on a previous version [1], and I see that my comments have been addressed. Also, Stefan says [2] (and implements in this patch): > > > If the working tree directory is empty for that submodule, it means > > > it is likely not initialized. But why would we use that as a signal to > > > skip the submodule? > > > > What I meant was: if empty, skip it completely. Otherwise, do the > > repo_submodule_init() and repo_init() thing, and if they both fail, set > > spf->result to 1, preserving existing behavior. > > I did it the other way round: > > If repo_[submodule_]init fails, see if we have a gitlink in tree and > an empty dir in the FS, to decide if we need to signal failure. This works too. [1] https://public-inbox.org/git/20181017225811.66554-1-jonathantanmy@xxxxxxxxxx/ [2] https://public-inbox.org/git/CAGZ79kbNXD35ZwevjLZcrGsT=2hNcUPmVUWvP1RjsKSH0Gd3ww@xxxxxxxxxxxxxx/
