[PATCH] diff/status: print submodule path when looking for changes fails

[Jens Lehmann <Jens.Lehmann@web.de>]

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