threads / bug / 29098

BUG: Confusing submodule error message

Subject: BUG: Confusing submodule error message

## tl;dr

3 messages between Dec 6, 2011 and Dec 8, 2011.

replies: 2people: 3as markdown or json

Seth Robertson· Dec 6, 2011, 19:30 UTC · lore

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
Jens Lehmann· Dec 7, 2011, 21:50 UTC · re: Seth Robertson · lore

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

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:
Show 18 quoted lines
> 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.
Show 16 quoted lines
> 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

← back to recent threads