{"thread":{"id":"21798","subject":"[RFC/PATCH] Detailed diagnostic when parsing an object name fails.","startedAt":"2009-11-30T17:50:21Z","lastAt":"2009-12-07T13:46:14Z","messageCount":4,"participants":["Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"128790","messageId":"1259603421-10055-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"21798","inReplyTo":null,"subject":"[RFC/PATCH] Detailed diagnostic when parsing an object name fails.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-11-30T17:50:21Z","receivedAt":"2009-11-30T17:50:21Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The 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\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n\nThe original motivation is actually to anwser the FAQ of \"git show\nHEAD:foo.txt\" being relative to the root, while some users would\nexpect it relative to $PWD but the patch ended up being far more\ngeneral. At least, \n\n$ cd git\n$ cd t\n$ ../git show HEAD:test-lib.sh \nfatal: Path 't/test-lib.sh' exists, but not 'test-lib.sh'.\nDid you mean 'HEAD:t/test-lib.sh'?\n\nA few remarks/RFH:\n\n* It's my first time playing with the index API, so my\n  diagnose_invalid_index_path may be incorrect/suboptimal/...\n\n* This definitely requires a few test-cases, but I don't have time for\n  it right now.\n\n* Is my way of having default arguments for get_sha1_with_mode OK with\n  the usual coding style of Git?\n\n cache.h     |    6 +++-\n setup.c     |   15 +++++++++-\n sha1_name.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 101 insertions(+), 7 deletions(-)\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..172c128 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -804,7 +804,69 @@ 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 (!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+\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 (either on disk or in the index).\", 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 +912,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 +926,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 }\n-- \n1.6.6.rc0.256.g6060\n"},{"id":"128804","messageId":"7vtywbkc05.fsf@alter.siamese.dyndns.org","threadId":"21798","inReplyTo":"1259603421-10055-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC/PATCH] Detailed diagnostic when parsing an object name fails.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T20:12:42Z","receivedAt":"2009-11-30T20:12:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> The original motivation is actually to anwser the FAQ of \"git show\n> HEAD:foo.txt\" being relative to the root, while some users would\n> expect it relative to $PWD but the patch ended up being far more\n> general. At least, \n>\n> $ cd git\n> $ cd t\n> $ ../git show HEAD:test-lib.sh \n> fatal: Path 't/test-lib.sh' exists, but not 'test-lib.sh'.\n> Did you mean 'HEAD:t/test-lib.sh'?\n\nThe first thought that comes to mind is that if it makes more sense to\njust fall back to the interpretation of the input when the tool has\nalready figured out to second guess the intention of the user like the\nabove message does.\n\nThat would obviously break scripts that try to make sure the _absense_ of\na path in a tree-ish (or in the index if you lack \"HEAD\" in your example),\nexpecting:\n\n   git rev-parse HEAD:test-lib.sh ||\n   echo \"test-lib.sh shouldn't be at the top level\"\n\nto work regardless of where you are in the work tree, so it won't fly\nwell.\n\nPerhaps the second step would be to teach the machinery to understand a\nsyntax like \"<tree-ish>:./<path>\" and have it prefix the path to the\ncurrent subdirectory from the root of the work tree, and with such an\nenhancement, the suggestion given by this patch would probably change to\n\"Did you mean 'HEAD:./test-lib.sh'?\", but that would be a separate topic.\n"},{"id":"128808","messageId":"vpqiqcrd93l.fsf@bauges.imag.fr","threadId":"21798","inReplyTo":"7vtywbkc05.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] Detailed diagnostic when parsing an object name fails.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-11-30T20:57:18Z","receivedAt":"2009-11-30T20:57:18Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> $ ../git show HEAD:test-lib.sh \n>> fatal: Path 't/test-lib.sh' exists, but not 'test-lib.sh'.\n>> Did you mean 'HEAD:t/test-lib.sh'?\n>\n> The first thought that comes to mind is that if it makes more sense to\n> just fall back to the interpretation of the input when the tool has\n> already figured out to second guess the intention of the user like the\n> above message does.\n>\n> That would obviously break scripts that try to make sure the _absense_ of\n> a path in a tree-ish (or in the index if you lack \"HEAD\" in your example),\n> expecting:\n>\n>    git rev-parse HEAD:test-lib.sh ||\n>    echo \"test-lib.sh shouldn't be at the top level\"\n\nThere's another (more important IMO) problem: if HEAD contains both\nfile.txt and subdir/file.txt, then what should\n\ncd subdir\ngit show HEAD:file.txt\n\ndo? Allowing HEAD:file-in-a-subdirectory.txt would indirectly mean\nteaching users to use in, and they'd be bitten by an ambiguous\nbehavior one day or another ...\n\n> Perhaps the second step would be to teach the machinery to understand a\n> syntax like \"<tree-ish>:./<path>\" and have it prefix the path to the\n> current subdirectory from the root of the work tree, and with such an\n> enhancement, the suggestion given by this patch would probably change to\n> \"Did you mean 'HEAD:./test-lib.sh'?\", but that would be a separate\n> topic.\n\nExactly. I think this HEAD:./relative-path syntax has been discussed\nhere already, but I don't remember the outcome of the discussion. If\nit's ever implemented, my patch, modified as you suggest will help\nusers to discover the feature ;-).\n\nBTW, I finally had time to write a test. Current version below FYI\n(fixed a segfault discovered by testing too), but I'll resend properly\nwhen I get enough feedback.\n\n\n>From 1bc1c8d114b538f57a5ba4a501436229d644d587 Mon Sep 17 00:00:00 2001\nFrom: Matthieu Moy <Matthieu.Moy@imag.fr>\nDate: Mon, 30 Nov 2009 17:32:39 +0100\nSubject: [PATCH] Detailed diagnosis when parsing an object name fails.\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---\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\n\n--\nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"129423","messageId":"vpqvdgizyl5.fsf@bauges.imag.fr","threadId":"21798","inReplyTo":"vpqiqcrd93l.fsf@bauges.imag.fr","subject":"Re: [RFC/PATCH] Detailed diagnostic when parsing an object name fails.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-12-07T13:46:14Z","receivedAt":"2009-12-07T13:46:14Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n>> Perhaps the second step would be to teach the machinery to understand a\n>> syntax like \"<tree-ish>:./<path>\" and have it prefix the path to the\n>> current subdirectory from the root of the work tree, and with such an\n>> enhancement, the suggestion given by this patch would probably change to\n>> \"Did you mean 'HEAD:./test-lib.sh'?\", but that would be a separate\n>> topic.\n>\n> Exactly. I think this HEAD:./relative-path syntax has been discussed\n> here already, but I don't remember the outcome of the discussion. If\n> it's ever implemented, my patch, modified as you suggest will help\n> users to discover the feature ;-).\n\nI gave it a try, and it seems it's not as easy to implement as it\nseems.\n\nThe main task is to teach get_sha1_with_mode_1(..., prefix, ...) to\nnotice a ./ in \"HEAD:./filename\", and to replace it with prefix, which\nis easy. The problem is to get \"prefix\" reliably. Since\nget_sha1_with_mode_1 is called by get_sha1, which doesn't know about\nprefix, and which is called from at least 92 places in the code, this\nwould require changing all these calls to add the \"prefix\" argument\n(and to find out where to get this prefix from).\n\nSo, I guess I'll give up :-(. (unless someone either show a great\nmotivation for the feature, or a magic formula to make the patch\nshort)\n\nFYI, here's the toy patch I started (works for \"git show HEAD:./file\",\nbut not for \"git rev-parse HEAD:./file\" for example):\n\ndiff --git a/cache.h b/cache.h\nindex c122bfa..a44f06f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -708,10 +708,10 @@ 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_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);\n-static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)\n+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, const char *prefix, int gently);\n+static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode, const char *prefix)\n {\n-\treturn get_sha1_with_mode_1(str, sha1, mode, 1, NULL);\n+\treturn get_sha1_with_mode_1(str, sha1, mode, prefix, 1);\n }\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! */\ndiff --git a/revision.c b/revision.c\nindex 25fa14d..19ddb21 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -944,7 +944,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \t\tlocal_flags = UNINTERESTING;\n \t\targ++;\n \t}\n-\tif (get_sha1_with_mode(arg, sha1, &mode))\n+\tif (get_sha1_with_mode(arg, sha1, &mode, revs->prefix))\n \t\treturn -1;\n \tif (!cant_be_filename)\n \t\tverify_non_filename(revs->prefix, arg);\n@@ -1419,7 +1419,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\tunsigned char sha1[20];\n \t\tstruct object *object;\n \t\tunsigned mode;\n-\t\tif (get_sha1_with_mode(revs->def, sha1, &mode))\n+\t\tif (get_sha1_with_mode(revs->def, sha1, &mode, revs->prefix))\n \t\t\tdie(\"bad default revision '%s'\", revs->def);\n \t\tobject = get_reference(revs, revs->def, sha1, 0);\n \t\tadd_pending_object_with_mode(revs, object, revs->def, mode);\ndiff --git a/setup.c b/setup.c\nindex 5792eb7..60c1e08 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -79,7 +79,7 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg)\n \tunsigned char sha1[20];\n \tunsigned mode;\n \t/* try a detailed diagnostic ... */\n-\tget_sha1_with_mode_1(arg, sha1, &mode, 0, prefix);\n+\tget_sha1_with_mode_1(arg, sha1, &mode, prefix, 0);\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);\ndiff --git a/sha1_name.c b/sha1_name.c\nindex ca8f9db..330d3fe 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -801,7 +801,7 @@ release_return:\n int get_sha1(const char *name, unsigned char *sha1)\n {\n \tunsigned unused;\n-\treturn get_sha1_with_mode(name, sha1, &unused);\n+\treturn get_sha1_with_mode(name, sha1, &unused, NULL);\n }\n \n /* Must be called only when object_name:filename doesn't exist. */\n@@ -893,7 +893,7 @@ static void diagnose_invalid_index_path(int stage,\n }\n \n \n-int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)\n+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, const char *prefix, int gently)\n {\n \tint ret, bracket_depth;\n \tint namelen = strlen(name);\n@@ -961,11 +961,26 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,\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 (!gently) {\n-\t\t\t\tdiagnose_invalid_sha1_path(prefix, filename,\n-\t\t\t\t\t\t\t   tree_sha1, object_name);\n-\t\t\t\tfree(object_name);\n+\t\t\tif (filename[0] == '.' && filename[1] == '/') {\n+\t\t\t\tif (!prefix)\n+\t\t\t\t\tprefix = \"\";\n+\t\t\t\tchar *absfilename = xmalloc((strlen(filename) - 2)\n+\t\t\t\t\t\t\t    + strlen(prefix) + 1);\n+\t\t\t\tstrcpy(absfilename, prefix);\n+\t\t\t\tstrcat(absfilename, filename+2);\n+\t\t\t\tret = get_tree_entry(tree_sha1, absfilename, sha1, mode);\n+\t\t\t\tif (!gently) {\n+\t\t\t\t\tdiagnose_invalid_sha1_path(prefix, absfilename,\n+\t\t\t\t\t\t\t\t   tree_sha1, object_name);\n+\t\t\t\t\tfree(object_name);\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tret = get_tree_entry(tree_sha1, filename, sha1, mode);\n+\t\t\t\tif (!gently) {\n+\t\t\t\t\tdiagnose_invalid_sha1_path(prefix, filename,\n+\t\t\t\t\t\t\t\t   tree_sha1, object_name);\n+\t\t\t\t\tfree(object_name);\n+\t\t\t\t}\n \t\t\t}\n \t\t\treturn ret;\n \t\t} else {\n\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}