Sam McKelvie <sam...@gmail.com> 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 <sam...@gmail.com>
> ---
> 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.
