BUG: Confusing submodule error message

BUG: Confusing submodule error message

[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)

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;
----------------------------------------------------------------------

Do more error messages in submodule.c need adjusting?  It seems likely.

					-Seth Robertson

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

[Jens Lehmann]

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

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

[Junio C Hamano]

Thanks.