Thread (18 messages) flat view 18 messages, 4 authors, 2019-02-02

Re: [PATCH 7/9] submodule: migrate get_next_submodule to use repository structs

From: Jonathan Tan <hidden>
Date: 2018-12-05 00:17:42

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):
quoted
quoted
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@google.com/
[2] https://public-inbox.org/git/CAGZ79kbNXD35ZwevjLZcrGsT=2hNcUPmVUWvP1RjsKSH0Gd3ww@mail.gmail.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help