{"thread":{"id":"7766","subject":"[PATCH 3/4] lockfile: record the primary process.","startedAt":"2007-04-21T10:40:55Z","lastAt":"2007-04-22T21:05:46Z","messageCount":22,"participants":["Junio C Hamano","Alex Riesen","Shawn O. Pearce","David Lang","Linus Torvalds","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"40041","messageId":"11771520591529-git-send-email-junkio@cox.net","threadId":"7766","inReplyTo":null,"subject":"[PATCH 0/4] External 'filter' attributes and drivers","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-21T10:40:55Z","receivedAt":"2007-04-21T10:40:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I know this is controversial, but here is a small four patch\nseries to let you insert arbitrary external filter in checkin\nand checkout codepath.\n\n[PATCH 1/4] Simplify calling of CR/LF conversion routines\n[PATCH 2/4] convert.c: restructure the attribute checking part.\n[PATCH 3/4] lockfile: record the primary process.\n[PATCH 4/4] Add 'filter' attribute and external filter driver definition.\n\n\n[1/4] is Alex's earlier patch, rebased on top of 'next'.\n\n[3/4] is necessary for the series because otherwise 'git add'\nwould not work with any external filter, but the change is\napplicable to 'master'.\n\n[4/4] is the body of the change.  I wanted to like run-command.h\nprocess spawning infrastructure, but I suspect I did not use it\noptimally.  People who were more involved in its evolution\nhopefully have suggestions for better use of it.\n"},{"id":"40040","messageId":"11771520593461-git-send-email-junkio@cox.net","threadId":"7766","inReplyTo":"11771520591529-git-send-email-junkio@cox.net","subject":"[PATCH 1/4] Simplify calling of CR/LF conversion routines","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-21T10:40:56Z","receivedAt":"2007-04-21T10:40:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Alex Riesen <raa.lkml@gmail.com>\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n * This is rebased on 'next'\n\n builtin-apply.c |   18 +++++--------\n cache.h         |    4 +-\n convert.c       |   71 +++++++++++++++++++++++++++----------------------------\n diff.c          |    4 +-\n entry.c         |    7 +----\n sha1_file.c     |    7 ++---\n 6 files changed, 51 insertions(+), 60 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex fd92ef7..ccd342c 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1475,8 +1475,8 @@ static int read_old_data(struct stat *st, const char *path, char **buf_p, unsign\n \t\t}\n \t\tclose(fd);\n \t\tnsize = got;\n-\t\tnbuf = buf;\n-\t\tif (convert_to_git(path, &nbuf, &nsize)) {\n+\t\tnbuf = convert_to_git(path, buf, &nsize);\n+\t\tif (nbuf) {\n \t\t\tfree(buf);\n \t\t\t*buf_p = nbuf;\n \t\t\t*alloc_p = nsize;\n@@ -2355,9 +2355,8 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \n static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)\n {\n-\tint fd, converted;\n+\tint fd;\n \tchar *nbuf;\n-\tunsigned long nsize;\n \n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n@@ -2369,13 +2368,10 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tif (fd < 0)\n \t\treturn -1;\n \n-\tnsize = size;\n-\tnbuf = (char *) buf;\n-\tconverted = convert_to_working_tree(path, &nbuf, &nsize);\n-\tif (converted) {\n+\tnbuf = convert_to_working_tree(path, buf, &size);\n+\tif (nbuf)\n \t\tbuf = nbuf;\n-\t\tsize = nsize;\n-\t}\n+\n \twhile (size) {\n \t\tint written = xwrite(fd, buf, size);\n \t\tif (written < 0)\n@@ -2387,7 +2383,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \t}\n \tif (close(fd) < 0)\n \t\tdie(\"closing file %s: %s\", path, strerror(errno));\n-\tif (converted)\n+\tif (nbuf)\n \t\tfree(nbuf);\n \treturn 0;\n }\ndiff --git a/cache.h b/cache.h\nindex 38ad006..8c804cb 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -496,8 +496,8 @@ extern void trace_printf(const char *format, ...);\n extern void trace_argv_printf(const char **argv, int count, const char *format, ...);\n \n /* convert.c */\n-extern int convert_to_git(const char *path, char **bufp, unsigned long *sizep);\n-extern int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep);\n+extern char *convert_to_git(const char *path, const char *src, unsigned long *sizep);\n+extern char *convert_to_working_tree(const char *path, const char *src, unsigned long *sizep);\n \n /* match-trees.c */\n void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);\ndiff --git a/convert.c b/convert.c\nindex da64253..742b895 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -79,25 +79,24 @@ static int is_binary(unsigned long size, struct text_stat *stats)\n \treturn 0;\n }\n \n-static int crlf_to_git(const char *path, char **bufp, unsigned long *sizep, int action)\n+static char *crlf_to_git(const char *path, const char *src, unsigned long *sizep, int action)\n {\n-\tchar *buffer, *nbuf;\n+\tchar *buffer, *dst;\n \tunsigned long size, nsize;\n \tstruct text_stat stats;\n \n \tif ((action == CRLF_BINARY) || (action == CRLF_GUESS && !auto_crlf))\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tsize = *sizep;\n \tif (!size)\n-\t\treturn 0;\n-\tbuffer = *bufp;\n+\t\treturn NULL;\n \n-\tgather_stats(buffer, size, &stats);\n+\tgather_stats(src, size, &stats);\n \n \t/* No CR? Nothing to convert, regardless. */\n \tif (!stats.cr)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tif (action == CRLF_GUESS) {\n \t\t/*\n@@ -106,13 +105,13 @@ static int crlf_to_git(const char *path, char **bufp, unsigned long *sizep, int\n \t\t * stuff?\n \t\t */\n \t\tif (stats.cr != stats.crlf)\n-\t\t\treturn 0;\n+\t\t\treturn NULL;\n \n \t\t/*\n \t\t * And add some heuristics for binary vs text, of course...\n \t\t */\n \t\tif (is_binary(size, &stats))\n-\t\t\treturn 0;\n+\t\t\treturn NULL;\n \t}\n \n \t/*\n@@ -120,10 +119,10 @@ static int crlf_to_git(const char *path, char **bufp, unsigned long *sizep, int\n \t * to let the caller know that we switched buffers on it.\n \t */\n \tnsize = size - stats.crlf;\n-\tnbuf = xmalloc(nsize);\n-\t*bufp = nbuf;\n+\tbuffer = xmalloc(nsize);\n \t*sizep = nsize;\n \n+\tdst = buffer;\n \tif (action == CRLF_GUESS) {\n \t\t/*\n \t\t * If we guessed, we already know we rejected a file with\n@@ -131,54 +130,53 @@ static int crlf_to_git(const char *path, char **bufp, unsigned long *sizep, int\n \t\t * follow it.\n \t\t */\n \t\tdo {\n-\t\t\tunsigned char c = *buffer++;\n+\t\t\tunsigned char c = *src++;\n \t\t\tif (c != '\\r')\n-\t\t\t\t*nbuf++ = c;\n+\t\t\t\t*dst++ = c;\n \t\t} while (--size);\n \t} else {\n \t\tdo {\n-\t\t\tunsigned char c = *buffer++;\n+\t\t\tunsigned char c = *src++;\n \t\t\tif (! (c == '\\r' && (1 < size && *buffer == '\\n')))\n-\t\t\t\t*nbuf++ = c;\n+\t\t\t\t*dst++ = c;\n \t\t} while (--size);\n \t}\n \n-\treturn 1;\n+\treturn buffer;\n }\n \n-static int crlf_to_worktree(const char *path, char **bufp, unsigned long *sizep, int action)\n+static char *crlf_to_worktree(const char *path, const char *src, unsigned long *sizep, int action)\n {\n-\tchar *buffer, *nbuf;\n+\tchar *buffer, *dst;\n \tunsigned long size, nsize;\n \tstruct text_stat stats;\n \tunsigned char last;\n \n \tif ((action == CRLF_BINARY) || (action == CRLF_INPUT) ||\n \t    (action == CRLF_GUESS && auto_crlf <= 0))\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tsize = *sizep;\n \tif (!size)\n-\t\treturn 0;\n-\tbuffer = *bufp;\n+\t\treturn NULL;\n \n-\tgather_stats(buffer, size, &stats);\n+\tgather_stats(src, size, &stats);\n \n \t/* No LF? Nothing to convert, regardless. */\n \tif (!stats.lf)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \t/* Was it already in CRLF format? */\n \tif (stats.lf == stats.crlf)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tif (action == CRLF_GUESS) {\n \t\t/* If we have any bare CR characters, we're not going to touch it */\n \t\tif (stats.cr != stats.crlf)\n-\t\t\treturn 0;\n+\t\t\treturn NULL;\n \n \t\tif (is_binary(size, &stats))\n-\t\t\treturn 0;\n+\t\t\treturn NULL;\n \t}\n \n \t/*\n@@ -186,19 +184,20 @@ static int crlf_to_worktree(const char *path, char **bufp, unsigned long *sizep,\n \t * to let the caller know that we switched buffers on it.\n \t */\n \tnsize = size + stats.lf - stats.crlf;\n-\tnbuf = xmalloc(nsize);\n-\t*bufp = nbuf;\n+\tbuffer = xmalloc(nsize);\n \t*sizep = nsize;\n \tlast = 0;\n+\n+\tdst = buffer;\n \tdo {\n-\t\tunsigned char c = *buffer++;\n+\t\tunsigned char c = *src++;\n \t\tif (c == '\\n' && last != '\\r')\n-\t\t\t*nbuf++ = '\\r';\n-\t\t*nbuf++ = c;\n+\t\t\t*dst++ = '\\r';\n+\t\t*dst++ = c;\n \t\tlast = c;\n \t} while (--size);\n \n-\treturn 1;\n+\treturn buffer;\n }\n \n static void setup_crlf_check(struct git_attr_check *check)\n@@ -231,12 +230,12 @@ static int git_path_check_crlf(const char *path)\n \treturn CRLF_GUESS;\n }\n \n-int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n+char *convert_to_git(const char *path, const char *src, unsigned long *sizep)\n {\n-\treturn crlf_to_git(path, bufp, sizep, git_path_check_crlf(path));\n+\treturn crlf_to_git(path, src, sizep, git_path_check_crlf(path));\n }\n \n-int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n+char *convert_to_working_tree(const char *path, const char *src, unsigned long *sizep)\n {\n-\treturn crlf_to_worktree(path, bufp, sizep, git_path_check_crlf(path));\n+\treturn crlf_to_worktree(path, src, sizep, git_path_check_crlf(path));\n }\ndiff --git a/diff.c b/diff.c\nindex 5f50186..1cb1230 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1493,9 +1493,9 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \t\t/*\n \t\t * Convert from working tree format to canonical git format\n \t\t */\n-\t\tbuf = s->data;\n \t\tsize = s->size;\n-\t\tif (convert_to_git(s->path, &buf, &size)) {\n+\t\tbuf = convert_to_git(s->path, s->data, &size);\n+\t\tif (buf) {\n \t\t\tmunmap(s->data, s->size);\n \t\t\ts->should_munmap = 0;\n \t\t\ts->data = buf;\ndiff --git a/entry.c b/entry.c\nindex d72f811..3771209 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -79,7 +79,6 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n \t}\n \tswitch (ntohl(ce->ce_mode) & S_IFMT) {\n \t\tchar *buf;\n-\t\tunsigned long nsize;\n \n \tcase S_IFREG:\n \t\tif (to_tempfile) {\n@@ -96,12 +95,10 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n \t\t/*\n \t\t * Convert from git internal format to working tree format\n \t\t */\n-\t\tbuf = new;\n-\t\tnsize = size;\n-\t\tif (convert_to_working_tree(ce->name, &buf, &nsize)) {\n+\t\tbuf = convert_to_working_tree(ce->name, new, &size);\n+\t\tif (buf) {\n \t\t\tfree(new);\n \t\t\tnew = buf;\n-\t\t\tsize = nsize;\n \t\t}\n \n \t\twrote = write_in_full(fd, new, size);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4304fe9..1978d5f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2277,10 +2277,9 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,\n \t */\n \tif ((type == OBJ_BLOB) && S_ISREG(st->st_mode)) {\n \t\tunsigned long nsize = size;\n-\t\tchar *nbuf = buf;\n-\t\tif (convert_to_git(path, &nbuf, &nsize)) {\n-\t\t\tif (size)\n-\t\t\t\tmunmap(buf, size);\n+\t\tchar *nbuf = convert_to_git(path, buf, &nsize);\n+\t\tif (nbuf) {\n+\t\t\tmunmap(buf, size);\n \t\t\tsize = nsize;\n \t\t\tbuf = nbuf;\n \t\t\tre_allocated = 1;\n-- \n1.5.1.1.948.g9f6f\n"},{"id":"40043","messageId":"1177152059853-git-send-email-junkio@cox.net","threadId":"7766","inReplyTo":"11771520591529-git-send-email-junkio@cox.net","subject":"[PATCH 2/4] convert.c: restructure the attribute checking part.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-21T10:40:57Z","receivedAt":"2007-04-21T10:40:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This separates the checkattr() call and interpretation of the\nreturned value specific to the 'crlf' attribute into separate\nroutines, so that we can run a single call to checkattr() to\ncheck for more than one attributes, and then interprete what\nthe returned settings mean separately.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n convert.c |   48 ++++++++++++++++++++++++++++--------------------\n 1 files changed, 28 insertions(+), 20 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 742b895..37239ac 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -200,7 +200,7 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *\n \treturn buffer;\n }\n \n-static void setup_crlf_check(struct git_attr_check *check)\n+static void setup_convert_check(struct git_attr_check *check)\n {\n \tstatic struct git_attr *attr_crlf;\n \n@@ -209,33 +209,41 @@ static void setup_crlf_check(struct git_attr_check *check)\n \tcheck->attr = attr_crlf;\n }\n \n-static int git_path_check_crlf(const char *path)\n+static int git_path_check_crlf(const char *path, struct git_attr_check *check)\n {\n-\tstruct git_attr_check attr_crlf_check;\n-\n-\tsetup_crlf_check(&attr_crlf_check);\n-\n-\tif (!git_checkattr(path, 1, &attr_crlf_check)) {\n-\t\tconst char *value = attr_crlf_check.value;\n-\t\tif (ATTR_TRUE(value))\n-\t\t\treturn CRLF_TEXT;\n-\t\telse if (ATTR_FALSE(value))\n-\t\t\treturn CRLF_BINARY;\n-\t\telse if (ATTR_UNSET(value))\n-\t\t\t;\n-\t\telse if (!strcmp(value, \"input\"))\n-\t\t\treturn CRLF_INPUT;\n-\t\t/* fallthru */\n-\t}\n+\tconst char *value = check->value;\n+\n+\tif (ATTR_TRUE(value))\n+\t\treturn CRLF_TEXT;\n+\telse if (ATTR_FALSE(value))\n+\t\treturn CRLF_BINARY;\n+\telse if (ATTR_UNSET(value))\n+\t\t;\n+\telse if (!strcmp(value, \"input\"))\n+\t\treturn CRLF_INPUT;\n \treturn CRLF_GUESS;\n }\n \n char *convert_to_git(const char *path, const char *src, unsigned long *sizep)\n {\n-\treturn crlf_to_git(path, src, sizep, git_path_check_crlf(path));\n+\tstruct git_attr_check check[1];\n+\tint crlf = CRLF_GUESS;\n+\n+\tsetup_convert_check(check);\n+\tif (!git_checkattr(path, 1, check)) {\n+\t\tcrlf = git_path_check_crlf(path, check);\n+\t}\n+\treturn crlf_to_git(path, src, sizep, crlf);\n }\n \n char *convert_to_working_tree(const char *path, const char *src, unsigned long *sizep)\n {\n-\treturn crlf_to_worktree(path, src, sizep, git_path_check_crlf(path));\n+\tstruct git_attr_check check[1];\n+\tint crlf = CRLF_GUESS;\n+\n+\tsetup_convert_check(check);\n+\tif (!git_checkattr(path, 1, check)) {\n+\t\tcrlf = git_path_check_crlf(path, check);\n+\t}\n+\treturn crlf_to_worktree(path, src, sizep, crlf);\n }\n-- \n1.5.1.1.948.g9f6f\n"},{"id":"40039","messageId":"1177152059809-git-send-email-junkio@cox.net","threadId":"7766","inReplyTo":"11771520591529-git-send-email-junkio@cox.net","subject":"[PATCH 3/4] lockfile: record the primary process.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-21T10:40:58Z","receivedAt":"2007-04-21T10:40:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The usual process flow is the main process opens and holds the lock to\nthe index, does its thing, perhaps spawning children during the course,\nand then writes the resulting index out by releaseing the lock.\n\nHowever, the lockfile interface uses atexit(3) to clean it up, without\nregard to who actually created the lock.  This typically leads to a\nconfusing behaviour of lock being released too early when the child\nexits, and then the parent process when it calls commit_lockfile()\nfinds that it cannot unlock it.\n\nThis fixes the problem by recording who created and holds the lock, and\nupon atexit(3) handler, child simply ignores the lockfile the parent\ncreated.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n cache.h    |    1 +\n lockfile.c |    6 +++++-\n 2 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 8c804cb..faddaf6 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -209,6 +209,7 @@ extern int refresh_cache(unsigned int flags);\n \n struct lock_file {\n \tstruct lock_file *next;\n+\tpid_t owner;\n \tchar on_list;\n \tchar filename[PATH_MAX];\n };\ndiff --git a/lockfile.c b/lockfile.c\nindex bed6b21..23db35a 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -8,8 +8,11 @@ static const char *alternate_index_output;\n \n static void remove_lock_file(void)\n {\n+\tpid_t me = getpid();\n+\n \twhile (lock_file_list) {\n-\t\tif (lock_file_list->filename[0])\n+\t\tif (lock_file_list->owner == me &&\n+\t\t    lock_file_list->filename[0])\n \t\t\tunlink(lock_file_list->filename);\n \t\tlock_file_list = lock_file_list->next;\n \t}\n@@ -28,6 +31,7 @@ static int lock_file(struct lock_file *lk, const char *path)\n \tsprintf(lk->filename, \"%s.lock\", path);\n \tfd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);\n \tif (0 <= fd) {\n+\t\tlk->owner = getpid();\n \t\tif (!lk->on_list) {\n \t\t\tlk->next = lock_file_list;\n \t\t\tlock_file_list = lk;\n-- \n1.5.1.1.948.g9f6f\n"},{"id":"40042","messageId":"11771520591703-git-send-email-junkio@cox.net","threadId":"7766","inReplyTo":"11771520591529-git-send-email-junkio@cox.net","subject":"[PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-21T10:40:59Z","receivedAt":"2007-04-21T10:40:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The interface is similar to the custom low-level merge drivers.\n\nFirst you configure your filter driver by defining 'filter.<name>.*'\nvariables in the configuration.\n\n\tfilter.<name>.clean\tfilter command to run upon checkin\n\tfilter.<name>.smudge\tfilter command to run upon checkout\n\nThen you assign filter attribute to each path, whose name\nmatches the custom filter driver's name.\n\nExample:\n\n\t(in .gitattributes)\n\t*.c\tfilter=indent\n\n\t(in config)\n\t[filter \"indent\"]\n\t\tclean = indent\n\t\tsmudge = cat\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n convert.c         |  253 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n t/t0021-filter.sh |   35 ++++++++\n 2 files changed, 279 insertions(+), 9 deletions(-)\n create mode 100755 t/t0021-filter.sh\n\ndiff --git a/convert.c b/convert.c\nindex 37239ac..62d8cee 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"attr.h\"\n+#include \"run-command.h\"\n \n /*\n  * convert.c - convert a file when checking it out and checking it in.\n@@ -200,18 +201,208 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *\n \treturn buffer;\n }\n \n+static int filter_buffer(const char *path, const char *src,\n+\t\t\t unsigned long size, const char *cmd)\n+{\n+\t/*\n+\t * Spawn cmd and feed the buffer contents through its stdin.\n+\t */\n+\tstruct child_process child_process;\n+\tint pipe_feed[2];\n+\tint write_err, status;\n+\n+\tmemset(&child_process, 0, sizeof(child_process));\n+\n+\tif (pipe(pipe_feed) < 0) {\n+\t\terror(\"cannot create pipe to run external filter %s\", cmd);\n+\t\treturn 1;\n+\t}\n+\n+\tchild_process.pid = fork();\n+\tif (child_process.pid < 0) {\n+\t\terror(\"cannot fork to run external filter %s\", cmd);\n+\t\tclose(pipe_feed[0]);\n+\t\tclose(pipe_feed[1]);\n+\t\treturn 1;\n+\t}\n+\tif (!child_process.pid) {\n+\t\tdup2(pipe_feed[0], 0);\n+\t\tclose(pipe_feed[0]);\n+\t\tclose(pipe_feed[1]);\n+\t\texeclp(cmd, cmd, NULL);\n+\t\treturn 1;\n+\t}\n+\tclose(pipe_feed[0]);\n+\n+\twrite_err = (write_in_full(pipe_feed[1], src, size) < 0);\n+\tif (close(pipe_feed[1]))\n+\t\twrite_err = 1;\n+\tif (write_err)\n+\t\terror(\"cannot feed the input to external filter %s\", cmd);\n+\n+\tstatus = finish_command(&child_process);\n+\tif (status)\n+\t\terror(\"external filter %s failed %d\", cmd, -status);\n+\treturn (write_err || status);\n+}\n+\n+static char *apply_filter(const char *path, const char *src,\n+\t\t\t  unsigned long *sizep, const char *cmd)\n+{\n+\t/*\n+\t * Create a pipeline to have the command filter the buffer's\n+\t * contents.\n+\t *\n+\t * (child --> cmd) --> us\n+\t */\n+\tconst int SLOP = 4096;\n+\tint pipe_feed[2];\n+\tint status;\n+\tchar *dst;\n+\tunsigned long dstsize, dstalloc;\n+\tstruct child_process child_process;\n+\n+\tif (!cmd)\n+\t\treturn NULL;\n+\n+\tmemset(&child_process, 0, sizeof(child_process));\n+\n+\tif (pipe(pipe_feed) < 0) {\n+\t\terror(\"cannot create pipe to run external filter %s\", cmd);\n+\t\treturn NULL;\n+\t}\n+\n+\tchild_process.pid = fork();\n+\tif (child_process.pid < 0) {\n+\t\terror(\"cannot fork to run external filter %s\", cmd);\n+\t\tclose(pipe_feed[0]);\n+\t\tclose(pipe_feed[1]);\n+\t\treturn NULL;\n+\t}\n+\tif (!child_process.pid) {\n+\t\tdup2(pipe_feed[1], 1);\n+\t\tclose(pipe_feed[0]);\n+\t\tclose(pipe_feed[1]);\n+\t\texit(filter_buffer(path, src, *sizep, cmd));\n+\t}\n+\tclose(pipe_feed[1]);\n+\n+\tdstalloc = *sizep;\n+\tdst = xmalloc(dstalloc);\n+\tdstsize = 0;\n+\n+\twhile (1) {\n+\t\tssize_t numread = xread(pipe_feed[0], dst + dstsize,\n+\t\t\t\t\tdstalloc - dstsize);\n+\n+\t\tif (numread <= 0) {\n+\t\t\tif (!numread)\n+\t\t\t\tbreak;\n+\t\t\terror(\"read from external filter %s failed\", cmd);\n+\t\t\tfree(dst);\n+\t\t\tdst = NULL;\n+\t\t\tbreak;\n+\t\t}\n+\t\tdstsize += numread;\n+\t\tif (dstalloc <= dstsize + SLOP) {\n+\t\t\tdstalloc = dstsize + SLOP;\n+\t\t\tdst = xrealloc(dst, dstalloc);\n+\t\t}\n+\t}\n+\n+\tstatus = finish_command(&child_process);\n+\tif (status) {\n+\t\terror(\"external filter %s failed %d\", cmd, -status);\n+\t\tfree(dst);\n+\t\tdst = NULL;\n+\t}\n+\n+\tif (dst)\n+\t\t*sizep = dstsize;\n+\treturn dst;\n+}\n+\n+static struct convert_driver {\n+\tconst char *name;\n+\tstruct convert_driver *next;\n+\tchar *smudge;\n+\tchar *clean;\n+} *user_convert, **user_convert_tail;\n+\n+static int read_convert_config(const char *var, const char *value)\n+{\n+\tconst char *ep, *name;\n+\tint namelen;\n+\tstruct convert_driver *drv;\n+\n+\t/*\n+\t * External conversion drivers are configured using\n+\t * \"filter.<name>.variable\".\n+\t */\n+\tif (prefixcmp(var, \"filter.\") || (ep = strrchr(var, '.')) == var + 6)\n+\t\treturn 0;\n+\tname = var + 7;\n+\tnamelen = ep - name;\n+\tfor (drv = user_convert; drv; drv = drv->next)\n+\t\tif (!strncmp(drv->name, name, namelen) && !drv->name[namelen])\n+\t\t\tbreak;\n+\tif (!drv) {\n+\t\tchar *namebuf;\n+\t\tdrv = xcalloc(1, sizeof(struct convert_driver));\n+\t\tnamebuf = xmalloc(namelen + 1);\n+\t\tmemcpy(namebuf, name, namelen);\n+\t\tnamebuf[namelen] = 0;\n+\t\tdrv->name = namebuf;\n+\t\tdrv->next = NULL;\n+\t\t*user_convert_tail = drv;\n+\t\tuser_convert_tail = &(drv->next);\n+\t}\n+\n+\tep++;\n+\n+\t/*\n+\t * filter.<name>.smudge and filter.<name>.clean specifies\n+\t * the command line:\n+\t *\n+\t *\tcommand-line\n+\t *\n+\t * The command-line will not be interpolated in any way.\n+\t */\n+\n+\tif (!strcmp(\"smudge\", ep)) {\n+\t\tif (!value)\n+\t\t\treturn error(\"%s: lacks value\", var);\n+\t\tdrv->smudge = strdup(value);\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(\"clean\", ep)) {\n+\t\tif (!value)\n+\t\t\treturn error(\"%s: lacks value\", var);\n+\t\tdrv->clean = strdup(value);\n+\t\treturn 0;\n+\t}\n+\treturn 0;\n+}\n+\n static void setup_convert_check(struct git_attr_check *check)\n {\n \tstatic struct git_attr *attr_crlf;\n+\tstatic struct git_attr *attr_filter;\n \n-\tif (!attr_crlf)\n+\tif (!attr_crlf) {\n \t\tattr_crlf = git_attr(\"crlf\", 4);\n-\tcheck->attr = attr_crlf;\n+\t\tattr_filter = git_attr(\"filter\", 6);\n+\t\tuser_convert_tail = &user_convert;\n+\t\tgit_config(read_convert_config);\n+\t}\n+\tcheck[0].attr = attr_crlf;\n+\tcheck[1].attr = attr_filter;\n }\n \n static int git_path_check_crlf(const char *path, struct git_attr_check *check)\n {\n-\tconst char *value = check->value;\n+\tconst char *value = check[0].value;\n \n \tif (ATTR_TRUE(value))\n \t\treturn CRLF_TEXT;\n@@ -224,26 +415,70 @@ static int git_path_check_crlf(const char *path, struct git_attr_check *check)\n \treturn CRLF_GUESS;\n }\n \n+static struct convert_driver *git_path_check_convert(const char *path,\n+\t\t\t\t\t     struct git_attr_check *check)\n+{\n+\tconst char *value = check[1].value;\n+\tstruct convert_driver *drv;\n+\n+\tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))\n+\t\treturn NULL;\n+\tfor (drv = user_convert; drv; drv = drv->next)\n+\t\tif (!strcmp(value, drv->name))\n+\t\t\treturn drv;\n+\treturn NULL;\n+}\n+\n char *convert_to_git(const char *path, const char *src, unsigned long *sizep)\n {\n-\tstruct git_attr_check check[1];\n+\tstruct git_attr_check check[2];\n \tint crlf = CRLF_GUESS;\n+\tchar *filter = NULL;\n+\tchar *buf, *buf2;\n \n \tsetup_convert_check(check);\n-\tif (!git_checkattr(path, 1, check)) {\n+\tif (!git_checkattr(path, 2, check)) {\n+\t\tstruct convert_driver *drv;\n \t\tcrlf = git_path_check_crlf(path, check);\n+\t\tdrv = git_path_check_convert(path, check);\n+\t\tif (drv && drv->clean)\n+\t\t\tfilter = drv->clean;\n \t}\n-\treturn crlf_to_git(path, src, sizep, crlf);\n+\n+\tbuf = apply_filter(path, src, sizep, filter);\n+\n+\tbuf2 = crlf_to_git(path, buf ? buf : src, sizep, crlf);\n+\tif (buf2) {\n+\t\tfree(buf);\n+\t\tbuf = buf2;\n+\t}\n+\n+\treturn buf;\n }\n \n char *convert_to_working_tree(const char *path, const char *src, unsigned long *sizep)\n {\n-\tstruct git_attr_check check[1];\n+\tstruct git_attr_check check[2];\n \tint crlf = CRLF_GUESS;\n+\tchar *filter = NULL;\n+\tchar *buf, *buf2;\n \n \tsetup_convert_check(check);\n-\tif (!git_checkattr(path, 1, check)) {\n+\tif (!git_checkattr(path, 2, check)) {\n+\t\tstruct convert_driver *drv;\n \t\tcrlf = git_path_check_crlf(path, check);\n+\t\tdrv = git_path_check_convert(path, check);\n+\t\tif (drv && drv->smudge)\n+\t\t\tfilter = drv->smudge;\n \t}\n-\treturn crlf_to_worktree(path, src, sizep, crlf);\n+\n+\tbuf = crlf_to_worktree(path, src, sizep, crlf);\n+\n+\tbuf2 = apply_filter(path, buf ? buf : src, sizep, filter);\n+\tif (buf2) {\n+\t\tfree(buf);\n+\t\tbuf = buf2;\n+\t}\n+\n+\treturn buf;\n }\ndiff --git a/t/t0021-filter.sh b/t/t0021-filter.sh\nnew file mode 100755\nindex 0000000..0f4cd05\n--- /dev/null\n+++ b/t/t0021-filter.sh\n@@ -0,0 +1,35 @@\n+#!/bin/sh\n+\n+test_description='external filter conversion'\n+\n+. ./test-lib.sh\n+\n+cat <<\\EOF >rot13.sh\n+tr '[a-zA-Z]' '[n-za-mN-ZA-M]'\n+EOF\n+chmod +x rot13.sh\n+\n+test_expect_success setup '\n+\tgit config filter.rot13.smudge ./rot13.sh &&\n+\tgit config filter.rot13.clean ./rot13.sh &&\n+\n+\techo \"*.t filter=rot13\" >.gitattributes &&\n+\n+\t{\n+\t    echo a b c d e f g h i j k l m\n+\t    echo n o p q r s t u v w x y z\n+\t} >test &&\n+\tcat test >test.t &&\n+\tcat test >test.o &&\n+\tgit add test test.t &&\n+\tgit checkout -- test test.t\n+'\n+\n+test_expect_success check '\n+\n+\tcmp test.o test &&\n+\tcmp test.o test.t\n+\n+'\n+\n+test_done\n-- \n1.5.1.1.948.g9f6f\n"},{"id":"40070","messageId":"20070421200340.GB2437@steel.home","threadId":"7766","inReplyTo":"11771520591529-git-send-email-junkio@cox.net","subject":"Re: [PATCH 0/4] External 'filter' attributes and drivers","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-21T20:03:40Z","receivedAt":"2007-04-21T20:03:40Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Sat, Apr 21, 2007 12:40:55 +0200:\n> \n> [1/4] is Alex's earlier patch, rebased on top of 'next'.\n> \n\nOh, thanks. I was almost about to resend it.\n"},{"id":"40077","messageId":"20070422003929.GD17480@spearce.org","threadId":"7766","inReplyTo":"11771520591703-git-send-email-junkio@cox.net","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-04-22T00:39:29Z","receivedAt":"2007-04-22T00:39:29Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> The interface is similar to the custom low-level merge drivers.\n... \n> +static int filter_buffer(const char *path, const char *src,\n> +\t\t\t unsigned long size, const char *cmd)\n> +{\n> +\t/*\n> +\t * Spawn cmd and feed the buffer contents through its stdin.\n> +\t */\n...\n\nick.  What about something like this on top?  I moved the extra child\nprocess for the input pipe down into the start_command routine,\nwhere we can do something a little smarter on some systems, like\nusing a thread rather than a full process.  Its also a shorter\npatch and uses more of the run-command API.\n\nIts not documented very well, but if you set child_process.{in,out}\nto <0 we open the pipe for you, and close your side of the pipe\nfor you.  That really simplifies the logic in the callers.\n\nI did consider rewriting this as a select() based loop to feed the\ninput and read the output, but that might not port well onto a more\nnative Win32 based set of APIs.\n\n---\ndiff --git a/convert.c b/convert.c\nindex 62d8cee..2ba7ea3 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -201,99 +201,37 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *\n \treturn buffer;\n }\n \n-static int filter_buffer(const char *path, const char *src,\n-\t\t\t unsigned long size, const char *cmd)\n-{\n-\t/*\n-\t * Spawn cmd and feed the buffer contents through its stdin.\n-\t */\n-\tstruct child_process child_process;\n-\tint pipe_feed[2];\n-\tint write_err, status;\n-\n-\tmemset(&child_process, 0, sizeof(child_process));\n-\n-\tif (pipe(pipe_feed) < 0) {\n-\t\terror(\"cannot create pipe to run external filter %s\", cmd);\n-\t\treturn 1;\n-\t}\n-\n-\tchild_process.pid = fork();\n-\tif (child_process.pid < 0) {\n-\t\terror(\"cannot fork to run external filter %s\", cmd);\n-\t\tclose(pipe_feed[0]);\n-\t\tclose(pipe_feed[1]);\n-\t\treturn 1;\n-\t}\n-\tif (!child_process.pid) {\n-\t\tdup2(pipe_feed[0], 0);\n-\t\tclose(pipe_feed[0]);\n-\t\tclose(pipe_feed[1]);\n-\t\texeclp(cmd, cmd, NULL);\n-\t\treturn 1;\n-\t}\n-\tclose(pipe_feed[0]);\n-\n-\twrite_err = (write_in_full(pipe_feed[1], src, size) < 0);\n-\tif (close(pipe_feed[1]))\n-\t\twrite_err = 1;\n-\tif (write_err)\n-\t\terror(\"cannot feed the input to external filter %s\", cmd);\n-\n-\tstatus = finish_command(&child_process);\n-\tif (status)\n-\t\terror(\"external filter %s failed %d\", cmd, -status);\n-\treturn (write_err || status);\n-}\n-\n static char *apply_filter(const char *path, const char *src,\n \t\t\t  unsigned long *sizep, const char *cmd)\n {\n-\t/*\n-\t * Create a pipeline to have the command filter the buffer's\n-\t * contents.\n-\t *\n-\t * (child --> cmd) --> us\n-\t */\n \tconst int SLOP = 4096;\n-\tint pipe_feed[2];\n+\tstruct child_process filter_process;\n+\tconst char *filter_argv[] = {cmd, NULL};\n \tint status;\n \tchar *dst;\n \tunsigned long dstsize, dstalloc;\n-\tstruct child_process child_process;\n \n \tif (!cmd)\n \t\treturn NULL;\n \n-\tmemset(&child_process, 0, sizeof(child_process));\n-\n-\tif (pipe(pipe_feed) < 0) {\n-\t\terror(\"cannot create pipe to run external filter %s\", cmd);\n-\t\treturn NULL;\n-\t}\n-\n-\tchild_process.pid = fork();\n-\tif (child_process.pid < 0) {\n-\t\terror(\"cannot fork to run external filter %s\", cmd);\n-\t\tclose(pipe_feed[0]);\n-\t\tclose(pipe_feed[1]);\n+\tmemset(&filter_process, 0, sizeof(filter_process));\n+\tfilter_process.in_bufptr = src;\n+\tfilter_process.in_buflen = *sizep;\n+\tfilter_process.out = -1;\n+\tfilter_process.argv = filter_argv;\n+\tstatus = start_command(&filter_process);\n+\tif (status) {\n+\t\terror(\"external filter %s failed %d\", cmd, -status);\n \t\treturn NULL;\n \t}\n-\tif (!child_process.pid) {\n-\t\tdup2(pipe_feed[1], 1);\n-\t\tclose(pipe_feed[0]);\n-\t\tclose(pipe_feed[1]);\n-\t\texit(filter_buffer(path, src, *sizep, cmd));\n-\t}\n-\tclose(pipe_feed[1]);\n \n \tdstalloc = *sizep;\n \tdst = xmalloc(dstalloc);\n \tdstsize = 0;\n \n \twhile (1) {\n-\t\tssize_t numread = xread(pipe_feed[0], dst + dstsize,\n-\t\t\t\t\tdstalloc - dstsize);\n+\t\tssize_t numread = xread(filter_process.out,\n+\t\t\tdst + dstsize, dstalloc - dstsize);\n \n \t\tif (numread <= 0) {\n \t\t\tif (!numread)\n@@ -310,7 +248,7 @@ static char *apply_filter(const char *path, const char *src,\n \t\t}\n \t}\n \n-\tstatus = finish_command(&child_process);\n+\tstatus = finish_command(&filter_process);\n \tif (status) {\n \t\terror(\"external filter %s failed %d\", cmd, -status);\n \t\tfree(dst);\ndiff --git a/run-command.c b/run-command.c\nindex eff523e..72887f8 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -20,12 +20,29 @@ int start_command(struct child_process *cmd)\n \tint need_in, need_out;\n \tint fdin[2], fdout[2];\n \n-\tneed_in = !cmd->no_stdin && cmd->in < 0;\n+\tneed_in = !cmd->no_stdin && (cmd->in_bufptr || cmd->in < 0);\n \tif (need_in) {\n \t\tif (pipe(fdin) < 0)\n \t\t\treturn -ERR_RUN_COMMAND_PIPE;\n-\t\tcmd->in = fdin[1];\n-\t\tcmd->close_in = 1;\n+\t\tif (cmd->in_bufptr) {\n+\t\t\tpid_t in_feeder = fork();\n+\t\t\tif (in_feeder < 0) {\n+\t\t\t\tclose_pair(fdin);\n+\t\t\t\treturn -ERR_RUN_COMMAND_PIPE;\n+\t\t\t}\n+\t\t\tif (!in_feeder) {\n+\t\t\t\tclose(fdin[0]);\n+\t\t\t\tif (write_in_full(fdin[1],\n+\t\t\t\t\tcmd->in_bufptr,\n+\t\t\t\t\tcmd->in_buflen) != cmd->in_buflen)\n+\t\t\t\t\tdie(\"'%s' did not read all input\", cmd->argv[0]);\n+\t\t\t\texit(0);\n+\t\t\t}\n+\t\t\tclose(fdin[1]);\n+\t\t} else {\n+\t\t\tcmd->in = fdin[1];\n+\t\t\tcmd->close_in = 1;\n+\t\t}\n \t}\n \n \tneed_out = !cmd->no_stdout\ndiff --git a/run-command.h b/run-command.h\nindex 3680ef9..7632843 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -13,6 +13,8 @@ enum {\n \n struct child_process {\n \tconst char **argv;\n+\tconst char *in_bufptr;\n+\tsize_t in_buflen;\n \tpid_t pid;\n \tint in;\n \tint out;\n-- \n1.5.1.1.135.gf948\n\n\n-- \nShawn.\n"},{"id":"40084","messageId":"Pine.LNX.4.63.0704211757210.5655@qynat.qvtvafvgr.pbz","threadId":"7766","inReplyTo":"11771520591529-git-send-email-junkio@cox.net","subject":"Re: [PATCH 0/4] External 'filter' attributes and drivers","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-22T01:19:05Z","receivedAt":"2007-04-22T01:19:05Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Sat, 21 Apr 2007, Junio C Hamano wrote:\n\n> I know this is controversial, but here is a small four patch\n> series to let you insert arbitrary external filter in checkin\n> and checkout codepath.\n>\n> [PATCH 1/4] Simplify calling of CR/LF conversion routines\n> [PATCH 2/4] convert.c: restructure the attribute checking part.\n> [PATCH 3/4] lockfile: record the primary process.\n> [PATCH 4/4] Add 'filter' attribute and external filter driver definition.\n>\n>\n> [1/4] is Alex's earlier patch, rebased on top of 'next'.\n>\n> [3/4] is necessary for the series because otherwise 'git add'\n> would not work with any external filter, but the change is\n> applicable to 'master'.\n>\n> [4/4] is the body of the change.  I wanted to like run-command.h\n> process spawning infrastructure, but I suspect I did not use it\n> optimally.  People who were more involved in its evolution\n> hopefully have suggestions for better use of it.\n\nthanks for doing this.\n\nfrom other discussions on the subject I was under the impression that in some \ncases git commands would look at the file in the working directory instead of \nextracing it from the object blob if the index and timestamp indicated that the \nfile had not been changed.\n\nwith an external filter in use this is not a safe thing to do and that needs to \nbe disabled if gitattributes specifies an external filter (and possibly if \ngitattributes specifies crlf)\n\nthis brings up a potential performance problem. if you have to look for \n.gitattributes in all parent directories for each file manipulation tha may want \nto use the working tree optimization, you can end up with a lot fo time wasted \nin just looking up non-existant files (or in finding files and parseing them). \nApache has this problem with their .htaccess files wher eyou can get a pretty \nsubstantial performance boost by disabling them. it may be worth it to be able \nto not use .gitattribute files, but only use the repository-wide gitattributes \nfile.\n\nFinally, a how-to question\n\nin my case I have two uses for this\n\n1. a permission preserving filter.\n\n  This records the permissions on checking, sets them on checkout\n\n2. host specific file munging for config files\n\n   I have a XML file that specifies what munging should be done for each host and \na perl script that does this munging\n\nin both of these cases I have a config for the filter (the list of permissions \nor the xml file defining the munging) that should be checked into the repository \nas part of the commit\n\nthe problems I see are\n\n1. the config file needs to be checked out before any files that go through the \nfilter so that the filter has it available to use\n\n2. when checking in files there is a potential of having the config file change \nbetween one addition to the index and another. Part of this is in the \"well, \ndon't do that then catagory\" of things, but for things like the permission \nfilter this is harder to do\n\nif there is one master permission file then each file checkin could potentially \nadd/modify a line in it. as long as lines related to other files aren't changed \nyou could re-add it to the index, overriding the prior version.\n\nif you do one permission file per file you are tracking the permissions of (say \n.permissions.$file) then you don't have to worry about different versions of the \nfile, but you still need to have the filter (which is running as part of a \nchecking) do a checkin of another file. This approach would also make it harder \nto know which files need to be checked out before the filter can run.\n\nany thoughts on how to deal with these issues?\n\nDavid Lang\n"},{"id":"40085","messageId":"Pine.LNX.4.63.0704211821560.5655@qynat.qvtvafvgr.pbz","threadId":"7766","inReplyTo":"11771520591703-git-send-email-junkio@cox.net","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-22T01:33:13Z","receivedAt":"2007-04-22T01:33:13Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Sat, 21 Apr 2007, Junio C Hamano wrote:\n\n> The interface is similar to the custom low-level merge drivers.\n>\n> First you configure your filter driver by defining 'filter.<name>.*'\n> variables in the configuration.\n>\n> \tfilter.<name>.clean\tfilter command to run upon checkin\n> \tfilter.<name>.smudge\tfilter command to run upon checkout\n>\n> Then you assign filter attribute to each path, whose name\n> matches the custom filter driver's name.\n>\n> Example:\n>\n> \t(in .gitattributes)\n> \t*.c\tfilter=indent\n>\n> \t(in config)\n> \t[filter \"indent\"]\n> \t\tclean = indent\n> \t\tsmudge = cat\n\n\nhmm, three things come to mind here\n\n1. it would be useful in many cases for the filter program to know what file \nit's working on (and probably some other things), so there are probably some \ncommand-line arguments that should be able to be passed to the filter.\n\n2. should this be done as a modification of the in-memory buffer (s this patch \ndoes it?) or should it be done at the time of the read/write, makeing the filter \nbe responsible for actually doing the disk I/O, which would give it the benifit \nof being able to do things like set permissions and other things that can't be \ndone until the file is actually on the filesystem (for something managing config \nfiles, this could include restarting the daemon related to the config file for \nexample)\n\n3. why specify seperate clean/smudge programs instead of just one script with a \nread/write parameter? I suspect that in most cases the external filter program \nthat cleans files will be the same one that smudges them. the clean/smudge \nversion does let you specify vastly different things without requireing a \nwrapper script around them, but it would mean duplicating the line when they are \nthe same.\n\nthe first two items seem fairly important to me, but the third is a niceity that \nI could live with as-is.\n\nDavid Lang\n"},{"id":"40086","messageId":"7vbqhhrw7m.fsf@assigned-by-dhcp.cox.net","threadId":"7766","inReplyTo":"20070422003929.GD17480@spearce.org","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T02:15:09Z","receivedAt":"2007-04-22T02:15:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> ick.  What about something like this on top?  I moved the extra child\n> process for the input pipe down into the start_command routine,\n> where we can do something a little smarter on some systems, like\n> using a thread rather than a full process.  Its also a shorter\n> patch and uses more of the run-command API.\n\nWell, I did not like start_command() that wanted to always\nperform the full exec of something else for its inflexibility,\nand this piles a specific hack on top of it...  Why not a\ncallback with void * pointer?\n\nOr are you trying to make this interface as inflexible and\nfeature-limited as possible, perhaps to make it easier to\nporting to Windows?\n"},{"id":"40088","messageId":"20070422030038.GF17480@spearce.org","threadId":"7766","inReplyTo":"7vbqhhrw7m.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-04-22T03:00:38Z","receivedAt":"2007-04-22T03:00:38Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> Well, I did not like start_command() that wanted to always\n> perform the full exec of something else for its inflexibility,\n> and this piles a specific hack on top of it...  Why not a\n> callback with void * pointer?\n\nWell, that's because its always used to execute some external\nprogram.  And some operating system designers once upon a time\nthought that was the only way anyone would ever need to start a\nnew parallel thread of execution.  ;-)\n\nBut why do you want a callback here in start_command() given\nthat all you are doing is running a filter command anyway?\nIs that so you could start a \"thread\" to handle the stdin\npipe?\n\n-- \nShawn.\n"},{"id":"40090","messageId":"20070422052008.GH17480@spearce.org","threadId":"7766","inReplyTo":"11771520591529-git-send-email-junkio@cox.net","subject":"Re: [PATCH 0/4] External 'filter' attributes and drivers","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-04-22T05:20:08Z","receivedAt":"2007-04-22T05:20:08Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> I know this is controversial, but here is a small four patch\n> series to let you insert arbitrary external filter in checkin\n> and checkout codepath.\n\nThis series is some pretty nice work.\n\nBut I really don't think we want filters.  Actually, I'm very\nagainst them, and I'm actually very against the CRLF work that\nhas already been added.  Since the CRLF ship has sailed I won't\ntry to call it back to port.  But I don't want to see the filter\nstuff raise anchor...\n\nI've only really seen a few arguments for the filters:\n\n1) Better compress structured content (e.g. ODF) by storing the\n   ZIP as a tree, allowing normal deltification within packfiles\n   to apply to the contained files.\n\n2) Use a custom diff function on special files (e.g. ODF) as they\n   are otherwise unreadable with the internal xdiff based engine.\n\n3) Mutate content prior to extracting from the tree, e.g. printing.\n\n\nLet me try to address these points.\n\n#1: There are a limited number of content formats that we could\nreasonably filter into the repository such that the standard\ndeltification routines will have good space/performance benefits.\nMost of them today are ZIP archives (e.g. ODF, JAR).\n\nWhy don't we just teach the packfile format how to better compress\nthese types of streams?  Let read_sha1_file() and pack-objects do all\nof the heavy translation work, just as they do today for text files.\nExplode them into a \"tree-like\" thing that allows deltification\nagainst any other content (even cross ZIP streams) just like we\ndo with trees, but always expose them to the working directory\nlevel of the system as blobs.\n\nThis way we never get into the mess that David Lang pointed out\nwhere we have many optimizations that reuse working tree files when\nstat data matches; nor do we have to worry about major structural\ndifferences between the working tree (1 file) and the repository\nformat (exploded ZIP as 10,000 files).\n\n\n#2: We already support using any diff tool you want: set the\nGIT_EXTERNAL_DIFF environment variable before running a program that\ngenerates a diff.  As Junio pointed out on #git tonight, that could\nbe any shell script that decides how to produce the diff based on\nits own logic.  Though we could also use the new attribute stuff\nto select diff programs, much like we do now for merge conflict\nresolution in merge-recursive.\n\n\n#3: This has already been discussed at length on the list.\nLetting the build system perform this sort of work is better than\nmaking the VCS do it; especially when you want the VCS to do its\nsole job well (track the state of the working directory) and the\nbuild system do its sole job well (produce files suitable for use\noutside of the repository).\n\n\nSo despite the fact that I tried to make 4/4 shorter, I really\ndon't think we should be doing this...\n\n-- \nShawn.\n"},{"id":"40091","messageId":"alpine.LFD.0.98.0704212243080.9964@woody.linux-foundation.org","threadId":"7766","inReplyTo":"11771520591703-git-send-email-junkio@cox.net","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-22T05:47:24Z","receivedAt":"2007-04-22T05:47:24Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 21 Apr 2007, Junio C Hamano wrote:\n>\n> The interface is similar to the custom low-level merge drivers.\n> \n> First you configure your filter driver by defining 'filter.<name>.*'\n> variables in the configuration.\n> \n> \tfilter.<name>.clean\tfilter command to run upon checkin\n> \tfilter.<name>.smudge\tfilter command to run upon checkout\n\nI have to say, I'm obviously not a huge fan of playing games, but the \ndiffs are very clean.\n\nAre they actually *useful?* I dunno. I'm a bit nervous about what this \nmeans for any actual user of the feature, but I have to admit to being \ncharmed by a clean implementation. \n\nI suspect that this gets some complaining off our back, but I *also* \nsuspect that people will actually end up really screwing themselves with \nsomething like this and then blaming us and causing a huge pain down the \nline when we've supported this and people want \"extended semantics\" that \nare no longer clean.\n\nBut I'm not sure how valid an argument that really is. I do happen to \nbelieve in the \"give them rope\" philosophy. I think you can probably screw \nyourself royally with this, but hey, anybody who does that only has \nhimself to blame ...\n\n\t\tLinus\n"},{"id":"40095","messageId":"7v7is5rl7v.fsf@assigned-by-dhcp.cox.net","threadId":"7766","inReplyTo":"alpine.LFD.0.98.0704212243080.9964@woody.linux-foundation.org","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T06:12:36Z","receivedAt":"2007-04-22T06:12:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> I suspect that this gets some complaining off our back, but I *also* \n> suspect that people will actually end up really screwing themselves with \n> something like this and then blaming us and causing a huge pain down the \n> line when we've supported this and people want \"extended semantics\" that \n> are no longer clean.\n>\n> But I'm not sure how valid an argument that really is. I do happen to \n> believe in the \"give them rope\" philosophy. I think you can probably screw \n> yourself royally with this, but hey, anybody who does that only has \n> himself to blame ...\n\nThat's exactly my argument when I had an exchange with Nico on\nthis thread.  I haven't decided if I want to make them (I sent\nout a replacement patch for the tip one, and added another one)\nmerged in 'next' yet for that exact reason.\n"},{"id":"40099","messageId":"7v4pn8rk8t.fsf@assigned-by-dhcp.cox.net","threadId":"7766","inReplyTo":"Pine.LNX.4.63.0704211821560.5655@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T06:33:38Z","receivedAt":"2007-04-22T06:33:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Lang <david.lang@digitalinsight.com> writes:\n\n> 1. it would be useful in many cases for the filter program to know\n> what file it's working on (and probably some other things), so there\n> are probably some command-line arguments that should be able to be\n> passed to the filter.\n\nI can see that you missed the class when Linus talked about how\nmessy things would get once you allow the conversion to be\nstateful.  I was in the class and remembered it ;-)\n\nAlthough I initially considered interpolating \"%P\" with\npathname, I ended up deciding against it, to discourage people\nfrom abusing the filter for stateful conversion that changes the\nresults depending on time, pathname, commit, branch and stuff.\n\n> 2. should this be done as a modification of the in-memory buffer (s\n> this patch does it?) or should it be done at the time of the\n> read/write, makeing the filter be responsible for actually doing the\n> disk I/O, which would give it the benifit of being able to do things\n> like set permissions and other things ...\n\nThe conversion is not about overriding the mode bits recorded in\ntree objects, nor making git as a replacement for build procedure.\n\n> 3. why specify seperate clean/smudge programs instead of just one\n> script with a read/write parameter?\n\nI think the most common two ways have clean as a cleaner and\nsmudge as a no-op (similar to crlf=input conversion), or clean\nand smudge are inverse operations (similar to crlf=true\nconversion.  I do not see a sane case where clean and smudge are\nthe same, unless you are thinking about the toy demonstration\ntest piece I added to t0021 which uses rot13 as both clean and\nsmudge filters.\n"},{"id":"40107","messageId":"Pine.LNX.4.63.0704220155230.5946@qynat.qvtvafvgr.pbz","threadId":"7766","inReplyTo":"20070422052008.GH17480@spearce.org","subject":"Re: [PATCH 0/4] External 'filter' attributes and drivers","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-22T09:01:55Z","receivedAt":"2007-04-22T09:01:55Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Sun, 22 Apr 2007, Shawn O. Pearce wrote:\n\n> Junio C Hamano <junkio@cox.net> wrote:\n>> I know this is controversial, but here is a small four patch\n>> series to let you insert arbitrary external filter in checkin\n>> and checkout codepath.\n>\n> This series is some pretty nice work.\n>\n> But I really don't think we want filters.  Actually, I'm very\n> against them, and I'm actually very against the CRLF work that\n> has already been added.  Since the CRLF ship has sailed I won't\n> try to call it back to port.  But I don't want to see the filter\n> stuff raise anchor...\n>\n> I've only really seen a few arguments for the filters:\n>\n> 1) Better compress structured content (e.g. ODF) by storing the\n>   ZIP as a tree, allowing normal deltification within packfiles\n>   to apply to the contained files.\n>\n> 2) Use a custom diff function on special files (e.g. ODF) as they\n>   are otherwise unreadable with the internal xdiff based engine.\n>\n> 3) Mutate content prior to extracting from the tree, e.g. printing.\n>\n>\n> Let me try to address these points.\n>\n> #1: There are a limited number of content formats that we could\n> reasonably filter into the repository such that the standard\n> deltification routines will have good space/performance benefits.\n> Most of them today are ZIP archives (e.g. ODF, JAR).\n\nI think that there are a lot more that could benifit from application specific \nknowledge. git today just deals with line-based formats. anything that has more \nstructure to it could benifit from a diff/merge/delta/etc specific to it\n\n> Why don't we just teach the packfile format how to better compress\n> these types of streams?  Let read_sha1_file() and pack-objects do all\n> of the heavy translation work, just as they do today for text files.\n> Explode them into a \"tree-like\" thing that allows deltification\n> against any other content (even cross ZIP streams) just like we\n> do with trees, but always expose them to the working directory\n> level of the system as blobs.\n>\n> This way we never get into the mess that David Lang pointed out\n> where we have many optimizations that reuse working tree files when\n> stat data matches; nor do we have to worry about major structural\n> differences between the working tree (1 file) and the repository\n> format (exploded ZIP as 10,000 files).\n>\n\nnobody is suggesting that the working tree format be one file and the repository \nformat be 1000 files. but if you think that skipping borrowing from a working \ntree is expensive, consider how expsndive it is to uncompress and recompress \nfiles.\n\n>\n> #2: We already support using any diff tool you want: set the\n> GIT_EXTERNAL_DIFF environment variable before running a program that\n> generates a diff.  As Junio pointed out on #git tonight, that could\n> be any shell script that decides how to produce the diff based on\n> its own logic.  Though we could also use the new attribute stuff\n> to select diff programs, much like we do now for merge conflict\n> resolution in merge-recursive.\n\nbut not diff tools per file, only per repository. not all files in a repository \nshould be handled the same way.\n\n>\n> #3: This has already been discussed at length on the list.\n> Letting the build system perform this sort of work is better than\n> making the VCS do it; especially when you want the VCS to do its\n> sole job well (track the state of the working directory) and the\n> build system do its sole job well (produce files suitable for use\n> outside of the repository).\n\nsometimes there isn't a build system to shove this work off to.\n\nI'll add a few more\n\nmanaging permissions on files\n\nmanaging system config files (where you want many systems to share what's \nlogicly the same config, but each system needs it's own specifics)\n\nDavid Lang\n"},{"id":"40108","messageId":"Pine.LNX.4.63.0704220202550.5946@qynat.qvtvafvgr.pbz","threadId":"7766","inReplyTo":"7v4pn8rk8t.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-22T09:09:54Z","receivedAt":"2007-04-22T09:09:54Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Sat, 21 Apr 2007, Junio C Hamano wrote:\n\n> David Lang <david.lang@digitalinsight.com> writes:\n>\n>> 1. it would be useful in many cases for the filter program to know\n>> what file it's working on (and probably some other things), so there\n>> are probably some command-line arguments that should be able to be\n>> passed to the filter.\n>\n> I can see that you missed the class when Linus talked about how\n> messy things would get once you allow the conversion to be\n> stateful.  I was in the class and remembered it ;-)\n>\n> Although I initially considered interpolating \"%P\" with\n> pathname, I ended up deciding against it, to discourage people\n> from abusing the filter for stateful conversion that changes the\n> results depending on time, pathname, commit, branch and stuff.\n\nI didn't miss it, I just don't think that the path in the repository is \nnessasarily as dangerous as the other things (time, branch, etc)\n\none thing that was listed as a possibilty was to use the sha1 of the file, but \nyou would force the filter to calculate that itself. it's already available when \nextracting and recalcuating it is a waste\n\n>> 2. should this be done as a modification of the in-memory buffer (s\n>> this patch does it?) or should it be done at the time of the\n>> read/write, makeing the filter be responsible for actually doing the\n>> disk I/O, which would give it the benifit of being able to do things\n>> like set permissions and other things ...\n>\n> The conversion is not about overriding the mode bits recorded in\n> tree objects, nor making git as a replacement for build procedure.\n\nwhat build procedures?\n\nI'm talking about doing things like managing files in /etc\n\ngit doesn't have all the hooks to be able to set the permissions when you \nextract a file, but if the filters were actual readers/writers instead of \nin-memory operators, this becomes trivial to implement with no further changes \nto git itself\n\n>> 3. why specify seperate clean/smudge programs instead of just one\n>> script with a read/write parameter?\n>\n> I think the most common two ways have clean as a cleaner and\n> smudge as a no-op (similar to crlf=input conversion), or clean\n> and smudge are inverse operations (similar to crlf=true\n> conversion.  I do not see a sane case where clean and smudge are\n> the same, unless you are thinking about the toy demonstration\n> test piece I added to t0021 which uses rot13 as both clean and\n> smudge filters.\n\nactually, I'm thinking of much more complicated filters, where it's easier to \nhave one program do both functions then it is to have two seperate programs \n(like tar -c /tar -x)\n\nDavid Lang\n"},{"id":"40109","messageId":"Pine.LNX.4.63.0704220215540.5946@qynat.qvtvafvgr.pbz","threadId":"7766","inReplyTo":"Pine.LNX.4.63.0704220202550.5946@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-22T09:20:18Z","receivedAt":"2007-04-22T09:20:18Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Sun, 22 Apr 2007, David Lang wrote:\n\n> On Sat, 21 Apr 2007, Junio C Hamano wrote:\n>\n>> David Lang <david.lang@digitalinsight.com> writes:\n>> \n>>> 1. it would be useful in many cases for the filter program to know\n>>> what file it's working on (and probably some other things), so there\n>>> are probably some command-line arguments that should be able to be\n>>> passed to the filter.\n>> \n>> I can see that you missed the class when Linus talked about how\n>> messy things would get once you allow the conversion to be\n>> stateful.  I was in the class and remembered it ;-)\n>> \n>> Although I initially considered interpolating \"%P\" with\n>> pathname, I ended up deciding against it, to discourage people\n>> from abusing the filter for stateful conversion that changes the\n>> results depending on time, pathname, commit, branch and stuff.\n>\n> I didn't miss it, I just don't think that the path in the repository is \n> nessasarily as dangerous as the other things (time, branch, etc)\n\nto clarify a bit more. I have a perl program that I can point at the \n'interesting' files on my systems and have it create a 'generic' version of that \nfile. I can then take that generic version of the file to any machine in the \ncluster and with the same program create a version of that generic file that's \ncorrect for the other system. however to know which substatutions are \nappropriate to do for the file, it needs to know the filename (well, I guess I \ncould create a whole bunch of seperate config files, and then define all the \nfiles with different filters, each filter including the config file to use fo \nrthat specific file, but this seems like a really ugly way to do it)\n\n> one thing that was listed as a possibilty was to use the sha1 of the file, \n> but you would force the filter to calculate that itself. it's already \n> available when extracting and recalcuating it is a waste\n\nignore this comment, I see you posted an example of this.\n\nDavid Lang\n"},{"id":"40130","messageId":"7v4pn8paqf.fsf@assigned-by-dhcp.cox.net","threadId":"7766","inReplyTo":"Pine.LNX.4.63.0704220202550.5946@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T17:42:00Z","receivedAt":"2007-04-22T17:42:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Lang <david.lang@digitalinsight.com> writes:\n\n>> The conversion is not about overriding the mode bits recorded in\n>> tree objects, nor making git as a replacement for build procedure.\n>\n> what build procedures?\n>\n> I'm talking about doing things like managing files in /etc\n\nThat's exactly what \"build procedure\" is about.  Nobody sane\nshould be managing /etc files *directly* under any SCM.  A saner\npractice is to have a separate source area with Makefile to\nregenerate /etc files, so that (1) \"make\" manages the target\nhost specific customizations, (2) \"make diff\" can be used to\ncompare and sanity check the result of \"make\" against what is\ncurrently installed under /etc, and (3) \"make install\" manages\nthe permission bits.  The same applies to dotfiles under $HOME/.\n"},{"id":"40136","messageId":"alpine.LFD.0.98.0704221409390.28339@xanadu.home","threadId":"7766","inReplyTo":"Pine.LNX.4.63.0704220202550.5946@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-22T18:11:53Z","receivedAt":"2007-04-22T18:11:53Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 22 Apr 2007, David Lang wrote:\n\n> On Sat, 21 Apr 2007, Junio C Hamano wrote:\n> \n> > > 3. why specify seperate clean/smudge programs instead of just one\n> > > script with a read/write parameter?\n> > \n> > I think the most common two ways have clean as a cleaner and\n> > smudge as a no-op (similar to crlf=input conversion), or clean\n> > and smudge are inverse operations (similar to crlf=true\n> > conversion.  I do not see a sane case where clean and smudge are\n> > the same, unless you are thinking about the toy demonstration\n> > test piece I added to t0021 which uses rot13 as both clean and\n> > smudge filters.\n> \n> actually, I'm thinking of much more complicated filters, where it's easier to\n> have one program do both functions then it is to have two seperate programs\n> (like tar -c /tar -x)\n\nJust specify the same program in both entries with the appropriate \nparameter and be happy.\n\nIt is much easier to have two entries with the same program than having \nonly one entry when you actually have two separate programs.\n\n\nNicolas\n"},{"id":"40152","messageId":"Pine.LNX.4.63.0704221327190.6340@qynat.qvtvafvgr.pbz","threadId":"7766","inReplyTo":"alpine.LFD.0.98.0704221409390.28339@xanadu.home","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driverdefinition.","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-22T20:27:49Z","receivedAt":"2007-04-22T20:27:49Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Sun, 22 Apr 2007, Nicolas Pitre wrote:\n\n> On Sun, 22 Apr 2007, David Lang wrote:\n>\n>> On Sat, 21 Apr 2007, Junio C Hamano wrote:\n>>\n>>>> 3. why specify seperate clean/smudge programs instead of just one\n>>>> script with a read/write parameter?\n>>>\n>>> I think the most common two ways have clean as a cleaner and\n>>> smudge as a no-op (similar to crlf=input conversion), or clean\n>>> and smudge are inverse operations (similar to crlf=true\n>>> conversion.  I do not see a sane case where clean and smudge are\n>>> the same, unless you are thinking about the toy demonstration\n>>> test piece I added to t0021 which uses rot13 as both clean and\n>>> smudge filters.\n>>\n>> actually, I'm thinking of much more complicated filters, where it's easier to\n>> have one program do both functions then it is to have two seperate programs\n>> (like tar -c /tar -x)\n>\n> Just specify the same program in both entries with the appropriate\n> parameter and be happy.\n>\n> It is much easier to have two entries with the same program than having\n> only one entry when you actually have two separate programs.\n\nagreed, this is why I said that this was a fairly minor thing.\n\nDavid Lang\n"},{"id":"40159","messageId":"Pine.LNX.4.63.0704221328300.6340@qynat.qvtvafvgr.pbz","threadId":"7766","inReplyTo":"7v4pn8paqf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-22T21:05:46Z","receivedAt":"2007-04-22T21:05:46Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Sun, 22 Apr 2007, Junio C Hamano wrote:\n\n> David Lang <david.lang@digitalinsight.com> writes:\n>\n>>> The conversion is not about overriding the mode bits recorded in\n>>> tree objects, nor making git as a replacement for build procedure.\n>>\n>> what build procedures?\n>>\n>> I'm talking about doing things like managing files in /etc\n>\n> That's exactly what \"build procedure\" is about.  Nobody sane\n> should be managing /etc files *directly* under any SCM.  A saner\n> practice is to have a separate source area with Makefile to\n> regenerate /etc files, so that (1) \"make\" manages the target\n> host specific customizations, (2) \"make diff\" can be used to\n> compare and sanity check the result of \"make\" against what is\n> currently installed under /etc, and (3) \"make install\" manages\n> the permission bits.  The same applies to dotfiles under $HOME/.\n\nthis I disagree with this, simply becouse it discourages people from useing a \nSCM for these things due to all the other work that needs to be done\n\ndoing it this way you need to\ncreate a new work area\ncreate a build system\nfind all the tools that change the files and change them to work on the files in \nthe work area (and note that some of these tools are distro provided that you \nwill have to fork, replace, or live without)\nchange all sysadmin work habits to use the new work area\nnot to mention, pick up the pieces when someone/something makes changes to the \nfiles directly.\n\nby comparison, working in /etc directly all you need to do is to train the \nsysadmins to do a commit after they make changes, and how to revert to the \nprior version if things break (and then train the senior sysadmins to do the \ncheckouts of specific versions, resolve merges, and other 'advanced' things to \npickup the pices when things get messed up)\n\nin fact, you can even get a lot of utility out of a daily cron job to do a \ncommit and teaching a few people about git while the remainder don't even know \nthat it's running.\n\neverything above is just dealing with the simple case of managing the config \nfiles on a single system\n\nnow, I agree that there are advantages to not working directly in /etc and \ninstead working in a seperate area with a build system to actually modify the \nfiles, but getting to that point is a lot of work, and getting everyone \n(including management) convinced that it's worth that much up-front effort for \nthe long-term benifits is hard. and if the sysadmins are under water currently, \nit won't happen becouse they can't let other things fall apart while they go and \nbuild this new system.\n\nI think this is a case of perfect being the enemy of better.\n\nthe simple case of handling files in /etc/directly is so non-intrusive that \ndistros could ship it with a nightly cron job enabled by default (ideally the \ncron job would detect that no changes were made and not do a commit) and once \nyou have a SCM in place managing the files by default it's much more likely that \nsysadmin tools will start to be changed to work with it directly.\n\nThis should not be a huge change for git to suppor this either (although I admit \nit is a change)\n\nwith filters enabled at all you need to be able to disable any optimizations \nthat use the working tree instead of opeing up an object/pack\n\nso the remaining changes are\n\n1. define a dependancy that allows for filter config files to be extracted \nbefore the filter is run on checkout\n\n2. allow the filter to do git-add of a file while it is running on git-add (or \notherwise tell git that a file needs to be added before a commit is done). I \ndon't know if this would work today, or if there are locks that would cause a \ndeadlock (I suspect a deadlock from comments about git-lib needing to be made \nre-entrent)\n\n3. move the filter from a pipe/pipe model to a pipe/file model (it's fair to \nrequire the filters to support - as a filename for stdin/stdout git needs this, \nit's fairly standard for unix utilities to do this)\n\nDavid Lang\n"}]}