{"thread":{"id":"42694","subject":"[PATCH v4 0/8] extend smudge/clean filters with direct file access (for pu)","startedAt":"2016-06-22T21:09:36Z","lastAt":"2016-06-22T22:50:04Z","messageCount":12,"participants":["Joey Hess","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":8},"messages":[{"id":"289918","messageId":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":null,"subject":"[PATCH v4 0/8] extend smudge/clean filters with direct file access (for pu)","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:10Z","receivedAt":"2016-06-22T21:09:36Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"This is the same as v3, except rebased on top of tb/convert-peek-in-index\nto fix a build failure in pu.\n\nJoey Hess (8):\n  clarify %f documentation\n  add smudgeToFile and cleanFromFile filter configs\n  use cleanFromFile in git add\n  use smudgeToFile in git checkout etc\n  warn on unusable smudgeToFile/cleanFromFile config\n  better recovery from failure of smudgeToFile filter\n  use smudgeToFile filter in git am\n  use smudgeToFile filter in recursive merge\n\n Documentation/config.txt        |  18 ++++-\n Documentation/gitattributes.txt |  42 ++++++++++++\n builtin/apply.c                 |  16 +++++\n convert.c                       | 147 +++++++++++++++++++++++++++++++++++-----\n convert.h                       |  11 +++\n entry.c                         |  53 +++++++++++----\n merge-recursive.c               |  42 +++++++++---\n sha1_file.c                     |  44 ++++++++++--\n t/t0021-conversion.sh           | 115 +++++++++++++++++++++++++++++++\n 9 files changed, 441 insertions(+), 47 deletions(-)\n\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289919","messageId":"1466629758-8035-5-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 4/8] use smudgeToFile in git checkout etc","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:14Z","receivedAt":"2016-06-22T21:09:39Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"This makes git checkout, git reset, etc use smudgeToFile.\n\nIncludes test cases.\n\n(There's a call to convert_to_working_tree in merge-recursive.c\nthat could also be made to use smudgeToFile as well.)\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n entry.c               | 37 +++++++++++++++++++++++++++++--------\n t/t0021-conversion.sh | 38 +++++++++++++++++++++++++++++++++-----\n 2 files changed, 62 insertions(+), 13 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex 519e042..97975e5 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -175,8 +175,13 @@ static int write_entry(struct cache_entry *ce,\n \n \t\t/*\n \t\t * Convert from git internal format to working tree format\n+\t\t * unless the smudgeToFile filter can write to the\n+\t\t * file directly.\n \t\t */\n-\t\tif (ce_mode_s_ifmt == S_IFREG &&\n+\t\tint regular_file = ce_mode_s_ifmt == S_IFREG;\n+\t\tint smudge_to_file = regular_file\n+\t\t\t&& can_smudge_to_file(ce->name);\n+\t\tif (regular_file && ! smudge_to_file &&\n \t\t    convert_to_working_tree(ce->name, new, size, &buf)) {\n \t\t\tfree(new);\n \t\t\tnew = strbuf_detach(&buf, &newsize);\n@@ -189,13 +194,29 @@ static int write_entry(struct cache_entry *ce,\n \t\t\treturn error_errno(\"unable to create file %s\", path);\n \t\t}\n \n-\t\twrote = write_in_full(fd, new, size);\n-\t\tif (!to_tempfile)\n-\t\t\tfstat_done = fstat_output(fd, state, &st);\n-\t\tclose(fd);\n-\t\tfree(new);\n-\t\tif (wrote != size)\n-\t\t\treturn error(\"unable to write file %s\", path);\n+\t\tif (! smudge_to_file) {\n+\t\t\twrote = write_in_full(fd, new, size);\n+\t\t\tif (!to_tempfile)\n+\t\t\t\tfstat_done = fstat_output(fd, state, &st);\n+\t\t\tclose(fd);\n+\t\t\tfree(new);\n+\t\t\tif (wrote != size)\n+\t\t\t\treturn error(\"unable to write file %s\", path);\n+\t\t}\n+\t\telse {\n+\t\t\tclose(fd);\n+\t\t\tconvert_to_working_tree_filter_to_file(ce->name, path, new, size);\n+\t\t\tfree(new);\n+\t\t\t/* The smudgeToFile filter may have replaced the\n+\t\t\t * file; open it to make sure that the file\n+\t\t\t * exists. */\n+\t\t\tfd = open(path, O_RDONLY);\n+\t\t\tif (fd < 0)\n+\t\t\t\treturn error_errno(\"unable to create file %s\", path);\n+\t\t\tif (!to_tempfile)\n+\t\t\t\tfstat_done = fstat_output(fd, state, &st);\n+\t\t\tclose(fd);\n+\t\t}\n \t\tbreak;\n \tcase S_IFGITLINK:\n \t\tif (to_tempfile)\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 407d5d6..cba03fd 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -14,12 +14,20 @@ chmod +x rot13.sh\n \n cat <<EOF >rot13-from-file.sh\n #!$SHELL_PATH\n-fsfile=\"\\$1\"\n+srcfile=\"\\$1\"\n touch rot13-from-file.ran\n-cat \"\\$fsfile\" | ./rot13.sh\n+cat \"\\$srcfile\" | ./rot13.sh\n EOF\n chmod +x rot13-from-file.sh\n \n+cat <<EOF >rot13-to-file.sh\n+#!$SHELL_PATH\n+destfile=\"\\$1\"\n+touch rot13-to-file.ran\n+./rot13.sh > \"\\$destfile\"\n+EOF\n+chmod +x rot13-to-file.sh\n+\n test_expect_success setup '\n \tgit config filter.rot13.smudge ./rot13.sh &&\n \tgit config filter.rot13.clean ./rot13.sh &&\n@@ -291,6 +299,17 @@ test_expect_success 'cleanFromFile filter is used when adding a file' '\n \tcmp test fstest.t\n '\n \n+test_expect_success 'smudgeToFile filter is used when checking out a file' '\n+\ttest_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&\n+\n+\trm -f fstest.t &&\n+\tgit checkout -- fstest.t &&\n+\tcmp test fstest.t &&\n+\n+\ttest -e rot13-to-file.ran &&\n+\trm -f rot13-to-file.ran\n+'\n+\n test_expect_success 'cleanFromFile filter is not used when clean filter is not configured' '\n \ttest_config filter.noclean.smudge ./rot13.sh &&\n \ttest_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&\n@@ -299,9 +318,18 @@ test_expect_success 'cleanFromFile filter is not used when clean filter is not c\n \n \tcat test >test.no &&\n \tgit add test.no &&\n-\ttest ! -e rot13-from-file.ran &&\n-\tgit cat-file blob :test.no >actual &&\n-\tcmp test actual\n+\ttest ! -e rot13-from-file.ran\n+'\n+\n+test_expect_success 'smudgeToFile filter is not used when smudge filter is not configured' '\n+\ttest_config filter.nosmudge.clean ./rot13.sh &&\n+\ttest_config filter.nosmudge.smudgeToFile ./rot13-to-file.sh &&\n+\n+\techo \"*.no filter=nosmudge\" >.gitattributes &&\n+\n+\trm -f fstest.t &&\n+\tgit checkout -- fstest.t &&\n+\ttest ! -e rot13-to-file.ran\n '\n \n test_done\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289920","messageId":"1466629758-8035-3-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 2/8] add smudgeToFile and cleanFromFile filter configs","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:12Z","receivedAt":"2016-06-22T21:09:43Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"This adds new smudgeToFile and cleanFromFile filter commands,\nwhich are similar to smudge and clean but allow direct access to files on\ndisk.\n\nThis interface can be much more efficient when operating on large files,\nbecause the whole file content does not need to be streamed through the\nfilter. It even allows for things like cleanFromFile commands that avoid\nreading the whole content of the file, and for smudgeToFile commands that\npopulate a work tree file using an efficient Copy On Write operation.\n\nThe new filter commands will not be used for all filtering. They are\nefficient to use when git add is adding a file, or when the work tree is\nbeing updated, but not a good fit when git is internally filtering blob\nobjects in memory for eg, a diff.\n\nSo, a user who wants to use smudgeToFile should also provide a smudge\ncommand to be used in cases where smudgeToFile is not used. And ditto\nwith cleanFromFile and clean. To avoid foot-shooting configurations, the\nnew commands are not used unless the old commands are also configured.\n\nThat also ensures that a filter driver configuration that includes these\nnew commands will work, although less efficiently, when used with an older\nversion of git that does not support them.\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n Documentation/config.txt        |  18 ++++++-\n Documentation/gitattributes.txt |  37 +++++++++++++\n convert.c                       | 114 ++++++++++++++++++++++++++++++++++------\n convert.h                       |  11 ++++\n 4 files changed, 162 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 0d4095f..b76dad3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1306,15 +1306,29 @@ format.useAutoBase::\n \tformat-patch by default.\n \n filter.<driver>.clean::\n-\tThe command which is used to convert the content of a worktree\n+\tThe command which is used as a filter to convert the content of a worktree\n \tfile to a blob upon checkin.  See linkgit:gitattributes[5] for\n \tdetails.\n \n filter.<driver>.smudge::\n-\tThe command which is used to convert the content of a blob\n+\tThe command which is used as a filter to convert the content of a blob\n \tobject to a worktree file upon checkout.  See\n \tlinkgit:gitattributes[5] for details.\n \n+filter.<driver>.cleanFromFile::\n+\tSimilar to filter.<driver>.clean but the specified command\n+\tdirectly accesses a worktree file on disk, rather than\n+\treceiving the file content from standard input.\n+\tOnly used when filter.<driver>.clean is also configured.\n+\tSee linkgit:gitattributes[5] for details.\n+\n+filter.<driver>.smudgeToFile::\n+\tSimilar to filter.<driver>.smudge but the specified command\n+\twrites the content of a blob directly to a worktree file,\n+\trather than to standard output.\n+\tOnly used when filter.<driver>.smudge is also configured.\n+\tSee linkgit:gitattributes[5] for details.\n+\n fsck.<msg-id>::\n \tAllows overriding the message type (error, warn or ignore) of a\n \tspecific message ID such as `missingEmail`.\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 197ece8..a58aafc 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -385,6 +385,43 @@ not exist, or may have different contents. So, smudge and clean commands\n should not try to access the file on disk, but only act as filters on the\n content provided to them on standard input.\n \n+There are two extra commands \"cleanFromFile\" and \"smudgeToFile\", which\n+can optionally be set in a filter driver. These are similar to the \"clean\"\n+and \"smudge\" commands, but avoid needing to pipe the contents of files\n+through the filters, and instead read/write files in the filesystem.\n+This can be more efficient when using filters with large files that are not\n+directly stored in the repository.\n+\n+Both \"cleanFromFile\" and \"smudgeToFile\" are provided a path as an\n+added parameter after the configured command line.\n+\n+The \"cleanFromFile\" command is provided the path to the file that\n+it should clean. Like the \"clean\" command, it should output the cleaned\n+version to standard output.\n+\n+The \"smudgeToFile\" command is provided a path to the file that it\n+should write to. (This file will already exist, as an empty file that can\n+be written to or replaced.) Like the \"smudge\" command, \"smudgeToFile\"\n+is fed the blob object from its standard input.\n+\n+Some git operations that need to apply filters cannot use \"cleanFromFile\"\n+and \"smudgeToFile\", since the files are not present to disk. So, to avoid\n+inconsistent behavior, \"cleanFromFile\" will only be used if \"clean\" is\n+also configured, and \"smudgeToFile\" will only be used if \"smudge\" is also\n+configured.\n+\n+An example large file storage filter driver using cleanFromFile and\n+smudgeToFile follows:\n+\n+------------------------\n+[filter \"bigfiles\"]\n+\tclean = store-bigfile --from-stdin\n+\tcleanFromFile = store-bigfile --from-file\n+\tsmudge = retrieve-bigfile --to-stdout\n+\tsmudgeToFile = retrieve-bigfile --to-file\n+\trequired\n+------------------------\n+\n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n \ndiff --git a/convert.c b/convert.c\nindex f9892e4..9dde406 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -372,7 +372,8 @@ struct filter_params {\n \tunsigned long size;\n \tint fd;\n \tconst char *cmd;\n-\tconst char *path;\n+\tconst char *path; /* Path within the git repository */\n+\tconst char *fspath; /* Path to file on disk */\n };\n \n static int filter_buffer_or_fd(int in, int out, void *data)\n@@ -401,6 +402,15 @@ static int filter_buffer_or_fd(int in, int out, void *data)\n \tstrbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);\n \tstrbuf_release(&path);\n \n+\t/* append fspath to the command if it's set, separated with a space */\n+\tif (params->fspath) {\n+\t\tstruct strbuf fspath = STRBUF_INIT;\n+\t\tsq_quote_buf(&fspath, params->fspath);\n+\t\tstrbuf_addstr(&cmd, \" \");\n+\t\tstrbuf_addbuf(&cmd, &fspath);\n+\t\tstrbuf_release(&fspath);\n+\t}\n+\n \targv[0] = cmd.buf;\n \n \tchild_process.argv = argv;\n@@ -439,7 +449,8 @@ static int filter_buffer_or_fd(int in, int out, void *data)\n \treturn (write_err || status);\n }\n \n-static int apply_filter(const char *path, const char *src, size_t len, int fd,\n+static int apply_filter(const char *path, const char *fspath,\n+\t\t\tconst char *src, size_t len, int fd,\n                         struct strbuf *dst, const char *cmd)\n {\n \t/*\n@@ -468,6 +479,7 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,\n \tparams.fd = fd;\n \tparams.cmd = cmd;\n \tparams.path = path;\n+\tparams.fspath = fspath;\n \n \tfflush(NULL);\n \tif (start_async(&async))\n@@ -498,6 +510,8 @@ static struct convert_driver {\n \tstruct convert_driver *next;\n \tconst char *smudge;\n \tconst char *clean;\n+\tconst char *smudge_to_file;\n+\tconst char *clean_from_file;\n \tint required;\n } *user_convert, **user_convert_tail;\n \n@@ -524,8 +538,9 @@ static int read_convert_config(const char *var, const char *value, void *cb)\n \t}\n \n \t/*\n-\t * filter.<name>.smudge and filter.<name>.clean specifies\n-\t * the command line:\n+\t * filter.<name>.smudge, filter.<name>.clean,\n+\t * filter.<name>.smudgeToFile, filter.<name>.cleanFromFile\n+\t * specifies the command line:\n \t *\n \t *\tcommand-line\n \t *\n@@ -538,6 +553,12 @@ static int read_convert_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"clean\", key))\n \t\treturn git_config_string(&drv->clean, var, value);\n \n+\tif (!strcmp(\"smudgetofile\", key))\n+\t\treturn git_config_string(&drv->smudge_to_file, var, value);\n+\n+\tif (!strcmp(\"cleanfromfile\", key))\n+\t\treturn git_config_string(&drv->clean_from_file, var, value);\n+\n \tif (!strcmp(\"required\", key)) {\n \t\tdrv->required = git_config_bool(var, value);\n \t\treturn 0;\n@@ -835,7 +856,35 @@ int would_convert_to_git_filter_fd(const char *path)\n \tif (!ca.drv->required)\n \t\treturn 0;\n \n-\treturn apply_filter(path, NULL, 0, -1, NULL, ca.drv->clean);\n+\treturn apply_filter(path, NULL, NULL, 0, -1, NULL, ca.drv->clean);\n+}\n+\n+int can_clean_from_file(const char *path)\n+{\n+\tstruct conv_attrs ca;\n+\n+\tconvert_attrs(&ca, path);\n+\tif (!ca.drv)\n+\t\treturn 0;\n+\n+\t/* Only use the cleanFromFile filter when the clean filter is also\n+\t * configured.\n+\t */\n+\treturn (ca.drv->clean_from_file && ca.drv->clean);\n+}\n+\n+int can_smudge_to_file(const char *path)\n+{\n+\tstruct conv_attrs ca;\n+\n+\tconvert_attrs(&ca, path);\n+\tif (!ca.drv)\n+\t\treturn 0;\n+\n+\t/* Only use the smudgeToFile filter when the smudge filter is also\n+\t * configured.\n+\t */\n+\treturn (ca.drv->smudge_to_file && ca.drv->smudge);\n }\n \n const char *get_convert_attr_ascii(const char *path)\n@@ -879,7 +928,7 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \t\trequired = ca.drv->required;\n \t}\n \n-\tret |= apply_filter(path, src, len, -1, dst, filter);\n+\tret |= apply_filter(path, NULL, src, len, -1, dst, filter);\n \tif (!ret && required)\n \t\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n \n@@ -905,7 +954,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,\n \tassert(ca.drv);\n \tassert(ca.drv->clean);\n \n-\tif (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))\n+\tif (!apply_filter(path, NULL, NULL, 0, fd, dst, ca.drv->clean))\n \t\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n \n \tcrlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action,\n@@ -913,9 +962,30 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,\n \tident_to_git(path, dst->buf, dst->len, dst, ca.ident);\n }\n \n-static int convert_to_working_tree_internal(const char *path, const char *src,\n-\t\t\t\t\t    size_t len, struct strbuf *dst,\n-\t\t\t\t\t    int normalizing)\n+void convert_to_git_filter_from_file(const char *path, struct strbuf *dst,\n+\t\t\t\t   enum safe_crlf checksafe,\n+\t\t\t\t   const unsigned char *index_blob_sha1)\n+{\n+\tstruct conv_attrs ca;\n+\tconvert_attrs(&ca, path);\n+\n+\tassert(ca.drv);\n+\tassert(ca.drv->clean);\n+\tassert(ca.drv->clean_from_file);\n+\n+\tif (!apply_filter(path, path, \"\", 0, -1, dst, ca.drv->clean_from_file))\n+\t\tdie(\"%s: cleanFromFile filter '%s' failed\", path, ca.drv->name);\n+\n+\tcrlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, \n+\t\tchecksafe, index_blob_sha1);\n+\tident_to_git(path, dst->buf, dst->len, dst, ca.ident);\n+}\n+\n+static int convert_to_working_tree_internal(const char *path,\n+\t\t\t\t\t    const char *destpath,\n+\t\t\t\t\t    const char *src,\n+ \t\t\t\t\t    size_t len, struct strbuf *dst,\n+ \t\t\t\t\t    int normalizing)\n {\n \tint ret = 0, ret_filter = 0;\n \tconst char *filter = NULL;\n@@ -924,7 +994,10 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \n \tconvert_attrs(&ca, path);\n \tif (ca.drv) {\n-\t\tfilter = ca.drv->smudge;\n+\t\tif (destpath)\n+\t\t\tfilter = ca.drv->smudge_to_file;\n+\t\telse\n+\t\t\tfilter = ca.drv->smudge;\n \t\trequired = ca.drv->required;\n \t}\n \n@@ -935,7 +1008,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t}\n \t/*\n \t * CRLF conversion can be skipped if normalizing, unless there\n-\t * is a smudge filter.  The filter might expect CRLFs.\n+\t * is a filter.  The filter might expect CRLFs.\n \t */\n \tif (filter || !normalizing) {\n \t\tret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);\n@@ -945,21 +1018,30 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t}\n \t}\n \n-\tret_filter = apply_filter(path, src, len, -1, dst, filter);\n+\tret_filter = apply_filter(path, destpath, src, len, -1, dst, filter);\n \tif (!ret_filter && required)\n-\t\tdie(\"%s: smudge filter %s failed\", path, ca.drv->name);\n+\t\tdie(\"%s: %s filter %s failed\", path, destpath ? \"smudgeToFile\" : \"smudge\", ca.drv->name);\n \n \treturn ret | ret_filter;\n }\n \n int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n-\treturn convert_to_working_tree_internal(path, src, len, dst, 0);\n+\treturn convert_to_working_tree_internal(path, NULL, src, len, dst, 0);\n+}\n+\n+int convert_to_working_tree_filter_to_file(const char *path, const char *destpath, const char *src, size_t len)\n+{\n+\tstruct strbuf output = STRBUF_INIT;\n+\tint ret = convert_to_working_tree_internal(path, destpath, src, len, &output, 0);\n+\t/* The smudgeToFile filter stdout is not used. */\n+\tstrbuf_release(&output);\n+\treturn ret;\n }\n \n int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n-\tint ret = convert_to_working_tree_internal(path, src, len, dst, 1);\n+\tint ret = convert_to_working_tree_internal(path, NULL, src, len, dst, 1);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\ndiff --git a/convert.h b/convert.h\nindex 8c34dd5..56f3f9a 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -44,6 +44,10 @@ extern int convert_to_git(const char *path, const char *src, size_t len,\n \n extern int convert_to_working_tree(const char *path, const char *src,\n \t\t\t\t   size_t len, struct strbuf *dst);\n+extern int convert_to_working_tree_filter_to_file(const char *path,\n+\t\t\t\t\t\t  const char *destpath,\n+\t\t\t\t\t\t  const char *src,\n+\t\t\t\t\t\t  size_t len);\n extern int renormalize_buffer(const char *path, const char *src, size_t len,\n \t\t\t      struct strbuf *dst);\n static inline int would_convert_to_git(const char *path,\n@@ -60,6 +64,13 @@ extern void convert_to_git_filter_fd(const char *path, int fd,\n \t\t\t\t     const unsigned char *index_blob_sha1);\n \n extern int would_convert_to_git_filter_fd(const char *path);\n+/* Precondition: can_clean_from_file(path) == true */\n+extern void convert_to_git_filter_from_file(const char *path,\n+\t\t\t\t\t    struct strbuf *dst,\n+\t\t\t\t\t    enum safe_crlf checksafe,\n+\t\t\t\t\t    const unsigned char *index_blob_sha1);\n+extern int can_clean_from_file(const char *path);\n+extern int can_smudge_to_file(const char *path);\n \n /*****************************************************************\n  *\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289921","messageId":"1466629758-8035-2-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 1/8] clarify %f documentation","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:11Z","receivedAt":"2016-06-22T21:09:50Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"It's natural to expect %f to be an actual file on disk; help avoid that\nmistake.\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n Documentation/gitattributes.txt | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex f2afdb6..197ece8 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -379,6 +379,11 @@ substitution.  For example:\n \tsmudge = git-p4-filter --smudge %f\n ------------------------\n \n+Note that \"%f\" is the name of the path that is being worked on. Depending\n+on the version that is being filtered, the corresponding file on disk may\n+not exist, or may have different contents. So, smudge and clean commands\n+should not try to access the file on disk, but only act as filters on the\n+content provided to them on standard input.\n \n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289922","messageId":"1466629758-8035-4-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 3/8] use cleanFromFile in git add","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:13Z","receivedAt":"2016-06-22T21:09:51Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Includes test cases.\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n sha1_file.c           | 44 ++++++++++++++++++++++++++++++++++++++------\n t/t0021-conversion.sh | 36 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 74 insertions(+), 6 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 55604b6..df62eaf 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -3339,6 +3339,31 @@ static int index_stream_convert_blob(unsigned char *sha1, int fd,\n \treturn ret;\n }\n \n+static int index_from_file_convert_blob(unsigned char *sha1,\n+\t\t\t\t      const char *path, unsigned flags)\n+{\n+\tint ret;\n+\tconst int write_object = flags & HASH_WRITE_OBJECT;\n+\tconst int valid_sha1 = flags & HASH_USE_SHA_NOT_PATH;\n+\tstruct strbuf sbuf = STRBUF_INIT;\n+\n+\tassert(path);\n+\tassert(can_clean_from_file(path));\n+\n+\tconvert_to_git_filter_from_file(path, &sbuf,\n+\t\t\t\t write_object ? safe_crlf : SAFE_CRLF_FALSE,\n+\t\t\t\t valid_sha1 ? sha1 : NULL);\n+\n+\tif (write_object)\n+\t\tret = write_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),\n+\t\t\t\t      sha1);\n+\telse\n+\t\tret = hash_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),\n+\t\t\t\t     sha1);\n+\tstrbuf_release(&sbuf);\n+\treturn ret;\n+}\n+\n static int index_pipe(unsigned char *sha1, int fd, enum object_type type,\n \t\t      const char *path, unsigned flags)\n {\n@@ -3433,12 +3458,19 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned\n \n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFREG:\n-\t\tfd = open(path, O_RDONLY);\n-\t\tif (fd < 0)\n-\t\t\treturn error_errno(\"open(\\\"%s\\\")\", path);\n-\t\tif (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)\n-\t\t\treturn error(\"%s: failed to insert into database\",\n-\t\t\t\t     path);\n+\t\tif (can_clean_from_file(path)) {\n+\t\t\tif (index_from_file_convert_blob(sha1, path, flags) < 0)\n+\t\t\t\treturn error(\"%s: failed to insert into database\",\n+\t\t\t\t\t     path);\n+\t\t}\n+\t\telse {\n+\t\t\tfd = open(path, O_RDONLY);\n+\t\t\tif (fd < 0)\n+\t\t\t\treturn error_errno(\"open(\\\"%s\\\")\", path);\n+\t\t\tif (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)\n+\t\t\t\treturn error(\"%s: failed to insert into database\",\n+\t\t\t\t\t     path);\n+\t\t}\n \t\tbreak;\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(&sb, path, st->st_size))\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 7bac2bc..407d5d6 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -12,6 +12,14 @@ tr \\\n EOF\n chmod +x rot13.sh\n \n+cat <<EOF >rot13-from-file.sh\n+#!$SHELL_PATH\n+fsfile=\"\\$1\"\n+touch rot13-from-file.ran\n+cat \"\\$fsfile\" | ./rot13.sh\n+EOF\n+chmod +x rot13-from-file.sh\n+\n test_expect_success setup '\n \tgit config filter.rot13.smudge ./rot13.sh &&\n \tgit config filter.rot13.clean ./rot13.sh &&\n@@ -268,4 +276,32 @@ test_expect_success 'disable filter with empty override' '\n \ttest_must_be_empty err\n '\n \n+test_expect_success 'cleanFromFile filter is used when adding a file' '\n+\ttest_config filter.rot13.cleanFromFile ./rot13-from-file.sh &&\n+\n+\techo \"*.t filter=rot13\" >.gitattributes &&\n+\n+\tcat test >fstest.t &&\n+\tgit add fstest.t &&\n+\ttest -e rot13-from-file.ran &&\n+\trm -f rot13-from-file.ran &&\n+\n+\trm -f fstest.t &&\n+\tgit checkout -- fstest.t &&\n+\tcmp test fstest.t\n+'\n+\n+test_expect_success 'cleanFromFile filter is not used when clean filter is not configured' '\n+\ttest_config filter.noclean.smudge ./rot13.sh &&\n+\ttest_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&\n+\n+\techo \"*.no filter=noclean\" >.gitattributes &&\n+\n+\tcat test >test.no &&\n+\tgit add test.no &&\n+\ttest ! -e rot13-from-file.ran &&\n+\tgit cat-file blob :test.no >actual &&\n+\tcmp test actual\n+'\n+\n test_done\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289923","messageId":"1466629758-8035-6-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 5/8] warn on unusable smudgeToFile/cleanFromFile config","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:15Z","receivedAt":"2016-06-22T21:09:55Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Let the user know when they have a smudgeToFile/cleanFromFile config\nthat cannot be used because the corresponding smudge/clean config\nis missing.\n\nThe warning is only displayed a maximum of once per git invocation,\nand only when doing an operation that would use the filter.\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n convert.c | 34 ++++++++++++++++++++++++++--------\n 1 file changed, 26 insertions(+), 8 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 9dde406..378a3bd 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -859,32 +859,50 @@ int would_convert_to_git_filter_fd(const char *path)\n \treturn apply_filter(path, NULL, NULL, 0, -1, NULL, ca.drv->clean);\n }\n \n+static int can_filter_file(const char *filefilter, const char *filefiltername,\n+\t\t\t   const char *stdiofilter, const char *stdiofiltername,\n+\t\t\t   const struct conv_attrs *ca,\n+\t\t\t   int *warncount)\n+{\n+\tif (! filefilter)\n+\t\treturn 0;\n+\n+\tif (stdiofilter)\n+\t\treturn 1;\n+\n+\tif (*warncount == 0)\n+\t\twarning(\"Not running your configured filter.%s.%s command, because filter.%s.%s is not configured\",\n+\t\t\tca->drv->name, filefiltername,\n+\t\t\tca->drv->name, stdiofiltername);\n+\t\t*warncount=*warncount+1;\n+\n+\treturn 0;\n+}\n+\n int can_clean_from_file(const char *path)\n {\n \tstruct conv_attrs ca;\n+\tstatic int warncount = 0;\n \n \tconvert_attrs(&ca, path);\n \tif (!ca.drv)\n \t\treturn 0;\n \n-\t/* Only use the cleanFromFile filter when the clean filter is also\n-\t * configured.\n-\t */\n-\treturn (ca.drv->clean_from_file && ca.drv->clean);\n+\treturn can_filter_file(ca.drv->clean_from_file, \"cleanFromFile\",\n+\t\t\t       ca.drv->clean, \"clean\", &ca, &warncount);\n }\n \n int can_smudge_to_file(const char *path)\n {\n \tstruct conv_attrs ca;\n+\tstatic int warncount = 0;\n \n \tconvert_attrs(&ca, path);\n \tif (!ca.drv)\n \t\treturn 0;\n \n-\t/* Only use the smudgeToFile filter when the smudge filter is also\n-\t * configured.\n-\t */\n-\treturn (ca.drv->smudge_to_file && ca.drv->smudge);\n+\treturn can_filter_file(ca.drv->smudge_to_file, \"smudgeToFile\",\n+\t\t\t       ca.drv->smudge, \"smudge\", &ca, &warncount);\n }\n \n const char *get_convert_attr_ascii(const char *path)\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289924","messageId":"1466629758-8035-9-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 8/8] use smudgeToFile filter in recursive merge","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:18Z","receivedAt":"2016-06-22T21:09:59Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Recursive merge updates the work tree and so should use the smudgeToFile\nfilter.\n\nAt this point, smudgeToFile is run by everything that updates work\ntree files.\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n merge-recursive.c     | 42 ++++++++++++++++++++++++++++++++----------\n t/t0021-conversion.sh | 16 +++++++++++++++-\n 2 files changed, 47 insertions(+), 11 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 48fe7e7..7d38cf8 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -765,14 +765,6 @@ static void update_file_flags(struct merge_options *o,\n \t\t\tdie(_(\"cannot read object %s '%s'\"), oid_to_hex(oid), path);\n \t\tif (type != OBJ_BLOB)\n \t\t\tdie(_(\"blob expected for %s '%s'\"), oid_to_hex(oid), path);\n-\t\tif (S_ISREG(mode)) {\n-\t\t\tstruct strbuf strbuf = STRBUF_INIT;\n-\t\t\tif (convert_to_working_tree(path, buf, size, &strbuf)) {\n-\t\t\t\tfree(buf);\n-\t\t\t\tsize = strbuf.len;\n-\t\t\t\tbuf = strbuf_detach(&strbuf, NULL);\n-\t\t\t}\n-\t\t}\n \n \t\tif (make_room_for_path(o, path) < 0) {\n \t\t\tupdate_wd = 0;\n@@ -781,6 +773,7 @@ static void update_file_flags(struct merge_options *o,\n \t\t}\n \t\tif (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {\n \t\t\tint fd;\n+\t\t\tint isreg = S_ISREG(mode);\n \t\t\tif (mode & 0100)\n \t\t\t\tmode = 0777;\n \t\t\telse\n@@ -788,8 +781,37 @@ static void update_file_flags(struct merge_options *o,\n \t\t\tfd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);\n \t\t\tif (fd < 0)\n \t\t\t\tdie_errno(_(\"failed to open '%s'\"), path);\n-\t\t\twrite_in_full(fd, buf, size);\n-\t\t\tclose(fd);\n+\n+\t\t\tint smudge_to_file = can_smudge_to_file(path);\n+\t\t\tif (smudge_to_file) {\n+\t\t\t\tclose(fd);\n+\t\t\t\tfd = convert_to_working_tree_filter_to_file(path, path, buf, size);\n+\t\t\t\tif (fd < 0) {\n+\t\t\t\t\t/* smudgeToFile filter failed;\n+\t\t\t\t\t * continue with regular file\n+\t\t\t\t\t * creation. */\n+\t\t\t\t\tsmudge_to_file = 0;\n+\t\t\t\t\tfd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);\n+\t\t\t\t\tif (fd < 0)\n+\t\t\t\t\t\tdie_errno(_(\"failed to open '%s'\"), path);\n+\t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\tclose(fd);\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tif (! smudge_to_file) {\n+\t\t\t\tif (isreg) {\n+\t\t\t\t\tstruct strbuf strbuf = STRBUF_INIT;\n+\t\t\t\t\tif (convert_to_working_tree(path, buf, size, &strbuf)) {\n+\t\t\t\t\t\tfree(buf);\n+\t\t\t\t\t\tsize = strbuf.len;\n+\t\t\t\t\t\tbuf = strbuf_detach(&strbuf, NULL);\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\twrite_in_full(fd, buf, size);\n+\t\t\t\tclose(fd);\n+\t\t\t}\n \t\t} else if (S_ISLNK(mode)) {\n \t\t\tchar *lnk = xmemdupz(buf, size);\n \t\t\tsafe_create_leading_directories_const(path);\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex fd07bd6..2722013 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -334,10 +334,24 @@ test_expect_success 'recovery from failure of smudgeToFile filter that deletes t\n \tcmp test fstest.t\n '\n \n+test_expect_success 'smudgeToFile filter is used in merge' '\n+\ttest_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&\n+\n+\tgit commit -m \"added fstest.t\" fstest.t &&\n+\tgit checkout -b old &&\n+\tgit reset --hard HEAD^ &&\n+\tgit merge master &&\n+\n+\ttest -e rot13-to-file.ran &&\n+\trm -f rot13-to-file.ran &&\n+\n+\tcmp test fstest.t &&\n+\tgit checkout master\n+'\n+\n test_expect_success 'smudgeToFile filter is used by git am' '\n \ttest_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&\n \n-\tgit commit fstest.t -m \"added fstest.t\" &&\n \tgit format-patch HEAD^ --stdout > fstest.patch &&\n \tgit reset --hard HEAD^ &&\n \tgit am < fstest.patch &&\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289925","messageId":"1466629758-8035-7-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 6/8] better recovery from failure of smudgeToFile filter","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:16Z","receivedAt":"2016-06-22T21:10:05Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"If the smudgeToFile filter fails, it can leave the worktree file with the\nwrong content, or even deleted. Recover from this by falling back to\nrunning the smudge filter.\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n entry.c               | 55 ++++++++++++++++++++++++++++++++-------------------\n t/t0021-conversion.sh | 24 ++++++++++++++++++++++\n 2 files changed, 59 insertions(+), 20 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex 97975e5..8322127 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -181,12 +181,6 @@ static int write_entry(struct cache_entry *ce,\n \t\tint regular_file = ce_mode_s_ifmt == S_IFREG;\n \t\tint smudge_to_file = regular_file\n \t\t\t&& can_smudge_to_file(ce->name);\n-\t\tif (regular_file && ! smudge_to_file &&\n-\t\t    convert_to_working_tree(ce->name, new, size, &buf)) {\n-\t\t\tfree(new);\n-\t\t\tnew = strbuf_detach(&buf, &newsize);\n-\t\t\tsize = newsize;\n-\t\t}\n \n \t\tfd = open_output_fd(path, ce, to_tempfile);\n \t\tif (fd < 0) {\n@@ -194,7 +188,42 @@ static int write_entry(struct cache_entry *ce,\n \t\t\treturn error_errno(\"unable to create file %s\", path);\n \t\t}\n \n+\t\tif (smudge_to_file) {\n+\t\t\tclose(fd);\n+\t\t\tif (! convert_to_working_tree_filter_to_file(ce->name, path, new, size)) {\n+\t\t\t\tsmudge_to_file = 0;\n+\t\t\t\t/* The failing smudgeToFile filter may have\n+\t\t\t\t * deleted or replaced the file; delete\n+\t\t\t\t * the file and re-open for recovery write.\n+\t\t\t\t */\n+\t\t\t\tunlink(path);\n+\t\t\t\tfd = open_output_fd(path, ce, to_tempfile);\n+\t\t\t\tif (fd < 0) {\n+\t\t\t\t\tfree(new);\n+\t\t\t\t\treturn error_errno(\"unable to create file %s\", path);\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tfree(new);\n+\t\t\t\t/* The smudgeToFile filter may have replaced\n+\t\t\t\t * or deleted the file; reopen it to make sure\n+\t\t\t\t * that the file exists. */\n+\t\t\t\tfd = open(path, O_RDONLY);\n+\t\t\t\tif (fd < 0)\n+\t\t\t\t\treturn error_errno(\"unable to create file %s\", path);\n+\t\t\t\tif (!to_tempfile)\n+\t\t\t\t\tfstat_done = fstat_output(fd, state, &st);\n+\t\t\t\tclose(fd);\n+\t\t\t}\n+\t\t}\n+\n \t\tif (! smudge_to_file) {\n+\t\t\tif (regular_file &&\n+\t\t\t    convert_to_working_tree(ce->name, new, size, &buf)) {\n+\t\t\t\tfree(new);\n+\t\t\t\tnew = strbuf_detach(&buf, &newsize);\n+\t\t\t\tsize = newsize;\n+\t\t\t}\n \t\t\twrote = write_in_full(fd, new, size);\n \t\t\tif (!to_tempfile)\n \t\t\t\tfstat_done = fstat_output(fd, state, &st);\n@@ -203,20 +232,6 @@ static int write_entry(struct cache_entry *ce,\n \t\t\tif (wrote != size)\n \t\t\t\treturn error(\"unable to write file %s\", path);\n \t\t}\n-\t\telse {\n-\t\t\tclose(fd);\n-\t\t\tconvert_to_working_tree_filter_to_file(ce->name, path, new, size);\n-\t\t\tfree(new);\n-\t\t\t/* The smudgeToFile filter may have replaced the\n-\t\t\t * file; open it to make sure that the file\n-\t\t\t * exists. */\n-\t\t\tfd = open(path, O_RDONLY);\n-\t\t\tif (fd < 0)\n-\t\t\t\treturn error_errno(\"unable to create file %s\", path);\n-\t\t\tif (!to_tempfile)\n-\t\t\t\tfstat_done = fstat_output(fd, state, &st);\n-\t\t\tclose(fd);\n-\t\t}\n \t\tbreak;\n \tcase S_IFGITLINK:\n \t\tif (to_tempfile)\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex cba03fd..c0b4709 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -28,6 +28,14 @@ touch rot13-to-file.ran\n EOF\n chmod +x rot13-to-file.sh\n \n+cat <<EOF >delete-file-and-fail.sh\n+#!$SHELL_PATH\n+destfile=\"\\$1\"\n+rm -f \"\\$destfile\"\n+exit 1\n+EOF\n+chmod +x delete-file-and-fail.sh\n+\n test_expect_success setup '\n \tgit config filter.rot13.smudge ./rot13.sh &&\n \tgit config filter.rot13.clean ./rot13.sh &&\n@@ -310,6 +318,22 @@ test_expect_success 'smudgeToFile filter is used when checking out a file' '\n \trm -f rot13-to-file.ran\n '\n \n+test_expect_success 'recovery from failure of smudgeToFile filter, using smudge filter' '\n+\ttest_config filter.rot13.smudgeToFile false &&\n+\n+\trm -f fstest.t &&\n+\tgit checkout -- fstest.t &&\n+\tcmp test fstest.t\n+'\n+\n+test_expect_success 'recovery from failure of smudgeToFile filter that deletes the worktree file' '\n+\ttest_config filter.rot13.smudgeToFile ./delete-file-and-fail.sh &&\n+\n+\trm -f fstest.t &&\n+\tgit checkout -- fstest.t &&\n+\tcmp test fstest.t\n+'\n+\n test_expect_success 'cleanFromFile filter is not used when clean filter is not configured' '\n \ttest_config filter.noclean.smudge ./rot13.sh &&\n \ttest_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289926","messageId":"1466629758-8035-8-git-send-email-joeyh@joeyh.name","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"[PATCH v4 7/8] use smudgeToFile filter in git am","fromName":"Joey Hess","fromEmail":"joeyh@joeyh.name","sentAt":"2016-06-22T21:09:17Z","receivedAt":"2016-06-22T21:10:12Z","isPatch":true,"sender":{"key":"joeyh@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"git am updates the work tree and so should use the smudgeToFile filter.\n\nThis includes some refactoring into convert_to_working_tree_filter_to_file\nto make it check the file after running the smudgeToFile command, and clean\nup from a failing command.\n\nSigned-off-by: Joey Hess <joeyh@joeyh.name>\n---\n builtin/apply.c       | 16 ++++++++++++++++\n convert.c             | 19 +++++++++++++++++--\n entry.c               | 15 +++------------\n t/t0021-conversion.sh | 13 +++++++++++++\n 4 files changed, 49 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex b24c6ba..b027ab7 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4250,6 +4250,22 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tif (fd < 0)\n \t\treturn -1;\n \n+\tif (can_smudge_to_file(path)) {\n+\t\tclose(fd);\n+\t\tfd = convert_to_working_tree_filter_to_file(path, path, buf, size);\n+\t\tif (fd < 0) {\n+\t\t\t/* smudgeToFile filter failed; continue\n+\t\t\t * with regular file creation. */\n+\t\t\tfd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);\n+\t\t\tif (fd < 0)\n+\t\t\t\treturn -1;\n+\t\t}\n+\t\telse {\n+\t\t\tclose(fd);\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n \tif (convert_to_working_tree(path, buf, size, &nbuf)) {\n \t\tsize = nbuf.len;\n \t\tbuf  = nbuf.buf;\ndiff --git a/convert.c b/convert.c\nindex 378a3bd..4fe89e0 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1048,13 +1048,28 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc\n \treturn convert_to_working_tree_internal(path, NULL, src, len, dst, 0);\n }\n \n+/* Returns fd open to read the worktree file on success.\n+ * On failure, the worktree file will not exist. */\n int convert_to_working_tree_filter_to_file(const char *path, const char *destpath, const char *src, size_t len)\n {\n \tstruct strbuf output = STRBUF_INIT;\n-\tint ret = convert_to_working_tree_internal(path, destpath, src, len, &output, 0);\n+\tint ok = convert_to_working_tree_internal(path, destpath, src, len, &output, 0);\n \t/* The smudgeToFile filter stdout is not used. */\n \tstrbuf_release(&output);\n-\treturn ret;\n+\tif (ok) {\n+\t\t/* Open the file to make sure that it's present\n+\t\t * (and readable) after the command populated it. */\n+\t\tint fd = open(path, O_RDONLY);\n+\t\tif (fd < 0)\n+\t\t\tunlink(path);\n+\t\treturn fd;\n+\t}\n+\telse {\n+\t\t/* The command could have created the file before failing,\n+\t\t * so delete it. */\n+\t\tunlink(path);\n+\t\treturn -1;\n+\t}\n }\n \n int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst)\ndiff --git a/entry.c b/entry.c\nindex 8322127..2f7c4fd 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -190,13 +190,10 @@ static int write_entry(struct cache_entry *ce,\n \n \t\tif (smudge_to_file) {\n \t\t\tclose(fd);\n-\t\t\tif (! convert_to_working_tree_filter_to_file(ce->name, path, new, size)) {\n+\t\t\tfd = convert_to_working_tree_filter_to_file(ce->name, path, new, size);\n+\t\t\tif (fd < 0) {\n \t\t\t\tsmudge_to_file = 0;\n-\t\t\t\t/* The failing smudgeToFile filter may have\n-\t\t\t\t * deleted or replaced the file; delete\n-\t\t\t\t * the file and re-open for recovery write.\n-\t\t\t\t */\n-\t\t\t\tunlink(path);\n+\t\t\t\t/* re-open for recovery write */\n \t\t\t\tfd = open_output_fd(path, ce, to_tempfile);\n \t\t\t\tif (fd < 0) {\n \t\t\t\t\tfree(new);\n@@ -205,12 +202,6 @@ static int write_entry(struct cache_entry *ce,\n \t\t\t}\n \t\t\telse {\n \t\t\t\tfree(new);\n-\t\t\t\t/* The smudgeToFile filter may have replaced\n-\t\t\t\t * or deleted the file; reopen it to make sure\n-\t\t\t\t * that the file exists. */\n-\t\t\t\tfd = open(path, O_RDONLY);\n-\t\t\t\tif (fd < 0)\n-\t\t\t\t\treturn error_errno(\"unable to create file %s\", path);\n \t\t\t\tif (!to_tempfile)\n \t\t\t\t\tfstat_done = fstat_output(fd, state, &st);\n \t\t\t\tclose(fd);\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex c0b4709..fd07bd6 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -334,6 +334,19 @@ test_expect_success 'recovery from failure of smudgeToFile filter that deletes t\n \tcmp test fstest.t\n '\n \n+test_expect_success 'smudgeToFile filter is used by git am' '\n+\ttest_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&\n+\n+\tgit commit fstest.t -m \"added fstest.t\" &&\n+\tgit format-patch HEAD^ --stdout > fstest.patch &&\n+\tgit reset --hard HEAD^ &&\n+\tgit am < fstest.patch &&\n+\n+\ttest -e rot13-to-file.ran &&\n+\trm -f rot13-to-file.ran &&\n+\tcmp test fstest.t\n+'\n+\n test_expect_success 'cleanFromFile filter is not used when clean filter is not configured' '\n \ttest_config filter.noclean.smudge ./rot13.sh &&\n \ttest_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&\n-- \n2.9.0.587.ga3bedf2\n\n"},{"id":"289928","messageId":"xmqqeg7olxyn.fsf@gitster.mtv.corp.google.com","threadId":"42694","inReplyTo":"1466629758-8035-1-git-send-email-joeyh@joeyh.name","subject":"Re: [PATCH v4 0/8] extend smudge/clean filters with direct file access (for pu)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-22T21:23:28Z","receivedAt":"2016-06-22T21:24:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess <joeyh@joeyh.name> writes:\n\n> This is the same as v3, except rebased on top of tb/convert-peek-in-index\n> to fix a build failure in pu.\n\nThis is somewhat unfortunate, as tb/convert-peek-in-index probably\nneeds further rerolls after getting reviewed by somebody (other than\nme) and this topic will have to be rebased every time.\n\nLet's see how it goes.\n\nThanks.\n\n>\n> Joey Hess (8):\n>   clarify %f documentation\n>   add smudgeToFile and cleanFromFile filter configs\n>   use cleanFromFile in git add\n>   use smudgeToFile in git checkout etc\n>   warn on unusable smudgeToFile/cleanFromFile config\n>   better recovery from failure of smudgeToFile filter\n>   use smudgeToFile filter in git am\n>   use smudgeToFile filter in recursive merge\n>\n>  Documentation/config.txt        |  18 ++++-\n>  Documentation/gitattributes.txt |  42 ++++++++++++\n>  builtin/apply.c                 |  16 +++++\n>  convert.c                       | 147 +++++++++++++++++++++++++++++++++++-----\n>  convert.h                       |  11 +++\n>  entry.c                         |  53 +++++++++++----\n>  merge-recursive.c               |  42 +++++++++---\n>  sha1_file.c                     |  44 ++++++++++--\n>  t/t0021-conversion.sh           | 115 +++++++++++++++++++++++++++++++\n>  9 files changed, 441 insertions(+), 47 deletions(-)\n"},{"id":"289930","messageId":"xmqq7fdglx83.fsf@gitster.mtv.corp.google.com","threadId":"42694","inReplyTo":"1466629758-8035-9-git-send-email-joeyh@joeyh.name","subject":"Re: [PATCH v4 8/8] use smudgeToFile filter in recursive merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-22T21:39:24Z","receivedAt":"2016-06-22T21:39:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess <joeyh@joeyh.name> writes:\n\n> @@ -781,6 +773,7 @@ static void update_file_flags(struct merge_options *o,\n>  \t\t}\n>  \t\tif (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {\n>  \t\t\tint fd;\n> +\t\t\tint isreg = S_ISREG(mode);\n\nYou probably want to move this isreg business up one scope\n(i.e. inside \"if (update_wd) {\").  Then the if () condition\nfor this block can use it already.\n\n>  \t\t\tif (mode & 0100)\n>  \t\t\t\tmode = 0777;\n>  \t\t\telse\n> @@ -788,8 +781,37 @@ static void update_file_flags(struct merge_options *o,\n>  \t\t\tfd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);\n>  \t\t\tif (fd < 0)\n>  \t\t\t\tdie_errno(_(\"failed to open '%s'\"), path);\n> -\t\t\twrite_in_full(fd, buf, size);\n> -\t\t\tclose(fd);\n> +\n> +\t\t\tint smudge_to_file = can_smudge_to_file(path);\n\nThis does not compile with decl-after-statement.  I suspect other\npatches in this series have the same issue but I did not check.  Do\nyou say \"make DEVELOPER=1\"?\n\n> +\t\t\tif (smudge_to_file) {\n> +\t\t\t\tclose(fd);\n> +\t\t\t\tfd = convert_to_working_tree_filter_to_file(path, path, buf, size);\n> +\t\t\t\tif (fd < 0) {\n> +\t\t\t\t\t/* smudgeToFile filter failed;\n> +\t\t\t\t\t * continue with regular file\n> +\t\t\t\t\t * creation. */\n\n/*\n * Style: We format our multi-line\n * comments like this.\n */\n\n> +\t\t\t\t\tsmudge_to_file = 0;\n\nAhh, I was wondering why this is not \"if (smudge_to_file) ... else ...\".\n\n> +\t\t\t\t\tfd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);\n> +\t\t\t\t\tif (fd < 0)\n> +\t\t\t\t\t\tdie_errno(_(\"failed to open '%s'\"), path);\n> +\t\t\t\t}\n> +\t\t\t\telse {\n> +\t\t\t\t\tclose(fd);\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tif (! smudge_to_file) {\n\nStyle: if (!smudge_to_file) {\n\n> +test_expect_success 'smudgeToFile filter is used in merge' '\n> +\ttest_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&\n> +\n> +\tgit commit -m \"added fstest.t\" fstest.t &&\n> +\tgit checkout -b old &&\n> +\tgit reset --hard HEAD^ &&\n> +\tgit merge master &&\n> +\n> +\ttest -e rot13-to-file.ran &&\n> +\trm -f rot13-to-file.ran &&\n> +\n> +\tcmp test fstest.t &&\n\n\"test_cmp test fstest.t\"?  The difference matters when running the\ntest with -v option.\n\n> +\tgit checkout master\n\nWhat happens if any of the previous steps failed?  Does the next\ntest get confused because you would fail to go back to the master\nbranch?\n\n> +'\n> +\n>  test_expect_success 'smudgeToFile filter is used by git am' '\n>  \ttest_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&\n>  \n> -\tgit commit fstest.t -m \"added fstest.t\" &&\n>  \tgit format-patch HEAD^ --stdout > fstest.patch &&\n\nStyle: \n\n\tgit format-patch HEAD^ --stdout >fstest.patch &&\n\n>  \tgit reset --hard HEAD^ &&\n>  \tgit am < fstest.patch &&\n\nStyle: \n\n\tgit am <fstest.patch &&\n\nbut in this case you do not even need to redirect, i.e.\n\n\tgit am fstest.patch &&\n\nis enough.\n\nThanks.\n"},{"id":"289932","messageId":"xmqqy45wkflp.fsf@gitster.mtv.corp.google.com","threadId":"42694","inReplyTo":"xmqq7fdglx83.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 8/8] use smudgeToFile filter in recursive merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-22T22:45:22Z","receivedAt":"2016-06-22T22:50:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> +\t\t\tint smudge_to_file = can_smudge_to_file(path);\n>\n> This does not compile with decl-after-statement.  I suspect other\n> patches in this series have the same issue but I did not check.  Do\n> you say \"make DEVELOPER=1\"?\n\nAs a tentative workaround, I've queued a squashable compilation fix\nat the tip of the topic after queuing.  Please find it in 'pu' when\nit gets pushed out sometime later today.\n\nThanks.\n"}]}