{"thread":{"id":"26099","subject":"smudge/clean filter needs filename","startedAt":"2010-12-18T22:38:22Z","lastAt":"2010-12-22T23:22:35Z","messageCount":17,"participants":["Pete Wyckoff","Junio C Hamano","Jeff King","david@lang.hm","Johannes Sixt","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"158346","messageId":"20101218223822.GA18902@arf.padd.com","threadId":"26099","inReplyTo":null,"subject":"smudge/clean filter needs filename","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2010-12-18T22:38:22Z","receivedAt":"2010-12-18T22:38:22Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"I'm using git-p4 to import and work with upstream p4\nrepositories.  Some of the files are ktext, meaning they expect\nexpansion of $Id$ and similar identifiers.\n\nUsing the filter driver for this file, I can do the \"clean\" part\neasily, but to calculate the \"smudge\" correctly, I need to know\nthe filename inside the filter driver.\n\nE.g., inside file foo/Makefile, the clean line:\n\n    # $File$\n\nshould be smudged into:\n\n    # $File: //depot/project/foo/Makefile $\n\nI know the //depot/project location from context in the commit\nlog message that git-p4 produces.  But I don't know the pathname\nin the git repo that my smudge script is working on.\n\nWould it make sense to pass that on the command line?  E.g.\n\n    [filter \"p4\"]\n    \tclean  = git-p4smudge --clean %s\n    \tsmudge = git-p4smudge --smudge %s\n\nOr maybe put the path in an environment variable?\n\n\t\t-- Pete\n"},{"id":"158371","messageId":"20101219212925.GA7393@arf.padd.com","threadId":"26099","inReplyTo":"20101218223822.GA18902@arf.padd.com","subject":"[PATCH] convert filter: supply path to external driver","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2010-12-19T21:29:25Z","receivedAt":"2010-12-19T21:29:25Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Filtering to support keyword expansion may need the name of\nthe file being filtered.  In particular, to support p4 keywords\nlike\n\n    $File: //depot/product/dir/script.sh $\n\nthe smudge filter needs to know the name of the file it is\nsmudging.\n\nAdd a \"%s\" conversion specifier to the gitattribute for filter.\nIt will be expanded with the path name to the file when invoking\nthe external filter command.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n\npw@padd.com wrote on Sat, 18 Dec 2010 17:38 -0500:\n> I'm using git-p4 to import and work with upstream p4\n> repositories.  Some of the files are ktext, meaning they expect\n> expansion of $Id$ and similar identifiers.\n> \n> Using the filter driver for this file, I can do the \"clean\" part\n> easily, but to calculate the \"smudge\" correctly, I need to know\n> the filename inside the filter driver.\n\nThis works fine for me.  It is backward compatible, and leaves\nopen the possibility of adding other % modifiers if we find\na need later.\n\nI have a filter that handles all the p4 $Keyword$ expansions,\nnow, using this patch.  It can go into contrib if others would\nfind it useful.  This seriously reduces many hassles associated\nwith my git-p4 workflow, where a plethora of ancillary systems\nrely on keyword expansion in the source.\n\nCc Junio for comments as the original author of convert filter\ncode.\n\n\t\t-- Pete\n\n\n Documentation/gitattributes.txt |   12 ++++++++++++\n convert.c                       |   15 ++++++++++++++-\n 2 files changed, 26 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 564586b..9ac2138 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -317,6 +317,18 @@ command is \"cat\").\n \tsmudge = cat\n ------------------------\n \n+If your filter needs the path of the file it is working on,\n+you can use the \"%s\" conversion specification.  It will be\n+replaced with the relative path to the file.  This is important\n+for keyword substitution that depends on the name of the\n+file.  Like this:\n+\n+------------------------\n+[filter \"p4\"]\n+\tclean = git-p4-filter --clean %s\n+\tsmudge = git-p4-filter --smudge %s\n+------------------------\n+\n \n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\ndiff --git a/convert.c b/convert.c\nindex e41a31e..d4c6ed1 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -317,6 +317,7 @@ struct filter_params {\n \tconst char *src;\n \tunsigned long size;\n \tconst char *cmd;\n+\tconst char *path;\n };\n \n static int filter_buffer(int in, int out, void *data)\n@@ -329,7 +330,16 @@ static int filter_buffer(int in, int out, void *data)\n \tint write_err, status;\n \tconst char *argv[] = { NULL, NULL };\n \n-\targv[0] = params->cmd;\n+\t/* replace optional %s with path */\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct strbuf_expand_dict_entry dict[] = {\n+\t    \"s\", params->path,\n+\t    NULL, NULL,\n+\t};\n+\n+\tstrbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);\n+\n+\targv[0] = cmd.buf;\n \n \tmemset(&child_process, 0, sizeof(child_process));\n \tchild_process.argv = argv;\n@@ -349,6 +359,8 @@ static int filter_buffer(int in, int out, void *data)\n \tstatus = finish_command(&child_process);\n \tif (status)\n \t\terror(\"external filter %s failed %d\", params->cmd, status);\n+\n+\tstrbuf_release(&cmd);\n \treturn (write_err || status);\n }\n \n@@ -376,6 +388,7 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tparams.src = src;\n \tparams.size = len;\n \tparams.cmd = cmd;\n+\tparams.path = path;\n \n \tfflush(NULL);\n \tif (start_async(&async))\n-- \n1.7.2.3\n"},{"id":"158372","messageId":"7vzks1e84p.fsf@alter.siamese.dyndns.org","threadId":"26099","inReplyTo":"20101219212925.GA7393@arf.padd.com","subject":"Re: [PATCH] convert filter: supply path to external driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-19T21:59:50Z","receivedAt":"2010-12-19T21:59:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> Filtering to support keyword expansion may need the name of\n> the file being filtered.  In particular, to support p4 keywords\n> like\n>\n>     $File: //depot/product/dir/script.sh $\n>\n> the smudge filter needs to know the name of the file it is\n> smudging.\n>\n> Add a \"%s\" conversion specifier to the gitattribute for filter.\n> It will be expanded with the path name to the file when invoking\n> the external filter command.\n>\n> Signed-off-by: Pete Wyckoff <pw@padd.com>\n> ---\n>\n> pw@padd.com wrote on Sat, 18 Dec 2010 17:38 -0500:\n>> I'm using git-p4 to import and work with upstream p4\n>> repositories.  Some of the files are ktext, meaning they expect\n>> expansion of $Id$ and similar identifiers.\n>> \n>> Using the filter driver for this file, I can do the \"clean\" part\n>> easily, but to calculate the \"smudge\" correctly, I need to know\n>> the filename inside the filter driver.\n>\n> This works fine for me.  It is backward compatible, and leaves\n> open the possibility of adding other % modifiers if we find\n> a need later.\n\nThis is not backward compatible for people who wanted to use '%' literal\non their filter command line for whatever reason, so please do not\nadvertise as such.  A fair argument you could make is \"Even though this is\nnot strictly backward compatible, it is very unlikely that people passed a\nliteral % to their filter command line, and the benefit of being able to\ngive the pathname information would outweigh the downside of not being\ncompatible\", and people can agree or disagree.\n\nI am personally moderately negative about $any expansion$ (I don't use it\nmyself, and I don't think sane people use it either).  As far as I can\ntell, this should has no impact on the correctness and very little impact\non the performance for people who do not use $any expansion$, so I am Ok\nwith the patch.\n\nModulo one worry.  Don't we have, or don't we at least plant to allow us\nto have, a facility to cache expensive blob conversion result, similar to\nthe textconv caching?  How would this change interact with two blobs that\nlive in different paths?\n"},{"id":"158377","messageId":"20101220022409.GA4625@sigill.intra.peff.net","threadId":"26099","inReplyTo":"7vzks1e84p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] convert filter: supply path to external driver","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-12-20T02:24:09Z","receivedAt":"2010-12-20T02:24:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 19, 2010 at 01:59:50PM -0800, Junio C Hamano wrote:\n\n> Modulo one worry.  Don't we have, or don't we at least plant to allow us\n> to have, a facility to cache expensive blob conversion result, similar to\n> the textconv caching?  How would this change interact with two blobs that\n> live in different paths?\n\nYeah, it has been talked about, but I don't think anyone is working on\nit (I don't personally use clean/smudge at all, so it is not something I\nhave thought _that_ much about).\n\nThis does definitely complicate matters, as the filtering is no longer a\npure mapping of sha1->sha1. However, I think in practice we could do\njust fine by using a multi-level lookup. I.e., mapping a sha1 to be\nfiltered into a tree. Each tree entry would represent the remaining\ncache parameters. In this case, the only other parameter we have is the\npath given to the filter (but it could easily be extended to include\nother parameters, if they existed, in this or other caching cases).\n\nWe only get a high-performance lookup for the first part of the\nmulti-level (i.e., the sha1), but that's OK if we assume the number of\nsecond-level items is going to be small. Which I think is the case here\n(a sha1 will tend to be found only under one or a few names).\n\nAn alternative would be to combine all parts of the filter under a\nsingle lookup key. E.g., calculate and store under sha1(sha1(blob) +\nfilename)). But that means the notes keys are not actual object sha1s,\nwhich throws off pruning.\n\nAnyway, that's just my quick thinking on the subject. I don't see any\nreason to restrict a feature just because we might want to cache it in\nthe future. At the very worst, we could always cache filters which do\nnot use %s, and make only %s users pay the penalty.\n\n-Peff\n"},{"id":"158386","messageId":"alpine.DEB.2.00.1012192151090.20181@asgard.lang.hm","threadId":"26099","inReplyTo":"20101220022409.GA4625@sigill.intra.peff.net","subject":"Re: [PATCH] convert filter: supply path to external driver","fromName":"","fromEmail":"david@lang.hm","sentAt":"2010-12-20T05:52:09Z","receivedAt":"2010-12-20T05:52:09Z","isPatch":true,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Sun, 19 Dec 2010, Jeff King wrote:\n\n> On Sun, Dec 19, 2010 at 01:59:50PM -0800, Junio C Hamano wrote:\n>\n>> Modulo one worry.  Don't we have, or don't we at least plant to allow us\n>> to have, a facility to cache expensive blob conversion result, similar to\n>> the textconv caching?  How would this change interact with two blobs that\n>> live in different paths?\n>\n> Yeah, it has been talked about, but I don't think anyone is working on\n> it (I don't personally use clean/smudge at all, so it is not something I\n> have thought _that_ much about).\n>\n> This does definitely complicate matters, as the filtering is no longer a\n> pure mapping of sha1->sha1. However, I think in practice we could do\n> just fine by using a multi-level lookup. I.e., mapping a sha1 to be\n> filtered into a tree. Each tree entry would represent the remaining\n> cache parameters. In this case, the only other parameter we have is the\n> path given to the filter (but it could easily be extended to include\n> other parameters, if they existed, in this or other caching cases).\n>\n> We only get a high-performance lookup for the first part of the\n> multi-level (i.e., the sha1), but that's OK if we assume the number of\n> second-level items is going to be small. Which I think is the case here\n> (a sha1 will tend to be found only under one or a few names).\n>\n> An alternative would be to combine all parts of the filter under a\n> single lookup key. E.g., calculate and store under sha1(sha1(blob) +\n> filename)). But that means the notes keys are not actual object sha1s,\n> which throws off pruning.\n>\n> Anyway, that's just my quick thinking on the subject. I don't see any\n> reason to restrict a feature just because we might want to cache it in\n> the future. At the very worst, we could always cache filters which do\n> not use %s, and make only %s users pay the penalty.\n\nor you could cache the stats of the filtered file, including any changes \nthat are made.\n\nDavid Lang\n"},{"id":"158389","messageId":"4D0F0DFB.20502@viscovery.net","threadId":"26099","inReplyTo":"20101219212925.GA7393@arf.padd.com","subject":"Re: [PATCH] convert filter: supply path to external driver","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-12-20T08:04:11Z","receivedAt":"2010-12-20T08:04:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12/19/2010 22:29, schrieb Pete Wyckoff:\n> Filtering to support keyword expansion may need the name of\n> the file being filtered.  In particular, to support p4 keywords\n> like\n> \n>     $File: //depot/product/dir/script.sh $\n> \n> the smudge filter needs to know the name of the file it is\n> smudging.\n> \n> Add a \"%s\" conversion specifier to the gitattribute for filter.\n> It will be expanded with the path name to the file when invoking\n> the external filter command.\n\nWhat happens if there are any shell special characters in the path name\n(or spaces, for that matter). Does this shell-escape the substituted path\nname anywhere in the call chain?\n\n-- Hannes\n"},{"id":"158391","messageId":"7vy67kddx3.fsf@alter.siamese.dyndns.org","threadId":"26099","inReplyTo":"4D0F0DFB.20502@viscovery.net","subject":"Re: [PATCH] convert filter: supply path to external driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-20T08:52:24Z","receivedAt":"2010-12-20T08:52:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> What happens if there are any shell special characters in the path name\n> (or spaces, for that matter). Does this shell-escape the substituted path\n> name anywhere in the call chain?\n\nGood eyes.  Thanks.\n\nYou would need something like %'s (that is \"'\" modifier applied to 's'\nplaceholder in strbuf_expand() to cause the expansion sq'ed), and then the\ncaller must write something like\n\n\tclean = git-p4-filter --clean '%s'\n"},{"id":"158398","messageId":"20101220144159.GB30945@honk.padd.com","threadId":"26099","inReplyTo":"4D0F0DFB.20502@viscovery.net","subject":"Re: [PATCH] convert filter: supply path to external driver","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2010-12-20T14:41:59Z","receivedAt":"2010-12-20T14:41:59Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"j.sixt@viscovery.net wrote on Mon, 20 Dec 2010 09:04 +0100:\n> Am 12/19/2010 22:29, schrieb Pete Wyckoff:\n> > Filtering to support keyword expansion may need the name of\n> > the file being filtered.  In particular, to support p4 keywords\n> > like\n> > \n> >     $File: //depot/product/dir/script.sh $\n> > \n> > the smudge filter needs to know the name of the file it is\n> > smudging.\n> > \n> > Add a \"%s\" conversion specifier to the gitattribute for filter.\n> > It will be expanded with the path name to the file when invoking\n> > the external filter command.\n> \n> What happens if there are any shell special characters in the path name\n> (or spaces, for that matter). Does this shell-escape the substituted path\n> name anywhere in the call chain?\n\nGood catch---it doesn't.  I'll see if running everything through\nsq_quote_buf will help.\n\nIncidentally there appears to be no way to quote spaces in\nfilenames listed in .gitattributes, although fnmatch wildcards\ncan be used to work around that.\n\n\t\t-- Pete\n"},{"id":"158400","messageId":"20101220160911.GA32136@honk.padd.com","threadId":"26099","inReplyTo":"7vzks1e84p.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] convert filter: supply path to external driver","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2010-12-20T16:09:11Z","receivedAt":"2010-12-20T16:09:11Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Filtering to support keyword expansion may need the name of\nthe file being filtered.  In particular, to support p4 keywords\nlike\n\n    $File: //depot/product/dir/script.sh $\n\nthe smudge filter needs to know the name of the file it is\nsmudging.\n\nAdd a \"%s\" conversion specifier to the gitattribute for filter.\nIt will be expanded with the path name to the file when invoking\nthe external filter command.  The path name is quoted and\nspecial characters are escaped to prevent the shell from splitting\nincorrectly.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\ngitster@pobox.com wrote on Sun, 19 Dec 2010 13:59 -0800:\n> This is not backward compatible for people who wanted to use '%' literal\n> on their filter command line for whatever reason, so please do not\n> advertise as such.  A fair argument you could make is \"Even though this is\n> not strictly backward compatible, it is very unlikely that people passed a\n> literal % to their filter command line, and the benefit of being able to\n> give the pathname information would outweigh the downside of not being\n> compatible\", and people can agree or disagree.\n\nI overlooked that, but agree it is unlikely anyone was using % in\nfilter definitions.  Putting the path in an environment variable\nis the other option I considered.\n\n> I am personally moderately negative about $any expansion$ (I don't use it\n> myself, and I don't think sane people use it either).  As far as I can\n> tell, this should has no impact on the correctness and very little impact\n> on the performance for people who do not use $any expansion$, so I am Ok\n> with the patch.\n\nKeyword expansion is indeed nasty.  My only motivation is to\nsupport working with an upstream that relies on it.\n\nThis version of the patch handles quoting of shell\nmeta-characters, as pointed out by Hannes.  I decided to invoke\nsq_quote_buf directly on the path before expanding %s, rather\nthan writing a dict entry to understand %'s.  There is no\nrequirement for users to use single-quotes around %s in their\nconfig files, this way, either.\n\nAlso added a test case to make sure %s and quoting works as\nadvertised.\n\n\t\t-- Pete\n\n Documentation/gitattributes.txt |   12 ++++++++++\n convert.c                       |   22 +++++++++++++++++-\n t/t0021-conversion.sh           |   47 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 80 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 564586b..9ac2138 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -317,6 +317,18 @@ command is \"cat\").\n \tsmudge = cat\n ------------------------\n \n+If your filter needs the path of the file it is working on,\n+you can use the \"%s\" conversion specification.  It will be\n+replaced with the relative path to the file.  This is important\n+for keyword substitution that depends on the name of the\n+file.  Like this:\n+\n+------------------------\n+[filter \"p4\"]\n+\tclean = git-p4-filter --clean %s\n+\tsmudge = git-p4-filter --smudge %s\n+------------------------\n+\n \n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\ndiff --git a/convert.c b/convert.c\nindex e41a31e..1ef83a0 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -317,6 +317,7 @@ struct filter_params {\n \tconst char *src;\n \tunsigned long size;\n \tconst char *cmd;\n+\tconst char *path;\n };\n \n static int filter_buffer(int in, int out, void *data)\n@@ -329,7 +330,23 @@ static int filter_buffer(int in, int out, void *data)\n \tint write_err, status;\n \tconst char *argv[] = { NULL, NULL };\n \n-\targv[0] = params->cmd;\n+\t/* replace optional %s with path */\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf_expand_dict_entry dict[] = {\n+\t    \"s\", NULL,\n+\t    NULL, NULL,\n+\t};\n+\n+\t/* quote the path to preserve spaces, etc. */\n+\tsq_quote_buf(&path, params->path);\n+\tdict[0].value = path.buf;\n+\n+\t/* expand all %s with the quoted path */\n+\tstrbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);\n+\tstrbuf_release(&path);\n+\n+\targv[0] = cmd.buf;\n \n \tmemset(&child_process, 0, sizeof(child_process));\n \tchild_process.argv = argv;\n@@ -349,6 +366,8 @@ static int filter_buffer(int in, int out, void *data)\n \tstatus = finish_command(&child_process);\n \tif (status)\n \t\terror(\"external filter %s failed %d\", params->cmd, status);\n+\n+\tstrbuf_release(&cmd);\n \treturn (write_err || status);\n }\n \n@@ -376,6 +395,7 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tparams.src = src;\n \tparams.size = len;\n \tparams.cmd = cmd;\n+\tparams.path = path;\n \n \tfflush(NULL);\n \tif (start_async(&async))\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 828e35b..c5c394d 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -93,4 +93,51 @@ test_expect_success expanded_in_repo '\n \tcmp expanded-keywords expected-output\n '\n \n+cat <<EOF >argc.sh\n+#!$SHELL_PATH\n+echo argc: \\$# \"\\$@\"\n+echo argc running >&2\n+EOF\n+chmod +x argc.sh\n+\n+#\n+# The use of %s in a filter definition is expanded to the path to\n+# the filename being smudged or cleaned.  It must be shell escaped.\n+#\n+test_expect_success 'shell-escaped filenames' '\n+    norm=name-no-magic &&\n+    spec=$(echo name:sgl\\\"dbl\\ spc!bang | tr : \\\\047) &&\n+    echo some test text > test\n+    cat test > $norm &&\n+    cat test > \"$spec\" &&\n+    git add $norm &&\n+    git add \"$spec\" &&\n+    git commit -m \"add files\" &&\n+\n+    echo \"name* filter=argc\" > .gitattributes &&\n+\n+    # delete the files and check them out again, using the smudge filter\n+    git config filter.argc.smudge \"./argc.sh %s\" &&\n+    rm $norm \"$spec\" &&\n+    git checkout -- $norm \"$spec\" &&\n+\n+    # make sure argc.sh counted the right number of args\n+    echo \"argc: 1 $norm\" > res &&\n+    cmp res $norm &&\n+    echo \"argc: 1 $spec\" > res &&\n+    cmp res \"$spec\" &&\n+\n+    # %s with other args\n+    git config filter.argc.smudge \"./argc.sh %s --myword\" &&\n+    rm $norm \"$spec\" &&\n+    git checkout -- $norm \"$spec\" &&\n+\n+    # make sure argc.sh counted the right number of args\n+    echo \"argc: 2 $norm --myword\" > res &&\n+    cmp res $norm &&\n+    echo \"argc: 2 $spec --myword\" > res &&\n+    cmp res \"$spec\" &&\n+    :\n+'\n+\n test_done\n-- \n1.7.2.3\n"},{"id":"158402","messageId":"7v8vzkcol8.fsf@alter.siamese.dyndns.org","threadId":"26099","inReplyTo":"20101220160911.GA32136@honk.padd.com","subject":"Re: [PATCH v2] convert filter: supply path to external driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-20T17:59:31Z","receivedAt":"2010-12-20T17:59:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> This version of the patch handles quoting of shell\n> meta-characters, as pointed out by Hannes.  I decided to invoke\n> sq_quote_buf directly on the path before expanding %s, rather\n> than writing a dict entry to understand %'s.\n\nAhh, that's much more preferable than my horrible suggestion ;-)  If the\nuser wants to have concatenation with other parameters, that can be left\nto the shell that is invoked, e.g. \"--clean foo/%s\" for \"hello world.c\"\nwould expand to \"--clean foo/'hello world.c'\" and does what we want.\n\nAnother nitpick is if 's' is the right letter to use for the pathname\ninformation.  I think you took 's' after \"string\", but if this is to be\nextensible, you should anticipate that later there will be other kinds of\ninformation you may want to throw at the filters, and expect that some of\nthem also will be of type \"string\", and you will have painted the person\nwho wants to add that new information into a tight corner as you already\ntook the valuable 's'.\n\nWhich would suggest that you shouldn't be naming the placeholder after the\ntype but after what the placeholder means, no?  Perhaps %f (for filename)\nwould be a better choice?\n"},{"id":"158435","messageId":"20101221134403.GA10401@honk.padd.com","threadId":"26099","inReplyTo":"7v8vzkcol8.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3] convert filter: supply path to external driver","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2010-12-21T13:44:03Z","receivedAt":"2010-12-21T13:44:03Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Filtering to support keyword expansion may need the name of\nthe file being filtered.  In particular, to support p4 keywords\nlike\n\n    $File: //depot/product/dir/script.sh $\n\nthe smudge filter needs to know the name of the file it is\nsmudging.\n\nAdd a \"%f\" conversion specifier to the gitattribute for filter.\nIt will be expanded with the path name to the file when invoking\nthe external filter command.  The path name is quoted and\nspecial characters are escaped to prevent the shell from splitting\nincorrectly.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\ngitster@pobox.com wrote on Mon, 20 Dec 2010 09:59 -0800:\n> Another nitpick is if 's' is the right letter to use for the pathname\n> information.  I think you took 's' after \"string\", but if this is to be\n> extensible, you should anticipate that later there will be other kinds of\n> information you may want to throw at the filters, and expect that some of\n> them also will be of type \"string\", and you will have painted the person\n> who wants to add that new information into a tight corner as you already\n> took the valuable 's'.\n> \n> Which would suggest that you shouldn't be naming the placeholder after the\n> type but after what the placeholder means, no?  Perhaps %f (for filename)\n> would be a better choice?\n\nAgree.  The \"s\" doesn't convey much meaning.  Nothing in\npretty-formats.txt really applies either, but \"f\" makes sense.\n\n\t\t-- Pete\n\n Documentation/gitattributes.txt |   12 ++++++++++\n convert.c                       |   22 +++++++++++++++++-\n t/t0021-conversion.sh           |   47 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 80 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 564586b..1afcf01 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -317,6 +317,18 @@ command is \"cat\").\n \tsmudge = cat\n ------------------------\n \n+If your filter needs the path of the file it is working on,\n+you can use the \"%f\" conversion specification.  It will be\n+replaced with the relative path to the file.  This is important\n+for keyword substitution that depends on the name of the\n+file.  Like this:\n+\n+------------------------\n+[filter \"p4\"]\n+\tclean = git-p4-filter --clean %f\n+\tsmudge = git-p4-filter --smudge %f\n+------------------------\n+\n \n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\ndiff --git a/convert.c b/convert.c\nindex e41a31e..8f020bc 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -317,6 +317,7 @@ struct filter_params {\n \tconst char *src;\n \tunsigned long size;\n \tconst char *cmd;\n+\tconst char *path;\n };\n \n static int filter_buffer(int in, int out, void *data)\n@@ -329,7 +330,23 @@ static int filter_buffer(int in, int out, void *data)\n \tint write_err, status;\n \tconst char *argv[] = { NULL, NULL };\n \n-\targv[0] = params->cmd;\n+\t/* apply % substitution to cmd */\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf_expand_dict_entry dict[] = {\n+\t    \"f\", NULL,\n+\t    NULL, NULL,\n+\t};\n+\n+\t/* quote the path to preserve spaces, etc. */\n+\tsq_quote_buf(&path, params->path);\n+\tdict[0].value = path.buf;\n+\n+\t/* expand all %f with the quoted path */\n+\tstrbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);\n+\tstrbuf_release(&path);\n+\n+\targv[0] = cmd.buf;\n \n \tmemset(&child_process, 0, sizeof(child_process));\n \tchild_process.argv = argv;\n@@ -349,6 +366,8 @@ static int filter_buffer(int in, int out, void *data)\n \tstatus = finish_command(&child_process);\n \tif (status)\n \t\terror(\"external filter %s failed %d\", params->cmd, status);\n+\n+\tstrbuf_release(&cmd);\n \treturn (write_err || status);\n }\n \n@@ -376,6 +395,7 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tparams.src = src;\n \tparams.size = len;\n \tparams.cmd = cmd;\n+\tparams.path = path;\n \n \tfflush(NULL);\n \tif (start_async(&async))\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 828e35b..534a735 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -93,4 +93,51 @@ test_expect_success expanded_in_repo '\n \tcmp expanded-keywords expected-output\n '\n \n+cat <<EOF >argc.sh\n+#!$SHELL_PATH\n+echo argc: \\$# \"\\$@\"\n+echo argc running >&2\n+EOF\n+chmod +x argc.sh\n+\n+#\n+# The use of %f in a filter definition is expanded to the path to\n+# the filename being smudged or cleaned.  It must be shell escaped.\n+#\n+test_expect_success 'shell-escaped filenames' '\n+    norm=name-no-magic &&\n+    spec=$(echo name:sgl\\\"dbl\\ spc!bang | tr : \\\\047) &&\n+    echo some test text > test\n+    cat test > $norm &&\n+    cat test > \"$spec\" &&\n+    git add $norm &&\n+    git add \"$spec\" &&\n+    git commit -m \"add files\" &&\n+\n+    echo \"name* filter=argc\" > .gitattributes &&\n+\n+    # delete the files and check them out again, using the smudge filter\n+    git config filter.argc.smudge \"./argc.sh %f\" &&\n+    rm $norm \"$spec\" &&\n+    git checkout -- $norm \"$spec\" &&\n+\n+    # make sure argc.sh counted the right number of args\n+    echo \"argc: 1 $norm\" > res &&\n+    cmp res $norm &&\n+    echo \"argc: 1 $spec\" > res &&\n+    cmp res \"$spec\" &&\n+\n+    # %f with other args\n+    git config filter.argc.smudge \"./argc.sh %f --myword\" &&\n+    rm $norm \"$spec\" &&\n+    git checkout -- $norm \"$spec\" &&\n+\n+    # make sure argc.sh counted the right number of args\n+    echo \"argc: 2 $norm --myword\" > res &&\n+    cmp res $norm &&\n+    echo \"argc: 2 $spec --myword\" > res &&\n+    cmp res \"$spec\" &&\n+    :\n+'\n+\n test_done\n-- \n1.7.2.3\n"},{"id":"158446","messageId":"20101221181924.GB25812@burratino","threadId":"26099","inReplyTo":"20101221134403.GA10401@honk.padd.com","subject":"Re: [PATCH v3] convert filter: supply path to external driver","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-21T18:19:24Z","receivedAt":"2010-12-21T18:19:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nPete Wyckoff wrote:\n\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n\nNitpicks (some silly, some not):\n\n> @@ -93,4 +93,51 @@ test_expect_success expanded_in_repo '\n>  \tcmp expanded-keywords expected-output\n>  '\n>  \n> +cat <<EOF >argc.sh\n> +#!$SHELL_PATH\n> +echo argc: \\$# \"\\$@\"\n> +echo argc running >&2\n> +EOF\n> +chmod +x argc.sh\n\nYou can embed this in a test_expect_success stanza (like the next\none or the earlier \"setup\") like so:\n\n\tcat <<-EOF >argc.sh &&\n\t#!$SHELL_PATH\n\t...\n\tEOF\n\tchmod +x argc.sh &&\n\nThis way if the \"chmod\" fails on some platform the test would\ncatch that.\n\n> +\n> +#\n> +# The use of %f in a filter definition is expanded to the path to\n> +# the filename being smudged or cleaned.  It must be shell escaped.\n> +#\n\nI'd even prefer to see this comment inside the test_expect_success\nassertion so it can be printed when running the test with \"-v\".\nBut I suppose consistency with the other test in the script suggests\notherwise.\n\n[...]\n> +    echo some test text > test\n> +    cat test > $norm &&\n> +    cat test > \"$spec\" &&\n\nMissing && after \"> test\".  Probably best to remove the space\nafter > (just for consistency[1]).  Also, please use tabs to indent.\n\n[...]\n> +    # make sure argc.sh counted the right number of args\n> +    echo \"argc: 1 $norm\" > res &&\n> +    cmp res $norm &&\n\ntest_cmp?  (for nicer output)  See t/README.\n\n[...]\n> +    # %f with other args\n> +    git config filter.argc.smudge \"./argc.sh %f --myword\" &&\n> +    rm $norm \"$spec\" &&\n> +    git checkout -- $norm \"$spec\" &&\n> +\n> +    # make sure argc.sh counted the right number of args\n> +    echo \"argc: 2 $norm --myword\" > res &&\n> +    cmp res $norm &&\n> +    echo \"argc: 2 $spec --myword\" > res &&\n> +    cmp res \"$spec\" &&\n\nProbably would be clearer if this were a separate test assertion.\n\n> +    :\n> +'\n> +\n>  test_done\n\nThanks for the tests.  I haven't looked at the substance, alas,\nbut hope that helps nonetheless.\n\nJonathan\n\n[1] Trumped up justification for the \"no space after >\" style: if I\nalways include a space after, I would be tempted to use\n\n\tnoisy_command > /dev/null 2> &1\n\nBut that does not work because >& is recognized as a single token.\n"},{"id":"158458","messageId":"20101221203322.GA13868@honk.padd.com","threadId":"26099","inReplyTo":"20101221181924.GB25812@burratino","subject":"[PATCH v4] convert filter: supply path to external driver","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2010-12-21T20:33:22Z","receivedAt":"2010-12-21T20:33:22Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Filtering to support keyword expansion may need the name of\nthe file being filtered.  In particular, to support p4 keywords\nlike\n\n    $File: //depot/product/dir/script.sh $\n\nthe smudge filter needs to know the name of the file it is\nsmudging.\n\nAdd a \"%f\" conversion specifier to the gitattribute for filter.\nIt will be expanded with the path name to the file when invoking\nthe external filter command.  The path name is quoted and\nspecial characters are escaped to prevent the shell from splitting\nincorrectly.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\njrnieder@gmail.com wrote on Tue, 21 Dec 2010 12:19 -0600:\n> [detailed review]\n\nThanks for the nitpicks.  I put the argc.sh and chmod +x into a\nsetup test.  Tried to put some more in there, and to break up the\ntest in two, but did not want to duplicate the complex\ncalculation of \"norm\" and \"spec\" variables.  So ended up with the\nsmall setup, and just one big test still.\n\nI couldn't quite bring myself to delete the nice spaces in\nredirections like \"> test\".  Rest of the usage in t/ seems\nto be about 1/3 for space, 2/3 against.\n\nGot the test_cmp, tabs and missing &&, too, thanks for finding\nthose.\n\n\t\t-- Pete\n\n Documentation/gitattributes.txt |   12 +++++++++\n convert.c                       |   22 +++++++++++++++++-\n t/t0021-conversion.sh           |   48 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 81 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 564586b..1afcf01 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -317,6 +317,18 @@ command is \"cat\").\n \tsmudge = cat\n ------------------------\n \n+If your filter needs the path of the file it is working on,\n+you can use the \"%f\" conversion specification.  It will be\n+replaced with the relative path to the file.  This is important\n+for keyword substitution that depends on the name of the\n+file.  Like this:\n+\n+------------------------\n+[filter \"p4\"]\n+\tclean = git-p4-filter --clean %f\n+\tsmudge = git-p4-filter --smudge %f\n+------------------------\n+\n \n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\ndiff --git a/convert.c b/convert.c\nindex e41a31e..8f020bc 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -317,6 +317,7 @@ struct filter_params {\n \tconst char *src;\n \tunsigned long size;\n \tconst char *cmd;\n+\tconst char *path;\n };\n \n static int filter_buffer(int in, int out, void *data)\n@@ -329,7 +330,23 @@ static int filter_buffer(int in, int out, void *data)\n \tint write_err, status;\n \tconst char *argv[] = { NULL, NULL };\n \n-\targv[0] = params->cmd;\n+\t/* apply % substitution to cmd */\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf_expand_dict_entry dict[] = {\n+\t    \"f\", NULL,\n+\t    NULL, NULL,\n+\t};\n+\n+\t/* quote the path to preserve spaces, etc. */\n+\tsq_quote_buf(&path, params->path);\n+\tdict[0].value = path.buf;\n+\n+\t/* expand all %f with the quoted path */\n+\tstrbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);\n+\tstrbuf_release(&path);\n+\n+\targv[0] = cmd.buf;\n \n \tmemset(&child_process, 0, sizeof(child_process));\n \tchild_process.argv = argv;\n@@ -349,6 +366,8 @@ static int filter_buffer(int in, int out, void *data)\n \tstatus = finish_command(&child_process);\n \tif (status)\n \t\terror(\"external filter %s failed %d\", params->cmd, status);\n+\n+\tstrbuf_release(&cmd);\n \treturn (write_err || status);\n }\n \n@@ -376,6 +395,7 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tparams.src = src;\n \tparams.size = len;\n \tparams.cmd = cmd;\n+\tparams.path = path;\n \n \tfflush(NULL);\n \tif (start_async(&async))\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 828e35b..69c22a6 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -93,4 +93,52 @@ test_expect_success expanded_in_repo '\n \tcmp expanded-keywords expected-output\n '\n \n+test_expect_success 'filter shell-escaped filenames setup' '\n+\tcat >argc.sh <<-EOF &&\n+\t#!$SHELL_PATH\n+\techo argc: \\$# \"\\$@\"\n+\tEOF\n+\tchmod +x argc.sh\n+'\n+\n+# The use of %f in a filter definition is expanded to the path to\n+# the filename being smudged or cleaned.  It must be shell escaped.\n+# First, set up some interesting file names and pet them in\n+# .gitattributes.\n+test_expect_success 'filter shell-escaped filenames test' '\n+\tnorm=name-no-magic &&\n+\tspec=$(echo name:sgl\\\"dbl\\ spc!bang | tr : \\\\047) &&\n+\techo some test text > test &&\n+\tcat test > $norm &&\n+\tcat test > \"$spec\" &&\n+\tgit add $norm &&\n+\tgit add \"$spec\" &&\n+\tgit commit -q -m \"add files\" &&\n+\techo \"name* filter=argc\" > .gitattributes &&\n+\n+\t# delete the files and check them out again, using a smudge filter\n+\t# that will count the args and echo the command-line back to us\n+\tgit config filter.argc.smudge \"./argc.sh %f\" &&\n+\trm $norm \"$spec\" &&\n+\tgit checkout -- $norm \"$spec\" &&\n+\n+\t# make sure argc.sh counted the right number of args\n+\techo \"argc: 1 $norm\" > res &&\n+\ttest_cmp res $norm &&\n+\techo \"argc: 1 $spec\" > res &&\n+\ttest_cmp res \"$spec\" &&\n+\n+\t# do the same thing, but with more args in the filter expression\n+\tgit config filter.argc.smudge \"./argc.sh %f --myword\" &&\n+\trm $norm \"$spec\" &&\n+\tgit checkout -- $norm \"$spec\" &&\n+\n+\t# make sure argc.sh counted the right number of args\n+\techo \"argc: 2 $norm --myword\" > res &&\n+\ttest_cmp res $norm &&\n+\techo \"argc: 2 $spec --myword\" > res &&\n+\ttest_cmp res \"$spec\" &&\n+\t:\n+'\n+\n test_done\n-- \n1.7.2.3\n"},{"id":"158461","messageId":"7vzkry7rb4.fsf@alter.siamese.dyndns.org","threadId":"26099","inReplyTo":"20101221203322.GA13868@honk.padd.com","subject":"Re: [PATCH v4] convert filter: supply path to external driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-21T21:24:15Z","receivedAt":"2010-12-21T21:24:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> Filtering to support keyword expansion may need the name of\n> the file being filtered.  In particular, to support p4 keywords\n> like\n>\n>     $File: //depot/product/dir/script.sh $\n>\n> the smudge filter needs to know the name of the file it is\n> smudging.\n>\n> Add a \"%f\" conversion specifier to the gitattribute for filter.\n> It will be expanded with the path name to the file when invoking\n> the external filter command.  The path name is quoted and\n> special characters are escaped to prevent the shell from splitting\n> incorrectly.\n\nYou do not specify the filter in attributes file, and this is not just\nabout splitting incorrectly (think $var substitutions).  I rephrased the\nlast paragraph when I tentatively queued v3 (and then I had to discard it\nafter seeing this round) like this:\n\n    Allow \"%f\" in the custom filter command line specified in the\n    configuration.  This will be substituted by the filename inside a\n    single-quote pair to be passed to the shell.\n\n> I couldn't quite bring myself to delete the nice spaces in\n> redirections like \"> test\".  Rest of the usage in t/ seems\n> to be about 1/3 for space, 2/3 against.\n\nWhile existing mistakes by others do not make a good excuse to throw\nunnecessary code-churn patches at me, it is not an excuse to introduce\nmore mistakes of the same kind in new code.\n\n>  Documentation/gitattributes.txt |   12 +++++++++\n>  convert.c                       |   22 +++++++++++++++++-\n>  t/t0021-conversion.sh           |   48 +++++++++++++++++++++++++++++++++++++++\n>  3 files changed, 81 insertions(+), 1 deletions(-)\n>\n> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\n> index 564586b..1afcf01 100644\n> --- a/Documentation/gitattributes.txt\n> +++ b/Documentation/gitattributes.txt\n> @@ -317,6 +317,18 @@ command is \"cat\").\n>  \tsmudge = cat\n>  ------------------------\n>  \n> +If your filter needs the path of the file it is working on,\n> +you can use the \"%f\" conversion specification.  It will be\n> +replaced with the relative path to the file.  This is important\n> +for keyword substitution that depends on the name of the\n> +file.  Like this:\n\nMaybe \"important\" to you, but not really.  Just let the reader decide the\nimportance.  I rephrased this when I tentatively queued v3 (and then I had\nto discard it after seeing this round) like this:\n\n    Sequence \"%f\" on the filter command line is replaced with the name of\n    the file the filter is working on (a filter can use this in keyword\n    substitution, for example):\n\n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index 828e35b..69c22a6 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -93,4 +93,52 @@ test_expect_success expanded_in_repo '\n>  \tcmp expanded-keywords expected-output\n>  '\n>  \n> +test_expect_success 'filter shell-escaped filenames setup' '\n> +\tcat >argc.sh <<-EOF &&\n> +\t#!$SHELL_PATH\n> +\techo argc: \\$# \"\\$@\"\n> +\tEOF\n> +\tchmod +x argc.sh\n> +'\n>\n> +# The use of %f in a filter definition is expanded to the path to\n> +# the filename being smudged or cleaned.  It must be shell escaped.\n> +# First, set up some interesting file names and pet them in\n> +# .gitattributes.\n> +test_expect_success 'filter shell-escaped filenames test' '\n> +\tnorm=name-no-magic &&\n> +\tspec=$(echo name:sgl\\\"dbl\\ spc!bang | tr : \\\\047) &&\n\nI think this is going overboard.  The correctness of sq_quote_buf() is not\nwhat we are really testing with this test; we are primarily interested in\ntesting that '%f' is substituted here.  You can and should drop at least\ndq from the funny pathname, so that this can be run on Windows as well.\nPerhaps (note two SPs between \"name\" and \"with\"):\n\n\tspecial=\"name  with '\\''sq'\\'' and \\$x\" &&\n\nDo not over-abbreviate variable names unnecessarily; it will only confuse\nthe readers.  \"spec\" typically stands for \"specification\", but you don't\nmean that here.\n\nAlso if you define the filter as \"sh ./argc.sh %f\", you should be able to\nstop worrying about the \"chmod +x\" failing, no?\n\n> +\techo some test text > test &&\n> +\tcat test > $norm &&\n> +\tcat test > \"$spec\" &&\n\nQuote both for consistency.\n\n\tcat test >\"$norm\"\n        cat test >\"$spec\"\n\n> +\tgit add $norm &&\n> +\tgit add \"$spec\" &&\n\nWhy add them separately?\n\n> +\techo \"name* filter=argc\" > .gitattributes &&\n> +\n> +\t# delete the files and check them out again, using a smudge filter\n> +\t# that will count the args and echo the command-line back to us\n> +\tgit config filter.argc.smudge \"./argc.sh %f\" &&\n> +\trm $norm \"$spec\" &&\n> +\tgit checkout -- $norm \"$spec\" &&\n> +\n> +\t# make sure argc.sh counted the right number of args\n> +\techo \"argc: 1 $norm\" > res &&\n\nPerhaps \"res\" stands for \"result\", but both are misleading as it is\nunclear if that is an expected result or an actual result.\n\n\techo \"argc: 1 $norm\" >expect &&\n\nThanks.\n"},{"id":"158487","messageId":"20101222144013.GA22089@honk.padd.com","threadId":"26099","inReplyTo":"7vzkry7rb4.fsf@alter.siamese.dyndns.org","subject":"[PATCH v5] convert filter: supply path to external driver","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2010-12-22T14:40:13Z","receivedAt":"2010-12-22T14:40:13Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Filtering to support keyword expansion may need the name of\nthe file being filtered.  In particular, to support p4 keywords\nlike\n\n    $File: //depot/product/dir/script.sh $\n\nthe smudge filter needs to know the name of the file it is\nsmudging.\n\nAllow \"%f\" in the custom filter command line specified in the\nconfiguration.  This will be substituted by the filename\ninside a single-quote pair to be passed to the shell.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n\ngitster@pobox.com wrote on Tue, 21 Dec 2010 13:24 -0800:\n> [detailed review]\n\nChanges from v4:\n- Updated commit message, docs per Junio mods\n- Removed space after shell redirection \">\"\n- Simplified test case per Junio recommendations\n\nHopefully this is the last round of review and it is\nsafe to stage now.  Thanks,\n\n\t\t-- Pete\n\n Documentation/gitattributes.txt |   10 +++++++++\n convert.c                       |   22 +++++++++++++++++++-\n t/t0021-conversion.sh           |   42 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 73 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 564586b..8c2fdd1 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -317,6 +317,16 @@ command is \"cat\").\n \tsmudge = cat\n ------------------------\n \n+Sequence \"%f\" on the filter command line is replaced with the name of\n+the file the filter is working on.  A filter might use this in keyword\n+substitution.  For example:\n+\n+------------------------\n+[filter \"p4\"]\n+\tclean = git-p4-filter --clean %f\n+\tsmudge = git-p4-filter --smudge %f\n+------------------------\n+\n \n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\ndiff --git a/convert.c b/convert.c\nindex e41a31e..8f020bc 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -317,6 +317,7 @@ struct filter_params {\n \tconst char *src;\n \tunsigned long size;\n \tconst char *cmd;\n+\tconst char *path;\n };\n \n static int filter_buffer(int in, int out, void *data)\n@@ -329,7 +330,23 @@ static int filter_buffer(int in, int out, void *data)\n \tint write_err, status;\n \tconst char *argv[] = { NULL, NULL };\n \n-\targv[0] = params->cmd;\n+\t/* apply % substitution to cmd */\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf_expand_dict_entry dict[] = {\n+\t    \"f\", NULL,\n+\t    NULL, NULL,\n+\t};\n+\n+\t/* quote the path to preserve spaces, etc. */\n+\tsq_quote_buf(&path, params->path);\n+\tdict[0].value = path.buf;\n+\n+\t/* expand all %f with the quoted path */\n+\tstrbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);\n+\tstrbuf_release(&path);\n+\n+\targv[0] = cmd.buf;\n \n \tmemset(&child_process, 0, sizeof(child_process));\n \tchild_process.argv = argv;\n@@ -349,6 +366,8 @@ static int filter_buffer(int in, int out, void *data)\n \tstatus = finish_command(&child_process);\n \tif (status)\n \t\terror(\"external filter %s failed %d\", params->cmd, status);\n+\n+\tstrbuf_release(&cmd);\n \treturn (write_err || status);\n }\n \n@@ -376,6 +395,7 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tparams.src = src;\n \tparams.size = len;\n \tparams.cmd = cmd;\n+\tparams.path = path;\n \n \tfflush(NULL);\n \tif (start_async(&async))\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 828e35b..aacfd00 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -93,4 +93,46 @@ test_expect_success expanded_in_repo '\n \tcmp expanded-keywords expected-output\n '\n \n+# The use of %f in a filter definition is expanded to the path to\n+# the filename being smudged or cleaned.  It must be shell escaped.\n+# First, set up some interesting file names and pet them in\n+# .gitattributes.\n+test_expect_success 'filter shell-escaped filenames' '\n+\tcat >argc.sh <<-EOF &&\n+\t#!$SHELL_PATH\n+\techo argc: \\$# \"\\$@\"\n+\tEOF\n+\tnormal=name-no-magic &&\n+\tspecial=\"name  with '\\''sq'\\'' and \\$x\" &&\n+\techo some test text >\"$normal\" &&\n+\techo some test text >\"$special\" &&\n+\tgit add \"$normal\" \"$special\" &&\n+\tgit commit -q -m \"add files\" &&\n+\techo \"name* filter=argc\" >.gitattributes &&\n+\n+\t# delete the files and check them out again, using a smudge filter\n+\t# that will count the args and echo the command-line back to us\n+\tgit config filter.argc.smudge \"sh ./argc.sh %f\" &&\n+\trm \"$normal\" \"$special\" &&\n+\tgit checkout -- \"$normal\" \"$special\" &&\n+\n+\t# make sure argc.sh counted the right number of args\n+\techo \"argc: 1 $normal\" >expect &&\n+\ttest_cmp expect \"$normal\" &&\n+\techo \"argc: 1 $special\" >expect &&\n+\ttest_cmp expect \"$special\" &&\n+\n+\t# do the same thing, but with more args in the filter expression\n+\tgit config filter.argc.smudge \"sh ./argc.sh %f --my-extra-arg\" &&\n+\trm \"$normal\" \"$special\" &&\n+\tgit checkout -- \"$normal\" \"$special\" &&\n+\n+\t# make sure argc.sh counted the right number of args\n+\techo \"argc: 2 $normal --my-extra-arg\" >expect &&\n+\ttest_cmp expect \"$normal\" &&\n+\techo \"argc: 2 $special --my-extra-arg\" >expect &&\n+\ttest_cmp expect \"$special\" &&\n+\t:\n+'\n+\n test_done\n-- \n1.7.2.3\n"},{"id":"158492","messageId":"7v4oa565lc.fsf@alter.siamese.dyndns.org","threadId":"26099","inReplyTo":"20101222144013.GA22089@honk.padd.com","subject":"Re: [PATCH v5] convert filter: supply path to external driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-22T18:10:55Z","receivedAt":"2010-12-22T18:10:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Just FYI.\n\nAs I build stuff with -Werror to be on the safe side, I had to fix this up\na bit before queueing the patch.\n\n convert.c |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 0b24790..d5aebed 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"attr.h\"\n #include \"run-command.h\"\n+#include \"quote.h\"\n \n /*\n  * convert.c - convert a file when checking it out and checking it in.\n@@ -335,8 +336,8 @@ static int filter_buffer(int in, int out, void *data)\n \tstruct strbuf cmd = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf_expand_dict_entry dict[] = {\n-\t    \"f\", NULL,\n-\t    NULL, NULL,\n+\t\t{ \"f\", NULL, },\n+\t\t{ NULL, NULL, },\n \t};\n \n \t/* quote the path to preserve spaces, etc. */\n"},{"id":"158501","messageId":"7vmxnx4clg.fsf@alter.siamese.dyndns.org","threadId":"26099","inReplyTo":"7v4oa565lc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5] convert filter: supply path to external driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-22T23:22:35Z","receivedAt":"2010-12-22T23:22:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is another, just FYI.\n\n-- >8 --\nSubject: [PATCH] t0021: avoid getting filter killed with SIGPIPE\n\nThe fake filter did not read from the standard input at all,\nwhich caused the calling side to die with SIGPIPE, depending\non the timing.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t0021-conversion.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex aacfd00..9078b84 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -100,6 +100,7 @@ test_expect_success expanded_in_repo '\n test_expect_success 'filter shell-escaped filenames' '\n \tcat >argc.sh <<-EOF &&\n \t#!$SHELL_PATH\n+\tcat >/dev/null\n \techo argc: \\$# \"\\$@\"\n \tEOF\n \tnormal=name-no-magic &&\n-- \n1.7.3.4.811.g38bda\n"}]}