{"thread":{"id":"21829","subject":"[PATCH v2] Detailed diagnosis when parsing an object name fails.","startedAt":"2009-12-02T20:01:01Z","lastAt":"2009-12-02T20:01:01Z","messageCount":1,"participants":["y@imag.fr"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"129059","messageId":"36497.9872749702$1259784125@news.gmane.org","threadId":"21829","inReplyTo":null,"subject":"[PATCH v2] Detailed diagnosis when parsing an object name fails.","fromName":"","fromEmail":"y@imag.fr","sentAt":"2009-12-02T20:01:01Z","receivedAt":"2009-12-02T20:01:01Z","isPatch":true,"sender":{"key":"y@imag.fr","avatar":null},"body":"From: Matthieu Moy <Matthieu.Moy@imag.fr>\n\nThe previous error message was the same in many situations (unknown\nrevision or path not in the working tree). We try to help the user as\nmuch as possible to understand the error, especially with the\nsha1:filename notation. In this case, we say whether the sha1 or the\nfilename is problematic, and diagnose the confusion between\nrelative-to-root and relative-to-$PWD confusion precisely.\n\nThe 6 new error messages are tested.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nChanges since v1:\n\n* Fixed a segfault with\n\n+\tif (!prefix)\n+\t\tprefix = \"\";\n\n* Added testcases.\n\n cache.h                        |    6 ++-\n setup.c                        |   15 +++++-\n sha1_name.c                    |   95 ++++++++++++++++++++++++++++++++++++++--\n t/t1506-rev-parse-diagnosis.sh |   67 ++++++++++++++++++++++++++++\n 4 files changed, 176 insertions(+), 7 deletions(-)\n create mode 100755 t/t1506-rev-parse-diagnosis.sh\n\ndiff --git a/cache.h b/cache.h\nindex 0e69384..5c8cb5f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -708,7 +708,11 @@ static inline unsigned int hexval(unsigned char c)\n #define DEFAULT_ABBREV 7\n \n extern int get_sha1(const char *str, unsigned char *sha1);\n-extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);\n+static inline get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)\n+{\n+\treturn get_sha1_with_mode_1(str, sha1, mode, 0, NULL);\n+}\n+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix);\n extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n extern char *sha1_to_hex(const unsigned char *sha1);\t/* static buffer result! */\n extern int read_ref(const char *filename, unsigned char *sha1);\ndiff --git a/setup.c b/setup.c\nindex f67250b..3094e8b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -74,6 +74,18 @@ int check_filename(const char *prefix, const char *arg)\n \tdie_errno(\"failed to stat '%s'\", arg);\n }\n \n+static void NORETURN die_verify_filename(const char *prefix, const char *arg)\n+{\n+\tunsigned char sha1[20];\n+\tunsigned mode;\n+\t/* try a detailed diagnostic ... */\n+\tget_sha1_with_mode_1(arg, sha1, &mode, 1, prefix);\n+\t/* ... or fall back the most general message. */\n+\tdie(\"ambiguous argument '%s': unknown revision or path not in the working tree.\\n\"\n+\t    \"Use '--' to separate paths from revisions\", arg);\n+\n+}\n+\n /*\n  * Verify a filename that we got as an argument for a pathspec\n  * entry. Note that a filename that begins with \"-\" never verifies\n@@ -87,8 +99,7 @@ void verify_filename(const char *prefix, const char *arg)\n \t\tdie(\"bad flag '%s' used after filename\", arg);\n \tif (check_filename(prefix, arg))\n \t\treturn;\n-\tdie(\"ambiguous argument '%s': unknown revision or path not in the working tree.\\n\"\n-\t    \"Use '--' to separate paths from revisions\", arg);\n+\tdie_verify_filename(prefix, arg);\n }\n \n /*\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 44bb62d..030e2ac 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -804,7 +804,77 @@ int get_sha1(const char *name, unsigned char *sha1)\n \treturn get_sha1_with_mode(name, sha1, &unused);\n }\n \n-int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)\n+static void diagnose_invalid_sha1_path(const char *prefix,\n+\t\t\t\t       const char *filename,\n+\t\t\t\t       const char *tree_sha1,\n+\t\t\t\t       const char *object_name)\n+{\n+\tstruct stat st;\n+\tunsigned char sha1[20];\n+\tunsigned mode;\n+\n+\tif (!prefix)\n+\t\tprefix = \"\";\n+\n+\tif (!lstat(filename, &st))\n+\t\tdie(\"Path '%s' exists on disk, but not in '%s'.\",\n+\t\t    filename, object_name);\n+\tif (errno == ENOENT || errno == ENOTDIR) {\n+\t\tchar *fullname = malloc(strlen(filename)\n+\t\t\t\t\t     + strlen(prefix) + 1);\n+\t\tstrcpy(fullname, prefix);\n+\t\tstrcat(fullname, filename);\n+\n+\t\tif (!get_tree_entry(tree_sha1, fullname,\n+\t\t\t\t    sha1, &mode)) {\n+\t\t\tdie(\"Path '%s' exists, but not '%s'.\\n\"\n+\t\t\t    \"Did you mean '%s:%s'?\",\n+\t\t\t    fullname,\n+\t\t\t    filename,\n+\t\t\t    object_name,\n+\t\t\t    fullname);\n+\t\t}\n+\t\tdie(\"Path '%s' does not exist in '%s'\",\n+\t\t    filename, object_name);\n+\t}\n+}\n+\n+static void diagnose_invalid_index_path(int stage,\n+\t\t\t\t\tconst char *prefix,\n+\t\t\t\t\tconst char *filename)\n+{\n+\tstruct stat st;\n+\n+\tif (!prefix)\n+\t\tprefix = \"\";\n+\n+\tif (!lstat(filename, &st))\n+\t\tdie(\"Path '%s' exists on disk, but not in the index.\", filename);\n+\tif (errno == ENOENT || errno == ENOTDIR) {\n+\t\tstruct cache_entry *ce;\n+\t\tint pos;\n+\t\tint namelen = strlen(filename) + strlen(prefix);\n+\t\tchar *fullname = malloc(namelen + 1);\n+\t\tstrcpy(fullname, prefix);\n+\t\tstrcat(fullname, filename);\n+\t\tpos = cache_name_pos(fullname, namelen);\n+\t\tif (pos < 0)\n+\t\t\tpos = -pos - 1;\n+\t\tce = active_cache[pos];\n+\t\tif (ce_namelen(ce) == namelen &&\n+\t\t    !memcmp(ce->name, fullname, namelen))\n+\t\t\tdie(\"Path '%s' is in the index, but not '%s'.\\n\"\n+\t\t\t    \"Did you mean ':%d:%s'?\",\n+\t\t\t    fullname, filename,\n+\t\t\t    stage, fullname);\n+\n+\t\tdie(\"Path '%s' does not exist (neither on disk nor in the index).\",\n+\t\t    filename);\n+\t}\n+}\n+\n+\n+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix)\n {\n \tint ret, bracket_depth;\n \tint namelen = strlen(name);\n@@ -850,6 +920,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)\n \t\t\t}\n \t\t\tpos++;\n \t\t}\n+\t\tif (fatal)\n+\t\t\tdiagnose_invalid_index_path(stage, prefix, cp);\n \t\treturn -1;\n \t}\n \tfor (cp = name, bracket_depth = 0; *cp; cp++) {\n@@ -862,9 +934,24 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)\n \t}\n \tif (*cp == ':') {\n \t\tunsigned char tree_sha1[20];\n-\t\tif (!get_sha1_1(name, cp-name, tree_sha1))\n-\t\t\treturn get_tree_entry(tree_sha1, cp+1, sha1,\n-\t\t\t\t\t      mode);\n+\t\tchar *object_name;\n+\t\tif (fatal) {\n+\t\t\tobject_name = malloc(cp-name+1);\n+\t\t\tstrncpy(object_name, name, cp-name);\n+\t\t\tobject_name[cp-name] = '\\0';\n+\t\t}\n+\t\tif (!get_sha1_1(name, cp-name, tree_sha1)) {\n+\t\t\tconst char *filename = cp+1;\n+\t\t\tret = get_tree_entry(tree_sha1, filename, sha1, mode);\n+\t\t\tif (fatal)\n+\t\t\t\tdiagnose_invalid_sha1_path(prefix, filename,\n+\t\t\t\t\t\t\t   tree_sha1, object_name);\n+\n+\t\t\treturn ret;\n+\t\t} else {\n+\t\t\tif (fatal)\n+\t\t\t\tdie(\"Invalid object name '%s'.\", object_name);\n+\t\t}\n \t}\n \treturn ret;\n }\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nnew file mode 100755\nindex 0000000..8112d56\n--- /dev/null\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -0,0 +1,67 @@\n+#!/bin/sh\n+\n+test_description='test git rev-parse diagnosis for invalid argument'\n+\n+exec </dev/null\n+\n+. ./test-lib.sh\n+\n+HASH_file=\n+\n+test_expect_success 'set up basic repo' '\n+\techo one > file.txt &&\n+\tmkdir subdir &&\n+\techo two > subdir/file.txt &&\n+\techo three > subdir/file2.txt &&\n+\tgit add . &&\n+\tgit commit -m init &&\n+\techo four > index-only.txt &&\n+\tgit add index-only.txt &&\n+\techo five > disk-only.txt\n+'\n+\n+test_expect_success 'correct file objects' '\n+\tHASH_file=$(git rev-parse HEAD:file.txt) &&\n+\tgit rev-parse HEAD:subdir/file.txt &&\n+\tgit rev-parse :index-only.txt &&\n+\tcd subdir &&\n+\tgit rev-parse HEAD:file.txt &&\n+\tgit rev-parse HEAD:subdir/file2.txt &&\n+\ttest $HASH_file = $(git rev-parse HEAD:file.txt) &&\n+\ttest $HASH_file = $(git rev-parse :file.txt) &&\n+\ttest $HASH_file = $(git rev-parse :0:file.txt) &&\n+\tcd ..\n+'\n+\n+test_expect_success 'incorrect revision id' '\n+\ttest_must_fail git rev-parse foobar:file.txt 2>&1 |\n+\t\tgrep \"Invalid object name '\"'\"'foobar'\"'\"'.\" &&\n+\ttest_must_fail git rev-parse foobar 2>&1 |\n+\t\tgrep \"unknown revision or path not in the working tree.\"\n+'\n+\n+test_expect_success 'incorrect file in sha1:path' '\n+\ttest_must_fail git rev-parse HEAD:nothing.txt 2>&1 |\n+\t\tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist in '\"'\"'HEAD'\"'\"'\" &&\n+\ttest_must_fail git rev-parse HEAD:index-only.txt 2>&1 |\n+\t\tgrep \"fatal: Path '\"'\"'index-only.txt'\"'\"' exists on disk, but not in '\"'\"'HEAD'\"'\"'.\" &&\n+\tcd subdir &&\n+\ttest_must_fail git rev-parse HEAD:file2.txt 2>&1 |\n+\t\tgrep \"Did you mean '\"'\"'HEAD:subdir/file2.txt'\"'\"'?\" &&\n+\tcd ..\n+'\n+\n+test_expect_success 'incorrect file in :path and :0:path' '\n+\ttest_must_fail git rev-parse :nothing.txt 2>&1 |\n+\t\tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist (neither on disk nor in the index).\" &&\n+\ttest_must_fail git rev-parse :1:nothing.txt 2>&1 |\n+\t\tgrep \"Path '\"'\"'nothing.txt'\"'\"' does not exist (neither on disk nor in the index).\" &&\n+\tcd subdir &&\n+\ttest_must_fail git rev-parse :file2.txt 2>&1 |\n+\t\tgrep \"Did you mean '\"'\"':0:subdir/file2.txt'\"'\"'?\" &&\n+\tcd .. &&\n+\ttest_must_fail git rev-parse :disk-only.txt 2>&1 |\n+\t\tgrep \"fatal: Path '\"'\"'disk-only.txt'\"'\"' exists on disk, but not in the index.\"\n+'\n+\n+test_done\n-- \n1.6.6.rc0.256.g6060\n"}]}