{"thread":{"id":"1266","subject":"[RFC/PATCH] git-rev-parse vs IFS","startedAt":"2005-07-17T06:59:57Z","lastAt":"2005-07-18T17:27:01Z","messageCount":4,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"6209","messageId":"7vmzol29zm.fsf@assigned-by-dhcp.cox.net","threadId":"1266","inReplyTo":null,"subject":"[RFC/PATCH] git-rev-parse vs IFS","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-07-17T06:59:57Z","receivedAt":"2005-07-17T06:59:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git whatchanged\" does not work with parameters that have an IFS\ncharacter; this includes a filename with SP in it.  A silly\nexample:\n\n    $ git-whatchanged --max-count=1\n    diff-tree 98800286dffc76736549d7279622157ca2be9a3b (from 9171e0...\n    Author: Junio C Hamano <junkio@cox.net>\n    Date:   Sat Jul 16 23:13:03 2005 -0700\n\n        add foo\n\n    :000000 100644 0000000000000000000000000000000000000000 8b13789...\n    $ git-whatchanged --max-count=1 'foo bar'\n    $ ;# no output!\n\nThis is because the tail parameter to git-diff-tree used in\ngit-whatchanged is $(git-rev-parse --no-revs \"$@\"), and the\noutput from git-rev-parse is split at $IFS, so it ends up\ngetting \"foo\" and \"bar\" separately).\n\nIf it were just embedded SP and TAB in the filnames, I would say\nwe can set IFS to LF in the script that use git-rev-parse,\ndeclare that we do not support filenames with embedded LF, and\nbe done with it.  The thing is, for pickaxe, it is very natural\nto give a multi-line parameter, so setting IFS to LF is not a\ngeneral workaround.\n\nDoing this portably is possible but a bit of pain.  I can add\ngit-rev-parse ability to sq_quote its outputs, and change the\ncallers to build a command line to \"eval\" using it.  Here is an\nRFC patch that does it.  My barf-o-tolerance for shell quoting\nand evaling is usually much higher than others, so while I feel\nthis is not too ugly, it may offend your aesthetics.\n\n------------\nHelp scripts that use git-rev-parse to grok args with SP/TAB/LF\n\nThe git-rev-parse command uses LF to separate each arguments it\nparses, so its users at least need to set IFS to LF to be able\nto handle filenames with embedded SPs and TABs.  Some commands,\nhowever, can take and do expect arguments with embedded LF\n(notably, -Spickaxe of diff family), so even this workaround\ndoes not work for them.\n\nWhen --sq flag to git-rev-parse is given, instead of showing one\nargument per line, it outputs arguments quoted for consumption\nwith \"eval\" by the caller, to remedy this situation.\n\nAs an example, this patch converts git-whatchanged to use this\nnew feature.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n git-whatchanged |    9 ++++++---\n rev-parse.c     |   36 ++++++++++++++++++++++++++++++++++--\n 2 files changed, 40 insertions(+), 5 deletions(-)\n\n79cdb9505b0ee9a47969c3cbc8c8bf2a35b23ca8\ndiff --git a/git-whatchanged b/git-whatchanged\n--- a/git-whatchanged\n+++ b/git-whatchanged\n@@ -1,4 +1,7 @@\n #!/bin/sh\n-git-rev-list $(git-rev-parse --default HEAD --revs-only \"$@\") |\n-\tgit-diff-tree --stdin --pretty -r $(git-rev-parse --no-revs \"$@\") |\n-\tLESS=\"$LESS -S\" ${PAGER:-less}\n+rev_list_args=$(git-rev-parse --sq --default HEAD --revs-only \"$@\") &&\n+diff_tree_args=$(git-rev-parse --sq --no-revs \"$@\") &&\n+\n+eval \"git-rev-list $rev_list_args\" |\n+eval \"git-diff-tree --stdin --pretty -r $diff_tree_args\" |\n+LESS=\"$LESS -S\" ${PAGER:-less}\ndiff --git a/rev-parse.c b/rev-parse.c\n--- a/rev-parse.c\n+++ b/rev-parse.c\n@@ -15,6 +15,7 @@ static int do_rev_argument = 1;\n static int output_revs = 0;\n static int flags_only = 0;\n static int no_flags = 0;\n+static int output_sq = 0;\n \n #define NORMAL 0\n #define REVERSED 1\n@@ -54,14 +55,41 @@ static void show_rev(int type, const uns\n \tif (no_revs)\n \t\treturn;\n \toutput_revs++;\n+\n+\t/* Hexadecimal string plus possibly a carret;\n+\t * this does not have to be quoted even under output_sq.\n+\t */\n \tprintf(\"%s%s\\n\", type == show_type ? \"\" : \"^\", sha1_to_hex(sha1));\n }\n \n+static void show(const char *arg)\n+{\n+\tif (output_sq) {\n+\t\tint sq = '\\'';\n+\t\tconst char *next;\n+\n+\t\tputchar(sq);\n+\t\twhile (*arg) {\n+\t\t\tnext = strchr(arg, sq);\n+\t\t\tif (!next) {\n+\t\t\t\tfputs(arg, stdout);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tfputs(\"'\\\\''\", stdout);\n+\t\t\targ = next + 1;\n+\t\t}\n+\t\tputchar(sq);\n+\t\tputchar('\\n');\n+\t}\n+\telse\n+\t\tputs(arg);\n+}\n+\n static void show_rev_arg(char *rev)\n {\n \tif (no_revs)\n \t\treturn;\n-\tputs(rev);\n+\tshow(rev);\n }\n \n static void show_norev(char *norev)\n@@ -70,7 +98,7 @@ static void show_norev(char *norev)\n \t\treturn;\n \tif (revs_only)\n \t\treturn;\n-\tputs(norev);\n+\tshow(norev);\n }\n \n static void show_arg(char *arg)\n@@ -328,6 +356,10 @@ int main(int argc, char **argv)\n \t\t\t\tsingle_rev = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--sq\")) {\n+\t\t\t\toutput_sq = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--not\")) {\n \t\t\t\tshow_type ^= REVERSED;\n \t\t\t\tcontinue;\n"},{"id":"6210","messageId":"7vslyd3lhk.fsf@assigned-by-dhcp.cox.net","threadId":"1266","inReplyTo":"7vmzol29zm.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] rev-parse --sq bugfixes.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-07-17T08:06:15Z","receivedAt":"2005-07-17T08:06:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We expect --sq output to be used to construct a string to be\n\"eval\"ed in shell scripts, so we should not be separating the\nparameter with LF, which would cause the current command to be\nended.\n\nAlso there was a silly thinko for doing the shell quoting.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n*** This is a fix on top of the \"git-rev-parse vs IFS\" RFC\n*** patch.\n\n rev-parse.c |   29 ++++++++++++++++-------------\n 1 files changed, 16 insertions(+), 13 deletions(-)\n\nbf5a2b2f3cd5693560ab72974f9956c76a8fd667\ndiff --git a/rev-parse.c b/rev-parse.c\n--- a/rev-parse.c\n+++ b/rev-parse.c\n@@ -50,18 +50,6 @@ static int is_rev_argument(const char *a\n \t}\n }\n \n-static void show_rev(int type, const unsigned char *sha1)\n-{\n-\tif (no_revs)\n-\t\treturn;\n-\toutput_revs++;\n-\n-\t/* Hexadecimal string plus possibly a carret;\n-\t * this does not have to be quoted even under output_sq.\n-\t */\n-\tprintf(\"%s%s\\n\", type == show_type ? \"\" : \"^\", sha1_to_hex(sha1));\n-}\n-\n static void show(const char *arg)\n {\n \tif (output_sq) {\n@@ -75,16 +63,31 @@ static void show(const char *arg)\n \t\t\t\tfputs(arg, stdout);\n \t\t\t\tbreak;\n \t\t\t}\n+\t\t\twhile (arg < next)\n+\t\t\t\tputchar(*arg++);\n \t\t\tfputs(\"'\\\\''\", stdout);\n \t\t\targ = next + 1;\n \t\t}\n \t\tputchar(sq);\n-\t\tputchar('\\n');\n+\t\tputchar(' ');\n \t}\n \telse\n \t\tputs(arg);\n }\n \n+static void show_rev(int type, const unsigned char *sha1)\n+{\n+\tif (no_revs)\n+\t\treturn;\n+\toutput_revs++;\n+\n+\t/* Hexadecimal string plus possibly a carret;\n+\t * this does not have to be quoted even under output_sq.\n+\t */\n+\tprintf(\"%s%s%c\", type == show_type ? \"\" : \"^\", sha1_to_hex(sha1),\n+\t       output_sq ? ' ' : '\\n');\n+}\n+\n static void show_rev_arg(char *rev)\n {\n \tif (no_revs)\n"},{"id":"6214","messageId":"7v8y053jok.fsf@assigned-by-dhcp.cox.net","threadId":"1266","inReplyTo":"7vmzol29zm.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Audit rev-parse users.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-07-17T08:45:15Z","receivedAt":"2005-07-17T08:45:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This patch changes rev-parse users that pass a single argument\nthat is supposed to be a rev parameter to use \"--verify\".\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n*** This does not have anything to do with the --sq flag.\n\n git-checkout-script |    2 +-\n git-cherry          |    8 ++++----\n git-commit-script   |    2 +-\n git-rebase-script   |    8 ++++----\n git-resolve-script  |    4 ++--\n 5 files changed, 12 insertions(+), 12 deletions(-)\n\neee0f165b4762203c0d827bae7480daf41514f17\ndiff --git a/git-checkout-script b/git-checkout-script\n--- a/git-checkout-script\n+++ b/git-checkout-script\n@@ -22,7 +22,7 @@ while [ \"$#\" != \"0\" ]; do\n \t\tforce=1\n \t\t;;\n \t*)\n-\t\trev=$(git-rev-parse --verify --revs-only \"$arg^0\") || exit\n+\t\trev=$(git-rev-parse --verify \"$arg^0\") || exit\n \t\tif [ -z \"$rev\" ]; then\n \t\t\techo \"unknown flag $arg\"\n \t\t\texit 1\ndiff --git a/git-cherry b/git-cherry\n--- a/git-cherry\n+++ b/git-cherry\n@@ -28,11 +28,11 @@ The output is intended to be used as:\n '\n \n case \"$#\" in\n-1) linus=`git-rev-parse \"$1\"` &&\n-   junio=`git-rev-parse HEAD` || exit\n+1) linus=`git-rev-parse --verify \"$1\"` &&\n+   junio=`git-rev-parse --verify HEAD` || exit\n    ;;\n-2) linus=`git-rev-parse \"$1\"` &&\n-   junio=`git-rev-parse \"$2\"` || exit\n+2) linus=`git-rev-parse --verify \"$1\"` &&\n+   junio=`git-rev-parse --verify \"$2\"` || exit\n    ;;\n *) echo >&2 \"$usage\"; exit 1 ;;\n esac\ndiff --git a/git-commit-script b/git-commit-script\n--- a/git-commit-script\n+++ b/git-commit-script\n@@ -15,7 +15,7 @@ do\n     -m) shift\n         case \"$#\" in\n \t0) usage ;;\n-\t*) use_commit=`git-rev-parse \"$1\"` ||\n+\t*) use_commit=`git-rev-parse --verify \"$1\"` ||\n \t   exit ;;\n \tesac\n \t;;\ndiff --git a/git-rebase-script b/git-rebase-script\n--- a/git-rebase-script\n+++ b/git-rebase-script\n@@ -11,11 +11,11 @@ upstream tree.'\n : ${GIT_DIR=.git}\n \n case \"$#\" in\n-1) linus=`git-rev-parse \"$1\"` &&\n-   junio=`git-rev-parse HEAD` || exit\n+1) linus=`git-rev-parse --verify \"$1\"` &&\n+   junio=`git-rev-parse --verify HEAD` || exit\n    ;;\n-2) linus=`git-rev-parse \"$1\"` &&\n-   junio=`git-rev-parse \"$2\"` || exit\n+2) linus=`git-rev-parse --verify \"$1\"` &&\n+   junio=`git-rev-parse --verify \"$2\"` || exit\n    ;;\n *) echo >&2 \"$usage\"; exit 1 ;;\n esac\ndiff --git a/git-resolve-script b/git-resolve-script\n--- a/git-resolve-script\n+++ b/git-resolve-script\n@@ -6,8 +6,8 @@\n #\n . git-sh-setup-script || die \"Not a git archive\"\n \n-head=$(git-rev-parse --revs-only \"$1\")\n-merge=$(git-rev-parse --revs-only \"$2\")\n+head=$(git-rev-parse --verify \"$1\")\n+merge=$(git-rev-parse --verify \"$2\")\n merge_msg=\"$3\"\n \n dropheads() {\n"},{"id":"6250","messageId":"7vbr50t47u.fsf_-_@assigned-by-dhcp.cox.net","threadId":"1266","inReplyTo":"7vslyd3lhk.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Help scripts that use git-rev-parse to grok args with SP/TAB/LF","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-07-18T17:27:01Z","receivedAt":"2005-07-18T17:27:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The git-rev-parse command uses LF to separate each arguments it\nparses, so its users at least need to set IFS to LF to be able\nto handle filenames with embedded SPs and TABs.  Some commands,\nhowever, can take and do expect arguments with embedded LF\n(notably, -Spickaxe of diff family), so even this workaround\ndoes not work for them.\n\nWhen --sq flag to git-rev-parse is given, instead of showing one\nargument per line, it outputs arguments quoted for consumption\nwith \"eval\" by the caller, to remedy this situation.\n\nAs an example, this patch converts git-whatchanged to use this\nfeature.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n*** This is a replacement for two patches I sent earlier:\n***   [RFC/PATCH] git-rev-parse vs IFS\n***   [PATCH] rev-parse --sq bugfixes.\n\n git-whatchanged |    9 ++++++---\n rev-parse.c     |   34 +++++++++++++++++++++++++++++++---\n 2 files changed, 37 insertions(+), 6 deletions(-)\n\na99f0d64fae9a121a1ecbfd07d7667cf4aa0f19d\ndiff --git a/git-whatchanged b/git-whatchanged\n--- a/git-whatchanged\n+++ b/git-whatchanged\n@@ -1,4 +1,7 @@\n #!/bin/sh\n-git-rev-list $(git-rev-parse --default HEAD --revs-only \"$@\") |\n-\tgit-diff-tree --stdin --pretty -r $(git-rev-parse --no-revs \"$@\") |\n-\tLESS=\"$LESS -S\" ${PAGER:-less}\n+rev_list_args=$(git-rev-parse --sq --default HEAD --revs-only \"$@\") &&\n+diff_tree_args=$(git-rev-parse --sq --no-revs \"$@\") &&\n+\n+eval \"git-rev-list $rev_list_args\" |\n+eval \"git-diff-tree --stdin --pretty -r $diff_tree_args\" |\n+LESS=\"$LESS -S\" ${PAGER:-less}\ndiff --git a/rev-parse.c b/rev-parse.c\n--- a/rev-parse.c\n+++ b/rev-parse.c\n@@ -15,6 +15,7 @@ static int do_rev_argument = 1;\n static int output_revs = 0;\n static int flags_only = 0;\n static int no_flags = 0;\n+static int output_sq = 0;\n \n #define NORMAL 0\n #define REVERSED 1\n@@ -49,19 +50,42 @@ static int is_rev_argument(const char *a\n \t}\n }\n \n+static void show(const char *arg)\n+{\n+\tif (output_sq) {\n+\t\tint sq = '\\'', ch;\n+\n+\t\tputchar(sq);\n+\t\twhile ((ch = *arg++)) {\n+\t\t\tif (ch == sq)\n+\t\t\t\tfputs(\"'\\\\'\", stdout);\n+\t\t\tputchar(ch);\n+\t\t}\n+\t\tputchar(sq);\n+\t\tputchar(' ');\n+\t}\n+\telse\n+\t\tputs(arg);\n+}\n+\n static void show_rev(int type, const unsigned char *sha1)\n {\n \tif (no_revs)\n \t\treturn;\n \toutput_revs++;\n-\tprintf(\"%s%s\\n\", type == show_type ? \"\" : \"^\", sha1_to_hex(sha1));\n+\n+\t/* Hexadecimal string plus possibly a carret;\n+\t * this does not have to be quoted even under output_sq.\n+\t */\n+\tprintf(\"%s%s%c\", type == show_type ? \"\" : \"^\", sha1_to_hex(sha1),\n+\t       output_sq ? ' ' : '\\n');\n }\n \n static void show_rev_arg(char *rev)\n {\n \tif (no_revs)\n \t\treturn;\n-\tputs(rev);\n+\tshow(rev);\n }\n \n static void show_norev(char *norev)\n@@ -70,7 +94,7 @@ static void show_norev(char *norev)\n \t\treturn;\n \tif (revs_only)\n \t\treturn;\n-\tputs(norev);\n+\tshow(norev);\n }\n \n static void show_arg(char *arg)\n@@ -328,6 +352,10 @@ int main(int argc, char **argv)\n \t\t\t\tsingle_rev = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--sq\")) {\n+\t\t\t\toutput_sq = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--not\")) {\n \t\t\t\tshow_type ^= REVERSED;\n \t\t\t\tcontinue;\n"}]}