{"thread":{"id":"3151","subject":"Make git-rev-list and git-rev-parse argument parsing stricter","startedAt":"2006-01-25T22:00:37Z","lastAt":"2006-01-25T22:40:35Z","messageCount":3,"participants":["Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"15123","messageId":"Pine.LNX.4.64.0601251655580.2644@evo.osdl.org","threadId":"3151","inReplyTo":null,"subject":"Make git-rev-list and git-rev-parse argument parsing stricter","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-01-25T22:00:37Z","receivedAt":"2006-01-25T22:00:37Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nIf you pass it a filename without the \"--\" marker to separate it from\nrevision information and flags, we now require that the file in question\nactually exists. This makes mis-typed revision information not be silently\njust considered a strange filename.\n\nWith the \"--\" marker, you can continue to pass in filenames that do not\nactually exists - useful for querying what happened to a file that you\nno longer have in the repository.\n\n[ All scripts should use the \"--\" format regardless, to make things\n  unambiguous. So this change should not affect any existing tools ]\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n----\n\nI hit this the hard way when I was showing off git at Linux.Conf.Au \nyesterday, and I asked for\n\n\tgit log v2.6.14..v2.6.16\n\nwhich silently did nothing at all. The reason? I don't actually have a \nv2.6.16, the most recent version is 2.6.16-rc1. So it thought \nv2.6.14..v2.6.16 was a _filename_.\n\nNow, normally, if you use the \"proper\" format (with the -- delimeter to \nmark where filenames start), you'd never have this ambiguity. But without \nthe \"--\", the way I decided that we should disambiguate things is to \nverify that any filename given with the ambiguous shorthand version \nactually exists.\n\nComments?\n\ndiff --git a/rev-list.c b/rev-list.c\nindex d060966..e00e6fc 100644\n--- a/rev-list.c\n+++ b/rev-list.c\n@@ -844,8 +844,12 @@ int main(int argc, const char **argv)\n \t\t\targ++;\n \t\t\tlimited = 1;\n \t\t}\n-\t\tif (get_sha1(arg, sha1) < 0)\n+\t\tif (get_sha1(arg, sha1) < 0) {\n+\t\t\tstruct stat st;\n+\t\t\tif (lstat(arg, &st) < 0)\n+\t\t\t\tdie(\"'%s': %s\", arg, strerror(errno));\n \t\t\tbreak;\n+\t\t}\n \t\tcommit = get_commit_reference(arg, sha1, flags);\n \t\thandle_one_commit(commit, &list);\n \t}\ndiff --git a/rev-parse.c b/rev-parse.c\nindex 0c951af..7abad35 100644\n--- a/rev-parse.c\n+++ b/rev-parse.c\n@@ -154,6 +154,7 @@ int main(int argc, char **argv)\n \tconst char *prefix = setup_git_directory();\n \t\n \tfor (i = 1; i < argc; i++) {\n+\t\tstruct stat st;\n \t\tchar *arg = argv[i];\n \t\tchar *dotdot;\n \t\n@@ -293,6 +294,8 @@ int main(int argc, char **argv)\n \t\t}\n \t\tif (verify)\n \t\t\tdie(\"Needed a single revision\");\n+\t\tif (lstat(arg, &st) < 0)\n+\t\t\tdie(\"'%s': %s\", arg, strerror(errno));\n \t\tas_is = 1;\n \t\tshow_file(arg);\n \t}\n"},{"id":"15124","messageId":"Pine.LNX.4.64.0601251700540.2644@evo.osdl.org","threadId":"3151","inReplyTo":"Pine.LNX.4.64.0601251655580.2644@evo.osdl.org","subject":"git-whatchanged: exit out early on errors","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-01-25T22:02:10Z","receivedAt":"2006-01-25T22:02:10Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nIf we get an error parsing the arguments, exit.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\nThis goes together with the previous diff - it just makes git-whatchanged \nexit gracefully when git-rev-parse errors out.\n\ndiff --git a/git-whatchanged.sh b/git-whatchanged.sh\nindex 80e2500..d4f985b 100755\n--- a/git-whatchanged.sh\n+++ b/git-whatchanged.sh\n@@ -4,7 +4,7 @@ USAGE='[-p] [--max-count=<n>] [<since>..\n SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n-diff_tree_flags=$(git-rev-parse --sq --no-revs --flags \"$@\")\n+diff_tree_flags=$(git-rev-parse --sq --no-revs --flags \"$@\") || exit\n test -z \"$diff_tree_flags\" &&\n \tdiff_tree_flags=$(git-repo-config --get whatchanged.difftree)\n test -z \"$diff_tree_flags\" &&\n"},{"id":"15125","messageId":"7vhd7sorrw.fsf@assigned-by-dhcp.cox.net","threadId":"3151","inReplyTo":"Pine.LNX.4.64.0601251655580.2644@evo.osdl.org","subject":"Re: Make git-rev-list and git-rev-parse argument parsing stricter","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-01-25T22:40:35Z","receivedAt":"2006-01-25T22:40:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> If you pass it a filename without the \"--\" marker to separate it from\n> revision information and flags, we now require that the file in question\n> actually exists. This makes mis-typed revision information not be silently\n> just considered a strange filename.\n>...\n> Comments?\n\nI think it is a good safety measure.  People has to say \"git\nrev-list -- git-commit-script\" if they are interested in\nhistorical paths, but that is less common.  We could even go\nstronger and always require '--', but that is probably too much\nfor normal use.  I like the balance you struck here.\n"}]}