On Sun, Mar 25, 2018 at 3:58 AM René Scharfe <l....@web.de> wrote: > Am 25.03.2018 um 11:50 schrieb Jeremy Feusi: > > > > Hmm... That's weird. I can reproduce it on 3 independant systems with > > versions 2.16.2 up, although it does not work with version 2.11.0. > > Anyway, I figured out how to reproduce this bug. It is caused when a > > submodule is added and then the directory it resides in is moved or > > deleted without commiting. For example: > > > > git init > > git submodule add https://github.com/git/git git > > mv git git.BAK > > git submodule status #this command segfaults
> With the patch I sent in my first reply the last command reports: > fatal: no ref store in submodule 'git' > That may not be the most helpful message -- not just the ref store is > missing, the whole submodule is gone! > Come to think about it, this removal may be intended. How about > showing the submodule as not being initialized at that point? At first I thought we could still retrieve the ref store via a lookup of path -> name in .gitmodules and then navigate to .git/modules<name>/ (as seen from the superproject) and load the ref store. But loading the refstore is a mere detail. "not initialized" is technically correct, the existing git directory inside the superproject doesn't matter. > -- >8 -- > Subject: [PATCH v2] submodule: check for NULL return of get_submodule_ref_store() Maybe more imperative, telling what we actually want to achieve instead of what we do? submodule: report deleted submodules as not initialized > If we can't find a ref store for a submodule then assume it the latter > is not initialized (or was removed). Print a status line accordingly > instead of causing a segmentation fault by passing NULL as the first > parameter of refs_head_ref(). Thanks for the message here. Looks good! > Reported-by: Jeremy Feusi <jer...@feusi.co> Please also sign off instead of just claiming to report it. (The sign off has legal implications, see https://developercertificate.org/ or our copy in Documentation/SubmittingPatches) > Signed-off-by: Rene Scharfe <l....@web.de> > --- > Test missing.. Which would be added in t/t7400-submodule-basic.sh Thanks for coming up with a sensible patch! Stefan > builtin/submodule--helper.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index ee020d4749..ae3014ac5a 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -654,9 +654,13 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, > displaypath); > } else if (!(flags & OPT_CACHED)) { > struct object_id oid; > + struct ref_store *refs = get_submodule_ref_store(path); > - if (refs_head_ref(get_submodule_ref_store(path), > - handle_submodule_head_ref, &oid)) > + if (!refs) { > + print_status(flags, '-', path, ce_oid, displaypath); > + goto cleanup; > + } > + if (refs_head_ref(refs, handle_submodule_head_ref, &oid)) > die(_("could not resolve HEAD ref inside the " > "submodule '%s'"), path); > -- > 2.17.0.rc1.38.g7c51fd80b8
