{"thread":{"id":"29098","subject":"BUG: Confusing submodule error message","startedAt":"2011-12-06T19:30:56Z","lastAt":"2011-12-08T19:15:39Z","messageCount":3,"participants":["Seth Robertson","Jens Lehmann","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"180400","messageId":"201112061930.pB6JUuDx004171@no.baka.org","threadId":"29098","inReplyTo":null,"subject":"BUG: Confusing submodule error message","fromName":"Seth Robertson","fromEmail":"in-gitvger@baka.org","sentAt":"2011-12-06T19:30:56Z","receivedAt":"2011-12-06T19:30:56Z","isPatch":false,"sender":{"key":"in-gitvger@baka.org","avatar":null},"body":"\nSomeone on #git just encountered a problem where `git init && git add . &&\ngit status` was failing with a message about a corrupted index.\n\n    error: bad index file sha1 signature\n    fatal: index file corrupt\n    fatal: git status --porcelain failed\n\nThis confused everyone for a while, until he provided access to the\ndirectory to play with.  I eventually tracked it down to a directory\nin the tree which already had a .git directory in it.  Unfortunately,\nthat .git repo was corrupted and was the one returning the message\nabout a corrupted index.  The problem is that the error message we\nwere seeing did not provide any direct hints that submodules were\ninvolved or that the problem was not at the top level (`git status\n--porcelain` is admittedly an indirect hint to both).  Here is a\nrecipe to reproduce a similar problem:\n\n(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)\n\nProviding an expanded error message which clarifies that this is\nfailing in a submodule directory makes everything clear.\n\n----------------------------------------------------------------------\n--- submodule.c~\t2011-12-02 14:25:08.000000000 -0500\n+++ submodule.c\t2011-12-06 14:13:00.554413432 -0500\n@@ -714,7 +714,7 @@\n \tclose(cp.out);\n \n \tif (finish_command(&cp))\n-\t\tdie(\"git status --porcelain failed\");\n+\t\tdie(\"git status --porcelain failed in submodule directory %s\", path);\n \n \tstrbuf_release(&buf);\n \treturn dirty_submodule;\n----------------------------------------------------------------------\n\nDo more error messages in submodule.c need adjusting?  It seems likely.\n\n\t\t\t\t\t-Seth Robertson\n"},{"id":"180526","messageId":"4EDFDF96.9030601@web.de","threadId":"29098","inReplyTo":"201112061930.pB6JUuDx004171@no.baka.org","subject":"[PATCH] diff/status: print submodule path when looking for changes fails","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-12-07T21:50:14Z","receivedAt":"2011-12-07T21:50:14Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"diff and status run \"git status --porcelain\" inside each populated\nsubmodule to see if it contains changes (unless told not to do so via\nconfig or command line option). When that fails, e.g. due to a corrupt\nsubmodule .git directory, it just prints \"git status --porcelain failed\"\nor \"Could not run git status --porcelain\" without giving the user a clue\nwhere that happened.\n\nAdd '\"in submodule %s\", path' to these error strings to tell the user\nwhere exactly the problem occurred.\n\nReported-by: Seth Robertson <in-gitvger@baka.org>\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 06.12.2011 20:30, schrieb Seth Robertson:\n> Someone on #git just encountered a problem where `git init && git add . &&\n> git status` was failing with a message about a corrupted index.\n> \n>     error: bad index file sha1 signature\n>     fatal: index file corrupt\n>     fatal: git status --porcelain failed\n> \n> This confused everyone for a while, until he provided access to the\n> directory to play with.  I eventually tracked it down to a directory\n> in the tree which already had a .git directory in it.  Unfortunately,\n> that .git repo was corrupted and was the one returning the message\n> about a corrupted index.  The problem is that the error message we\n> were seeing did not provide any direct hints that submodules were\n> involved or that the problem was not at the top level (`git status\n> --porcelain` is admittedly an indirect hint to both).  Here is a\n> recipe to reproduce a similar problem:\n> \n> (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)\n\nThanks for the report and the recipe to reproduce it.\n\n> Providing an expanded error message which clarifies that this is\n> failing in a submodule directory makes everything clear.\n> \n> ----------------------------------------------------------------------\n> --- submodule.c~\t2011-12-02 14:25:08.000000000 -0500\n> +++ submodule.c\t2011-12-06 14:13:00.554413432 -0500\n> @@ -714,7 +714,7 @@\n>  \tclose(cp.out);\n>  \n>  \tif (finish_command(&cp))\n> -\t\tdie(\"git status --porcelain failed\");\n> +\t\tdie(\"git status --porcelain failed in submodule directory %s\", path);\n>  \n>  \tstrbuf_release(&buf);\n>  \treturn dirty_submodule;\n> ----------------------------------------------------------------------\n\nMakes lots of sense.\n\n> Do more error messages in submodule.c need adjusting?  It seems likely.\n\nIt looks like only the die() after the start_command() in the same\nis_submodule_modified() function would also need to print the path.\n\nThe only other place that dies after starting a command inside a\nsubmodule is in submodule_needs_pushing(), and it already says:\n\tdie(\"Could not run 'git rev-list %s --not --remotes -n 1' command in submodule %s\",\n\t...\nSo let's do the same in is_submodule_modified().\n\n\n submodule.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 52cdcc6..68c1ba9 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -689,7 +689,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)\n \tcp.out = -1;\n \tcp.dir = path;\n \tif (start_command(&cp))\n-\t\tdie(\"Could not run git status --porcelain\");\n+\t\tdie(\"Could not run 'git status --porcelain' in submodule %s\", path);\n\n \tlen = strbuf_read(&buf, cp.out, 1024);\n \tline = buf.buf;\n@@ -714,7 +714,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)\n \tclose(cp.out);\n\n \tif (finish_command(&cp))\n-\t\tdie(\"git status --porcelain failed\");\n+\t\tdie(\"'git status --porcelain' failed in submodule %s\", path);\n\n \tstrbuf_release(&buf);\n \treturn dirty_submodule;\n-- \n1.7.8.111.gd3732\n"},{"id":"180703","messageId":"7vobvhebno.fsf@alter.siamese.dyndns.org","threadId":"29098","inReplyTo":"4EDFDF96.9030601@web.de","subject":"Re: [PATCH] diff/status: print submodule path when looking for changes fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-08T19:15:39Z","receivedAt":"2011-12-08T19:15:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}