| Message ID | 20180924212352.41909-1-smckelvie@xevo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | submodule.c: Make get_superproject_working_tree() work when supermodule has unmerged changes of the submodule reference | expand |
On Mon, Sep 24, 2018 at 2:25 PM Sam McKelvie <sammck@gmail.com> wrote:
Thanks for writing a patch!
I wonder if we can shorten the subject and make it a bit more concise,
how about
submodule: Alllow staged changes for get_superproject_working_tree
or
rev-parse: allow staged submodule in --show-superproject-working-tree
> Previously, "fatal: BUG: returned path string doesn't match cwd?" is displayed and
> git-rev-parse aborted.
Usually it is hard to read, continuing from the commit title to the body
of the commit message, as the title may be displayed differently or
somewhere else, so it would be good to restate it or rather state the
command that is broken, which we are trying to fix.
Invoking 'git rev-parse --show-superproject-working-tree' exits with
fatal: BUG ...
instead of displaying the superproject working tree when ....
> The problem is due to the fact that when a merge of the submodule reference is in progress,
> "git --stage —full-name <submodule-relative-path>” returns three seperate entries for the
"ls-files" is missing in that git invocation?
> 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.
The code looks good.
I wonder if we want to add a test for it, see t/t1500-rev-parse.sh
(at the very end 'showing the superproject correctly').
>
> Signed-off-by: Sam McKelvie <smckelvie@xevo.com>
Side note: the commit message seems to be indented,
but the patch applies fine, which makes me curious:
How did you produce&send the patch?
(Usually a patch would not apply as white spaces are
mangled in the code for example in some email setups)
(If I had to guess I could imagine that it is the output of
git-show as that indents the commit message usually,
see git-format-patch for a better tool)
Thanks for writing the patch!
Stefan
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); if (super_sub_len > cwd_len || strcmp(&cwd[cwd_len - super_sub_len], super_sub))
Previously, "fatal: BUG: returned path string doesn't match cwd?" is displayed and git-rev-parse aborted. The problem is due to the fact that when a merge of the submodule reference is in progress, "git --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. Signed-off-by: Sam McKelvie <smckelvie@xevo.com> --- submodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)