{"thread":{"id":"10232","subject":"git-rev-parse --verify could be friendlier","startedAt":"2007-10-10T14:24:17Z","lastAt":"2007-10-10T16:37:22Z","messageCount":2,"participants":["linux@horizon.com","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"55370","messageId":"20071010142417.8892.qmail@science.horizon.com","threadId":"10232","inReplyTo":null,"subject":"git-rev-parse --verify could be friendlier","fromName":"","fromEmail":"linux@horizon.com","sentAt":"2007-10-10T14:24:17Z","receivedAt":"2007-10-10T14:24:17Z","isPatch":false,"sender":{"key":"linux@horizon.com","avatar":null},"body":"I accidentally typed \"git rebase --onto typo ancestor\", where \"typo\"\nwas a branch that didn't exist.\n\nThe error message (which is actually generated by git-rev-parse --verify)\nis \"fatal: Needed a single revision\".\n\nThis is perhaps a bit obscure.  For starters, it doesn't even tell me\nwhich argument is problematic.\n\nIf I do \"git rebase --onto dest typo\", I at least get\nfatal: Needed a single revision\ninvalid upstream typo\n\nSomething like the following would certainly help, but perhaps git-rev-parse\ncould be slightly more forthcoming, too?\n\ndiff --git a/git-rebase b/git-rebase\nindex 058fcac..b14ac95 100755\n--- a/git-rebase\n+++ b/git-rebase\n@@ -268,7 +268,8 @@ upstream=`git rev-parse --verify \"${upstream_name}^0\"` ||\n \n # Make sure the branch to rebase onto is valid.\n onto_name=${newbase-\"$upstream_name\"}\n-onto=$(git rev-parse --verify \"${onto_name}^0\") || exit\n+onto=$(git rev-parse --verify \"${onto_name}^0\") ||\n+    die \"invalid target $upstream_name\"\n \n # If a hook exists, give it a chance to interrupt\n if test -x \"$GIT_DIR/hooks/pre-rebase\"\n@@ -294,7 +295,8 @@ case \"$#\" in\n \tfi\n \t;;\n esac\n-branch=$(git rev-parse --verify \"${branch_name}^0\") || exit\n+branch=$(git rev-parse --verify \"${branch_name}^0\") ||\n+    die \"invalid branch name $upstream_name\"\n \n # Now we are rebasing commits $upstream..$branch on top of $onto\n \n"},{"id":"55387","messageId":"vpq3awj0vul.fsf@bauges.imag.fr","threadId":"10232","inReplyTo":"20071010142417.8892.qmail@science.horizon.com","subject":"Re: git-rev-parse --verify could be friendlier","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-10-10T16:37:22Z","receivedAt":"2007-10-10T16:37:22Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"linux@horizon.com writes:\n\n> Something like the following would certainly help, but perhaps git-rev-parse\n> could be slightly more forthcoming, too?\n\nI think this should be fixed in git-rev-parse itself, yes.\n\nFollowing is an attempt to make the message better. It still has at\nleast the following problems (which are already in today's git):\n\n* Duplicate error message when the revision is ambiguous.\n\n* No distinction between syntactically incorrect revisions\n  (impossible~branchname for example), and inexisting revision\n  (a34b6c, if there's no such revision).\n\nWhat would be really cool is to extend this to have a fine-granularity\nerror number returned by get_sha1 and friends.\n\n\n>From f033ba755e6bc46dcd0f5767699458bacb840587 Mon Sep 17 00:00:00 2001\nFrom: Matthieu Moy <Matthieu.Moy@imag.fr>\nDate: Wed, 10 Oct 2007 18:15:39 +0200\nSubject: [PATCH] Better error message for git-rev-parse.\n\nThe existing error message is \"Needed a single revision\", which is\ninacurate if the revision provided is syntactically incorrect or\ninexistant.\n---\n builtin-rev-parse.c |   20 ++++++++++++++++----\n cache.h             |    3 +++\n sha1_name.c         |    5 +----\n 3 files changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex 8d78b69..d2145f1 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -213,6 +213,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n {\n \tint i, as_is = 0, verify = 0;\n \tunsigned char sha1[20];\n+\tconst char *error_param;\n+\tint err_no = 0;\n \n \tgit_config(git_default_config);\n \n@@ -249,6 +251,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--default\")) {\n \t\t\t\tdef = argv[i+1];\n+\t\t\t\terror_param = def;\n \t\t\t\ti++;\n \t\t\t\tcontinue;\n \t\t\t}\n@@ -390,16 +393,20 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \n+\t\terror_param = arg;\n \t\t/* Not a flag argument */\n \t\tif (try_difference(arg))\n \t\t\tcontinue;\n-\t\tif (!get_sha1(arg, sha1)) {\n+\t\tif (!(err_no = get_sha1(arg, sha1))) {\n \t\t\tshow_rev(NORMAL, sha1, arg);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (*arg == '^' && !get_sha1(arg+1, sha1)) {\n-\t\t\tshow_rev(REVERSED, sha1, arg+1);\n-\t\t\tcontinue;\n+\t\tif (*arg == '^') {\n+\t\t\terror_param = arg+1;\n+\t\t\tif (!(err_no = get_sha1(arg+1, sha1))) {\n+\t\t\t\tshow_rev(REVERSED, sha1, arg+1);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t}\n \t\tas_is = 1;\n \t\tif (!show_file(arg))\n@@ -409,6 +416,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\tverify_filename(prefix, arg);\n \t}\n \tshow_default();\n+\tif (verify && revs_count == 0)\n+\t\tif (err_no == SHORT_NAME_AMBIGUOUS)\n+\t\t\tdie(\"%s: Ambiguous revision\", error_param);\n+\t\telse\n+\t\t\tdie(\"%s: No such revision\", error_param);\n \tif (verify && revs_count != 1)\n \t\tdie(\"Needed a single revision\");\n \treturn 0;\ndiff --git a/cache.h b/cache.h\nindex e0abcd6..a537afa 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -399,6 +399,9 @@ static inline unsigned int hexval(unsigned char c)\n #define MINIMUM_ABBREV 4\n #define DEFAULT_ABBREV 7\n \n+#define SHORT_NAME_NOT_FOUND (-1)\n+#define SHORT_NAME_AMBIGUOUS (-2)\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 extern int get_sha1_hex(const char *hex, unsigned char *sha1);\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 2d727d5..5091420 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -127,9 +127,6 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \treturn found;\n }\n \n-#define SHORT_NAME_NOT_FOUND (-1)\n-#define SHORT_NAME_AMBIGUOUS (-2)\n-\n static int find_unique_short_object(int len, char *canonical,\n \t\t\t\t    unsigned char *res, unsigned char *sha1)\n {\n@@ -186,7 +183,7 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \n \tstatus = find_unique_short_object(i, canonical, res, sha1);\n \tif (!quietly && (status == SHORT_NAME_AMBIGUOUS))\n-\t\treturn error(\"short SHA1 %.*s is ambiguous.\", len, canonical);\n+\t\terror(\"short SHA1 %.*s is ambiguous.\", len, canonical);\n \treturn status;\n }\n \n-- \n1.5.3.4.205.g52b6c\n\n\n\n-- \nMatthieu\n"}]}