Sam McKelvie <sammck@xxxxxxxxx> writes: > Subject: Re: [PATCH] submodule: Alllow staged changes for get_superproject_working_tree s/Alllow/allow/; > Invoking 'git rev-parse --show-superproject-working-tree' exits with > > "fatal: BUG: returned path string doesn't match cwd?" > > when the superproject has an unmerged entry for the current submodule, > instead of displaying the superproject's working tree. > > The problem is due to the fact that when a merge of the submodule reference > is in progress, "git ls-files --stage —full-name <submodule-relative-path>” > returns three seperate entries for the submodule (one for each stage) rather > than a single entry; e.g., > > $ git ls-files --stage --full-name submodule-child-test > 160000 dbbd2766fa330fa741ea59bb38689fcc2d283ac5 1 submodule-child-test > 160000 f174d1dbfe863a59692c3bdae730a36f2a788c51 2 submodule-child-test > 160000 e6178f3a58b958543952e12824aa2106d560f21d 3 submodule-child-test > > The code in get_superproject_working_tree() expected exactly one entry to > be returned; this patch makes it use the first entry if multiple entries > are returned. > > Test t1500-rev-parse is extended to cover this case. > > Signed-off-by: Sam McKelvie <sammck@xxxxxxxxx> > --- > submodule.c | 2 +- > t/t1500-rev-parse.sh | 17 ++++++++++++++++- > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/submodule.c b/submodule.c > index 33de6ee5f..5b9d5ad7e 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -1885,7 +1885,7 @@ const char *get_superproject_working_tree(void) > * We're only interested in the name after the tab. > */ > super_sub = strchr(sb.buf, '\t') + 1; > - super_sub_len = sb.buf + sb.len - super_sub - 1; > + super_sub_len = strlen(super_sub); As we are reading from "ls-files -z -s", we know that the name is terminated with NUL, so we can just use strlen(). Good. > > if (super_sub_len > cwd_len || > strcmp(&cwd[cwd_len - super_sub_len], super_sub)) > diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh > index 5c715fe2c..b774cafc5 100755 > --- a/t/t1500-rev-parse.sh > +++ b/t/t1500-rev-parse.sh > @@ -134,7 +134,6 @@ test_expect_success 'rev-parse --is-shallow-repository in non-shallow repo' ' > test_expect_success 'showing the superproject correctly' ' > git rev-parse --show-superproject-working-tree >out && > test_must_be_empty out && > - I have a feeling that this break made the series of tests in this block easier to follow. Shouldn't we be moving in the other direction, namely ... > test_create_repo super && > test_commit -C super test_commit && > test_create_repo sub && > @@ -142,6 +141,22 @@ test_expect_success 'showing the superproject correctly' ' > git -C super submodule add ../sub dir/sub && > echo $(pwd)/super >expect && > git -C super/dir/sub rev-parse --show-superproject-working-tree >out && > + test_cmp expect out && Here is an end of one subtest, deserves to have a break like the above. > + test_commit -C super submodule_add && > + git -C super checkout -b branch1 && > + git -C super/dir/sub checkout -b branch1 && > + test_commit -C super/dir/sub branch1_commit && > + git -C super add dir/sub && > + test_commit -C super branch1_commit && > + git -C super checkout master && > + git -C super checkout -b branch2 && > + git -C super/dir/sub checkout master && > + git -C super/dir/sub checkout -b branch2 && > + test_commit -C super/dir/sub branch2_commit && > + git -C super add dir/sub && > + test_commit -C super branch2_commit && > + test_must_fail git -C super merge branch1 && and all of the above is just a set-up for another subtest, so a solid block of text like we see in the above is good. Side note: there are a few of git -C $there checkout $onebranch && git -C $there checkout -b $anotherbranch && as recurring pattern. Shouldn't they be more like a single liner git -C $there checkout -b $anotherbranch $onebranch && ? It wasn't clear if the split was an attempt to hide some breakage (e.g. "checkout -b B A" did not work but "checkout A && checkout -b B" did) or just being verbose because the author is not used to "checkout -b B A" form. > + git -C super/dir/sub rev-parse --show-superproject-working-tree >out && > test_cmp expect out > ' Thanks.
