* [PATCH] diff/status: print submodule path when looking for changes fails
2011-12-06 19:30 BUG: Confusing submodule error message Seth Robertson
@ 2011-12-07 21:50 ` Jens Lehmann
2011-12-08 19:15 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Jens Lehmann @ 2011-12-07 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Seth Robertson, git
diff and status run "git status --porcelain" inside each populated
submodule to see if it contains changes (unless told not to do so via
config or command line option). When that fails, e.g. due to a corrupt
submodule .git directory, it just prints "git status --porcelain failed"
or "Could not run git status --porcelain" without giving the user a clue
where that happened.
Add '"in submodule %s", path' to these error strings to tell the user
where exactly the problem occurred.
Reported-by: Seth Robertson <in-gitvger@baka.org>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Am 06.12.2011 20:30, schrieb Seth Robertson:
> Someone on #git just encountered a problem where `git init && git add . &&
> git status` was failing with a message about a corrupted index.
>
> error: bad index file sha1 signature
> fatal: index file corrupt
> fatal: git status --porcelain failed
>
> This confused everyone for a while, until he provided access to the
> directory to play with. I eventually tracked it down to a directory
> in the tree which already had a .git directory in it. Unfortunately,
> that .git repo was corrupted and was the one returning the message
> about a corrupted index. The problem is that the error message we
> were seeing did not provide any direct hints that submodules were
> involved or that the problem was not at the top level (`git status
> --porcelain` is admittedly an indirect hint to both). Here is a
> recipe to reproduce a similar problem:
>
> (mkdir -p z/foo; cd z/foo; git init; echo A>A; git add A; git commit -m A; cd ..; echo B>B; rm -f foo/.git/objects/*/*; git init; git add .; git status)
Thanks for the report and the recipe to reproduce it.
> Providing an expanded error message which clarifies that this is
> failing in a submodule directory makes everything clear.
>
> ----------------------------------------------------------------------
> --- submodule.c~ 2011-12-02 14:25:08.000000000 -0500
> +++ submodule.c 2011-12-06 14:13:00.554413432 -0500
> @@ -714,7 +714,7 @@
> close(cp.out);
>
> if (finish_command(&cp))
> - die("git status --porcelain failed");
> + die("git status --porcelain failed in submodule directory %s", path);
>
> strbuf_release(&buf);
> return dirty_submodule;
> ----------------------------------------------------------------------
Makes lots of sense.
> Do more error messages in submodule.c need adjusting? It seems likely.
It looks like only the die() after the start_command() in the same
is_submodule_modified() function would also need to print the path.
The only other place that dies after starting a command inside a
submodule is in submodule_needs_pushing(), and it already says:
die("Could not run 'git rev-list %s --not --remotes -n 1' command in submodule %s",
...
So let's do the same in is_submodule_modified().
submodule.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/submodule.c b/submodule.c
index 52cdcc6..68c1ba9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -689,7 +689,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
cp.out = -1;
cp.dir = path;
if (start_command(&cp))
- die("Could not run git status --porcelain");
+ die("Could not run 'git status --porcelain' in submodule %s", path);
len = strbuf_read(&buf, cp.out, 1024);
line = buf.buf;
@@ -714,7 +714,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
close(cp.out);
if (finish_command(&cp))
- die("git status --porcelain failed");
+ die("'git status --porcelain' failed in submodule %s", path);
strbuf_release(&buf);
return dirty_submodule;
--
1.7.8.111.gd3732
^ permalink raw reply related [flat|nested] 3+ messages in thread