{"thread":{"id":"24841","subject":"Fix for normalization of foreign idents","startedAt":"2010-08-23T07:05:23Z","lastAt":"2010-09-13T22:00:49Z","messageCount":19,"participants":["Marcus Comstedt","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"150249","messageId":"E1Ot4NP-0002xn-Nc@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"yf9sk1l73bt.fsf@chiyo.mc.pp.se","subject":"[PATCH v2 1/2] convert: fix normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-08-23T07:05:23Z","receivedAt":"2010-08-23T07:05:23Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"Since ident_to_worktree() does not touch $Id$ tags which contain\nforeign expansions in the repository, make sure that ident_to_git()\ndoes not either.  This fixes the problem that such files show\nspurious modification upon checkout.\n\nThere is however one case where we want ident_to_git() to normalize\nthe tag to $Id$ despite the asymmetry:  When committing a modification\nto a file which has a foreign ident, the foreign ident should be\nreplaced with a regular git ident.  Thus, add a new parameter to\nconvert_to_git() that indicates if we want the foreign idents\nnormalized after all.\n\nSigned-off-by: Marcus Comstedt <marcus@mc.pp.se>\n---\n builtin/apply.c |    2 +-\n builtin/blame.c |    2 +-\n cache.h         |    3 ++-\n combine-diff.c  |    2 +-\n convert.c       |   23 ++++++++++++++++++-----\n diff.c          |    2 +-\n sha1_file.c     |    3 ++-\n 7 files changed, 26 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 23c18c5..7abff80 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1932,7 +1932,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(\"unable to open or read %s\", path);\n-\t\tconvert_to_git(path, buf->buf, buf->len, buf, 0);\n+\t\tconvert_to_git(path, buf->buf, buf->len, buf, 0, 0);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 1015354..4f3b004 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2095,7 +2095,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \t\tif (strbuf_read(&buf, 0, 0) < 0)\n \t\t\tdie_errno(\"failed to read from stdin\");\n \t}\n-\tconvert_to_git(path, buf.buf, buf.len, &buf, 0);\n+\tconvert_to_git(path, buf.buf, buf.len, &buf, 0, 0);\n \torigin->file.ptr = buf.buf;\n \torigin->file.size = buf.len;\n \tpretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);\ndiff --git a/cache.h b/cache.h\nindex be02a42..23ae1f1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1055,7 +1055,8 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);\n /* convert.c */\n /* returns 1 if *dst was used */\n extern int convert_to_git(const char *path, const char *src, size_t len,\n-                          struct strbuf *dst, enum safe_crlf checksafe);\n+\t\t\t  struct strbuf *dst, enum safe_crlf checksafe,\n+\t\t\t  int normalize_foreign_ident);\n extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);\n extern int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst);\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex 655fa89..e81aa7d 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -758,7 +758,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\tif (is_file) {\n \t\t\t\tstruct strbuf buf = STRBUF_INIT;\n \n-\t\t\t\tif (convert_to_git(elem->path, result, len, &buf, safe_crlf)) {\n+\t\t\t\tif (convert_to_git(elem->path, result, len, &buf, safe_crlf, 0)) {\n \t\t\t\t\tfree(result);\n \t\t\t\t\tresult = strbuf_detach(&buf, &len);\n \t\t\t\t\tresult_size = len;\ndiff --git a/convert.c b/convert.c\nindex 01de9a8..6abab3f 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -520,9 +520,10 @@ static int count_ident(const char *cp, unsigned long size)\n }\n \n static int ident_to_git(const char *path, const char *src, size_t len,\n-                        struct strbuf *buf, int ident)\n+\t\t\tstruct strbuf *buf, int ident,\n+\t\t\tint normalize_foreign_ident)\n {\n-\tchar *dst, *dollar;\n+\tchar *dst, *dollar, *spc;\n \n \tif (!ident || !count_ident(src, len))\n \t\treturn 0;\n@@ -549,6 +550,16 @@ static int ident_to_git(const char *path, const char *src, size_t len,\n \t\t\t\tcontinue;\n \t\t\t}\n \n+\t\t\tspc = memchr(src + 4, ' ', dollar - src - 4);\n+\t\t\tif (spc && spc < dollar-1 &&\n+\t\t\t    !normalize_foreign_ident) {\n+\t\t\t\t/* There are spaces in unexpected places.\n+\t\t\t\t * This is probably an id from some other\n+\t\t\t\t * versioning system. Keep it for now.\n+\t\t\t\t */\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n \t\t\tmemcpy(dst, \"Id$\", 3);\n \t\t\tdst += 3;\n \t\t\tlen -= dollar + 1 - src;\n@@ -706,7 +717,8 @@ static enum action determine_action(enum action text_attr, enum eol eol_attr)\n }\n \n int convert_to_git(const char *path, const char *src, size_t len,\n-                   struct strbuf *dst, enum safe_crlf checksafe)\n+\t\t   struct strbuf *dst, enum safe_crlf checksafe,\n+\t\t   int normalize_foreign_ident)\n {\n \tstruct git_attr_check check[5];\n \tenum action action = CRLF_GUESS;\n@@ -738,7 +750,8 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\treturn ret | ident_to_git(path, src, len, dst, ident);\n+\treturn ret | ident_to_git(path, src, len, dst, ident,\n+\t\t\t\t  normalize_foreign_ident);\n }\n \n static int convert_to_working_tree_internal(const char *path, const char *src,\n@@ -796,5 +809,5 @@ int renormalize_buffer(const char *path, const char *src, size_t len, struct str\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\treturn ret | convert_to_git(path, src, len, dst, 0);\n+\treturn ret | convert_to_git(path, src, len, dst, 0, 0);\n }\ndiff --git a/diff.c b/diff.c\nindex 144f2aa..d481cb6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2372,7 +2372,7 @@ 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\tif (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf)) {\n+\t\tif (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf, 0)) {\n \t\t\tsize_t size = 0;\n \t\t\tmunmap(s->data, s->size);\n \t\t\ts->should_munmap = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 0cd9435..37e8657 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2434,7 +2434,8 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \tif ((type == OBJ_BLOB) && path) {\n \t\tstruct strbuf nbuf = STRBUF_INIT;\n \t\tif (convert_to_git(path, buf, size, &nbuf,\n-\t\t                   write_object ? safe_crlf : 0)) {\n+\t\t\t\t   write_object ? safe_crlf : 0,\n+\t\t\t\t   write_object)) {\n \t\t\tbuf = strbuf_detach(&nbuf, &size);\n \t\t\tre_allocated = 1;\n \t\t}\n-- \n1.7.2\n"},{"id":"148785","messageId":"1282599032-11369-1-git-send-email-marcus@mc.pp.se","threadId":"24841","inReplyTo":null,"subject":"Fix for normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-08-23T21:30:31Z","receivedAt":"2010-08-23T21:30:31Z","isPatch":false,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"Hi.\n\nThe new behaviour that $Id$ tags containing expanded idents from\nother version control systems, nice though it is, has a rather\nserious problem.  This is because convert_to_git is no longer\nthe inverse operation of convert_to_working_tree.  For native\ngit idents, the transformations are\n\n  $Id$ --(c_t_w_t)--> $Id: 123...$ --(c_t_g)--> $Id$\n\nbut for foreign idents, it becomes\n\n  $Id: blah$ --(c_t_w_t)--> $Id: blah$ --(c_t_g)--> $Id$\n\nThe result of this is that git may consider even newly checked out\nfiles as modified, even though neither the file contents nor its\nattributes have been modified after the checkout.  I say _may_, because\nit can also happen that it decides based on the time stamps that it\ndoesn't need to compare the actual contents, in which case the file\ndoes not show as modified.\n\nThe following patch fixes this by preserving the foreign ident\nalso in convert_to_git, meaning that convert_to_git is again the\ninverse operation of convert_to_working_tree, with the following\ntransformation series:\n\n  $Id: blah$ --(c_t_w_t)--> $Id: blah$ --(c_t_g)--> $Id: blah$\n\nThis restores correct and deterministic operation of status and\ndiff, meaning that if the file hasn't actually been modified, no\nmodifications are shown.\n\nAs you might suggest, always keeping the foreign ident would mean it\nis never updated when you commit new versions of the file, which isn't\nreally what we want.  Keeping the foreign ident as long as the last\nmodification to the file was made in the previous version control\nsystem makes perfect sense, but once we make a commit to the file\nwithin git, it should be replaced with a git ident.  The patch is\ntherefore slightly more complex, adding an extra parameter to control\nwhether foreign idents are collapsed or not.  This parameter is set\nto true only in the case when index_mem is called with write_object\nset to true, which is to say when we create a new blob from the\nworking tree (i.e. we are committing the file).\n\nI hope we can agree that this is a sound and unintrusive way of\nhandling the problem.  :-)\n\nIncidentally, should one want to create a commit to replace a foreign\nident with a native git one without making any other changes to the\nfile, this is still simple to do.  All that is needed is to change any\ncharacter inside the expanded ident in the working tree, and the file\nwill show as modified, and will have the foreign ident removed on\ncommit.\n\n\n  // Marcus\n"},{"id":"148786","messageId":"1282599032-11369-2-git-send-email-marcus@mc.pp.se","threadId":"24841","inReplyTo":"1282599032-11369-1-git-send-email-marcus@mc.pp.se","subject":"[PATCH] convert: fix normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-08-23T21:30:32Z","receivedAt":"2010-08-23T21:30:32Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"Since ident_to_worktree() does not touch $Id$ tags which contain\nforeign expansions in the repository, make sure that ident_to_git()\ndoes not either.  This fixes the problem that such files show\nspurious modification upon checkout.\n\nThere is however one case where we want ident_to_git() to normalize\nthe tag to $Id$ despite the asymmetry:  When committing a modification\nto a file which has a foreign ident, the foreign ident should be\nreplaced with a regular git ident.  Thus, add a new parameter to\nconvert_to_git() that indicates if we want the foreign idents\nnormalized after all.\n\nSigned-off-by: Marcus Comstedt <marcus@mc.pp.se>\n---\n builtin/apply.c |    2 +-\n builtin/blame.c |    2 +-\n cache.h         |    3 ++-\n combine-diff.c  |    2 +-\n convert.c       |   21 +++++++++++++++++----\n diff.c          |    2 +-\n sha1_file.c     |    3 ++-\n 7 files changed, 25 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex f38c1f7..7e503f4 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1759,7 +1759,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(\"unable to open or read %s\", path);\n-\t\tconvert_to_git(path, buf->buf, buf->len, buf, 0);\n+\t\tconvert_to_git(path, buf->buf, buf->len, buf, 0, 0);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 437b1a4..83c6561 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2095,7 +2095,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \t\tif (strbuf_read(&buf, 0, 0) < 0)\n \t\t\tdie_errno(\"failed to read from stdin\");\n \t}\n-\tconvert_to_git(path, buf.buf, buf.len, &buf, 0);\n+\tconvert_to_git(path, buf.buf, buf.len, &buf, 0, 0);\n \torigin->file.ptr = buf.buf;\n \torigin->file.size = buf.len;\n \tpretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);\ndiff --git a/cache.h b/cache.h\nindex eb77e1d..b98042f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1055,7 +1055,8 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);\n /* convert.c */\n /* returns 1 if *dst was used */\n extern int convert_to_git(const char *path, const char *src, size_t len,\n-                          struct strbuf *dst, enum safe_crlf checksafe);\n+\t\t\t  struct strbuf *dst, enum safe_crlf checksafe,\n+\t\t\t  int normalize_foreign_ident);\n extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);\n \n /* add */\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 655fa89..e81aa7d 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -758,7 +758,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\tif (is_file) {\n \t\t\t\tstruct strbuf buf = STRBUF_INIT;\n \n-\t\t\t\tif (convert_to_git(elem->path, result, len, &buf, safe_crlf)) {\n+\t\t\t\tif (convert_to_git(elem->path, result, len, &buf, safe_crlf, 0)) {\n \t\t\t\t\tfree(result);\n \t\t\t\t\tresult = strbuf_detach(&buf, &len);\n \t\t\t\t\tresult_size = len;\ndiff --git a/convert.c b/convert.c\nindex e41a31e..00d0612 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -519,9 +519,10 @@ static int count_ident(const char *cp, unsigned long size)\n }\n \n static int ident_to_git(const char *path, const char *src, size_t len,\n-                        struct strbuf *buf, int ident)\n+\t\t\tstruct strbuf *buf, int ident,\n+\t\t\tint normalize_foreign_ident)\n {\n-\tchar *dst, *dollar;\n+\tchar *dst, *dollar, *spc;\n \n \tif (!ident || !count_ident(src, len))\n \t\treturn 0;\n@@ -548,6 +549,16 @@ static int ident_to_git(const char *path, const char *src, size_t len,\n \t\t\t\tcontinue;\n \t\t\t}\n \n+\t\t\tspc = memchr(src + 4, ' ', dollar - src - 4);\n+\t\t\tif (spc && spc < dollar-1 &&\n+\t\t\t    !normalize_foreign_ident) {\n+\t\t\t\t/* There are spaces in unexpected places.\n+\t\t\t\t * This is probably an id from some other\n+\t\t\t\t * versioning system. Keep it for now.\n+\t\t\t\t */\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n \t\t\tmemcpy(dst, \"Id$\", 3);\n \t\t\tdst += 3;\n \t\t\tlen -= dollar + 1 - src;\n@@ -704,7 +715,8 @@ enum action determine_action(enum action text_attr, enum eol eol_attr) {\n }\n \n int convert_to_git(const char *path, const char *src, size_t len,\n-                   struct strbuf *dst, enum safe_crlf checksafe)\n+\t\t   struct strbuf *dst, enum safe_crlf checksafe,\n+\t\t   int normalize_foreign_ident)\n {\n \tstruct git_attr_check check[5];\n \tenum action action = CRLF_GUESS;\n@@ -736,7 +748,8 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\treturn ret | ident_to_git(path, src, len, dst, ident);\n+\treturn ret | ident_to_git(path, src, len, dst, ident,\n+\t\t\t\t  normalize_foreign_ident);\n }\n \n int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\ndiff --git a/diff.c b/diff.c\nindex 6fb97d4..c5be513 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2372,7 +2372,7 @@ 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\tif (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf)) {\n+\t\tif (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf, 0)) {\n \t\t\tsize_t size = 0;\n \t\t\tmunmap(s->data, s->size);\n \t\t\ts->should_munmap = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 0cd9435..37e8657 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2434,7 +2434,8 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \tif ((type == OBJ_BLOB) && path) {\n \t\tstruct strbuf nbuf = STRBUF_INIT;\n \t\tif (convert_to_git(path, buf, size, &nbuf,\n-\t\t                   write_object ? safe_crlf : 0)) {\n+\t\t\t\t   write_object ? safe_crlf : 0,\n+\t\t\t\t   write_object)) {\n \t\t\tbuf = strbuf_detach(&nbuf, &size);\n \t\t\tre_allocated = 1;\n \t\t}\n-- \n1.7.2\n"},{"id":"148787","messageId":"20100823213531.GD2120@burratino","threadId":"24841","inReplyTo":"1282599032-11369-1-git-send-email-marcus@mc.pp.se","subject":"Re: Fix for normalization of foreign idents","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-23T21:35:31Z","receivedAt":"2010-08-23T21:35:31Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nMarcus Comstedt wrote:\n\n>   $Id: blah$ --(c_t_w_t)--> $Id: blah$ --(c_t_g)--> $Id: blah$\n> \n> This restores correct and deterministic operation of status and\n> diff, meaning that if the file hasn't actually been modified, no\n> modifications are shown.\n> \n> As you might suggest, always keeping the foreign ident would mean it\n> is never updated when you commit new versions of the file, which isn't\n> really what we want.  Keeping the foreign ident as long as the last\n> modification to the file was made in the previous version control\n> system makes perfect sense, but once we make a commit to the file\n> within git, it should be replaced with a git ident.\n\nI was with you up to here.  Is commit time really the right moment\nto clobber a foreign ident?  I suspect it would be confusing.\n\nIt might be simpler to just never clobber a foreign ident and instead\nrely on local policy (scripts, pre-commit hooks, and so on) to remove\nthem at the appropriate time.\n"},{"id":"148788","messageId":"yf97hjhrol5.fsf@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"20100823213531.GD2120@burratino","subject":"Re: Fix for normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-08-23T21:44:38Z","receivedAt":"2010-08-23T21:44:38Z","isPatch":false,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"\nHi Jonathan.\n\nThanks for the quick feedback.\n\n\nJonathan Nieder <jrnieder@gmail.com> writes:\n\n> I was with you up to here.  Is commit time really the right moment\n> to clobber a foreign ident?  I suspect it would be confusing.\n\nWell, it's how ident is normally expected to behave; when you\ncommit something new, the file should get a new ident.\n\n\n> It might be simpler to just never clobber a foreign ident and instead\n> rely on local policy (scripts, pre-commit hooks, and so on) to remove\n> them at the appropriate time.\n\nSimpler as far as patch size is concerned (although this patch isn't\nreally that complex as it is), but it sounds like much more of a\nhassle to actually use.  Do you have a use case in mind where you have\nthe ident attribute on a file but do _not_ want a new ident each time\nyou commit a change to the file?\n\n\n  // Marcus\n"},{"id":"148791","messageId":"20100823223321.GE1308@burratino","threadId":"24841","inReplyTo":"yf97hjhrol5.fsf@chiyo.mc.pp.se","subject":"Re: Fix for normalization of foreign idents","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-23T22:33:21Z","receivedAt":"2010-08-23T22:33:21Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Marcus Comstedt wrote:\n\n> it sounds like much more of a\n> hassle to actually use.  Do you have a use case in mind where you have\n> the ident attribute on a file but do _not_ want a new ident each time\n> you commit a change to the file?\n\nNo, I don't use the $Id$ feature at all and if I inherited a codebase\nwith a bunch of foreign $Id$ tags, I don't know what I'd do. :)  So I\ncan trust your judgement on this.\n"},{"id":"148799","messageId":"7vd3t8zyc0.fsf@alter.siamese.dyndns.org","threadId":"24841","inReplyTo":"20100823223321.GE1308@burratino","subject":"Re: Fix for normalization of foreign idents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-23T23:46:55Z","receivedAt":"2010-08-23T23:46:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Marcus Comstedt wrote:\n>\n>> it sounds like much more of a\n>> hassle to actually use.  Do you have a use case in mind where you have\n>> the ident attribute on a file but do _not_ want a new ident each time\n>> you commit a change to the file?\n>\n> No, I don't use the $Id$ feature at all and if I inherited a codebase\n> with a bunch of foreign $Id$ tags, I don't know what I'd do.\n\nHeh, I know what I would do---the first commit will be to remove them.\n"},{"id":"148828","messageId":"yf9r5ho79tg.fsf@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"7vd3t8zyc0.fsf@alter.siamese.dyndns.org","subject":"Re: Fix for normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-08-24T07:23:55Z","receivedAt":"2010-08-24T07:23:55Z","isPatch":false,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>\n>> No, I don't use the $Id$ feature at all and if I inherited a codebase\n>> with a bunch of foreign $Id$ tags, I don't know what I'd do.\n>\n> Heh, I know what I would do---the first commit will be to remove them.\n\n\nRemove just the foreign ident, or remove $Id$ completely?\n\nEither way, this doesn't sound like a vote for preserving the old $Id$\nexpansion on commit.  :-)\n\n\n  // Marcus\n"},{"id":"150059","messageId":"yf9vd6j5hti.fsf@mc.pp.se","threadId":"24841","inReplyTo":"yf97hjhrol5.fsf@chiyo.mc.pp.se","subject":"Re: Fix for normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-09-06T09:42:33Z","receivedAt":"2010-09-06T09:42:33Z","isPatch":false,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"\nHi.\n\nWas this patch simply forgotten, or are there some remaining\nconcerns about it?\n\nShould I submit a new patch which simply fixes the inconsistency\nwhich breaks checkout, and leaves the removal of foreign idents\non commit to user interaction or hook scripts, as suggested by\nJonathan?  That would at least restore deterministic behavior...\n\n\n  // Marcus\n"},{"id":"150134","messageId":"20100906210719.GD26371@burratino","threadId":"24841","inReplyTo":"yf9vd6j5hti.fsf@mc.pp.se","subject":"Re: Fix for normalization of foreign idents","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-06T21:07:19Z","receivedAt":"2010-09-06T21:07:19Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Marcus,\n\nMarcus Comstedt wrote:\n\n> Was this patch simply forgotten, or are there some remaining\n> concerns about it?\n\nI assume it is just that no one using the $ident$ feature took a\nlook at it, which leaves us without a sanity-check that it\nconsistently works and improves things.\n\nIf you have the time, a test and documentation might help (the former\nplays the role of an artificial user, who can describe the feature and\nwill make noise if we break it with future changes).\n\n> Should I submit a new patch which simply fixes the inconsistency\n> which breaks checkout, and leaves the removal of foreign idents\n> on commit to user interaction or hook scripts, as suggested by\n> Jonathan?  That would at least restore deterministic behavior...\n\nThat doesn't sound necessary to me.\n"},{"id":"150247","messageId":"E1Ot4NU-0002xs-OO@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"yf9sk1l73bt.fsf@chiyo.mc.pp.se","subject":"[PATCH v2 2/2] t0021: test checkout and commit of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-09-07T19:16:03Z","receivedAt":"2010-09-07T19:16:03Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"Add test cases for the following behaviors:\n\n  * Checking out a file with a foreign ident should not flag\n    the file as modified.  This is to prevent a mess when checking\n    out old versions, and to allow a migration model where files\n    are allowed to keep their foreign ident as long as their\n    content is also \"foreign\", i.e. not modified since the migration\n    to git.\n\n  * Committing to a file with a foreign ident should replace the\n    foreign ident with a native ident.  This is simply to get\n    the normal behavior of ident:  When the contents of the file is\n    updated, so is the ident.\n\nSigned-off-by: Marcus Comstedt <marcus@mc.pp.se>\n---\n t/t0021-conversion.sh |   58 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 58 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 828e35b..c0ad9e8 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -93,4 +93,62 @@ test_expect_success expanded_in_repo '\n \tcmp expanded-keywords expected-output\n '\n \n+# Check that a file containing idents (native or foreign) is not\n+# spuriously flagged as modified on checkout\n+test_expect_success 'ident pristine after checkout' '\n+\t{\n+\t\techo \"File with expanded keywords\"\n+\t\techo \"\\$Id\\$\"\n+\t\techo \"\\$Id: Foreign Commit With Spaces \\$\"\n+\t} > native-and-foreign-idents &&\n+\n+\t{\n+\t\techo \"File with expanded keywords\"\n+\t\techo \"\\$Id: 9bdc217750894eed31bb870e9ffa00599f0573a2 \\$\"\n+\t\techo \"\\$Id: Foreign Commit With Spaces \\$\"\n+\t} > expected-output &&\n+\n+\tgit add native-and-foreign-idents &&\n+\tgit commit -m \"File with native and foreign idents\" &&\n+\n+\techo \"native-and-foreign-idents ident\" >> .gitattributes &&\n+\n+\trm -f native-and-foreign-idents &&\n+\tgit checkout -- native-and-foreign-idents &&\n+\tcat native-and-foreign-idents &&\n+\tcmp native-and-foreign-idents expected-output &&\n+\ttouch native-and-foreign-idents &&\n+\tgit status --porcelain native-and-foreign-idents > output &&\n+\ttest ! -s output &&\n+\tgit diff -- native-and-foreign-idents > output &&\n+\ttest ! -s output\n+'\n+\n+# Check that actually modifying the file and committing it produces a\n+# new ident on checkout\n+test_expect_success 'foreign ident replaced on commit' '\n+\t{\n+\t\techo \"File with expanded keywords\"\n+\t\techo \"\\$Id: 2499e26293e36cc92399835e497ef6396d710055 \\$\"\n+\t\techo \"\\$Id: 2499e26293e36cc92399835e497ef6396d710055 \\$\"\n+\t\techo \"Some new content\"\n+\t} > expected-output &&\n+\n+\techo \"1\t0\tnative-and-foreign-idents\" > expected-stat1 &&\n+\techo \"2\t1\tnative-and-foreign-idents\" > expected-stat2 &&\n+\n+\techo \"Some new content\" >> native-and-foreign-idents &&\n+\tgit diff --numstat -- native-and-foreign-idents > output &&\n+\tcmp output expected-stat1 &&\n+\tgit add native-and-foreign-idents &&\n+\tgit commit -m \"Modified file\" &&\n+\tgit diff --numstat HEAD^ HEAD -- native-and-foreign-idents > output &&\n+\tcmp output expected-stat2 &&\n+\trm -f native-and-foreign-idents &&\n+\tgit checkout -- native-and-foreign-idents &&\n+\tcat native-and-foreign-idents &&\n+\tcmp native-and-foreign-idents expected-output\n+'\n+\n+\n test_done\n-- \n1.7.2\n"},{"id":"150244","messageId":"yf9sk1l73bt.fsf@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"20100906210719.GD26371@burratino","subject":"Re: Fix for normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-09-07T19:37:10Z","receivedAt":"2010-09-07T19:37:10Z","isPatch":false,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"\nJonathan Nieder <jrnieder@gmail.com> writes:\n\n> If you have the time, a test and documentation might help (the former\n> plays the role of an artificial user, who can describe the feature and\n> will make noise if we break it with future changes).\n\nOk.  I'll shortly post an updated patch with some test cases.\n\nAs for documentation, I suppose that would mean documenting the\n\"foreign ident\" concept as a whole, as I don't think there's currently\nanything about that in the documentation?  Would the `ident` section\nof gitattributes.txt be a suitable place for this?\n\n\nThanks\n\n\n  // Marcus\n"},{"id":"150246","messageId":"E1Ot4NM-0002xk-Ed@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"yf9sk1l73bt.fsf@chiyo.mc.pp.se","subject":"[PATCH v2 0/2] fix normalization of foreign idents (now with test cases)","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-09-07T20:00:24Z","receivedAt":"2010-09-07T20:00:24Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"* Rebased against current master (7505ae2)\n* Test cases added\n\nMarcus Comstedt (2):\n  convert: fix normalization of foreign idents\n  t0021: test checkout and commit of foreign idents\n\n builtin/apply.c       |    2 +-\n builtin/blame.c       |    2 +-\n cache.h               |    3 +-\n combine-diff.c        |    2 +-\n convert.c             |   23 +++++++++++++++----\n diff.c                |    2 +-\n sha1_file.c           |    3 +-\n t/t0021-conversion.sh |   58 +++++++++++++++++++++++++++++++++++++++++++++++++\n 8 files changed, 84 insertions(+), 11 deletions(-)\n\n-- \n1.7.2\n"},{"id":"150275","messageId":"20100908043227.GD24444@capella.cs.uchicago.edu","threadId":"24841","inReplyTo":"yf9sk1l73bt.fsf@chiyo.mc.pp.se","subject":"Re: Fix for normalization of foreign idents","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-08T04:32:28Z","receivedAt":"2010-09-08T04:32:28Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Marcus Comstedt wrote:\n\n> Ok.  I'll shortly post an updated patch with some test cases.\n\nThanks.\n\n> As for documentation, I suppose that would mean documenting the\n> \"foreign ident\" concept as a whole, as I don't think there's currently\n> anything about that in the documentation?  Would the `ident` section\n> of gitattributes.txt be a suitable place for this?\n\nYep, that sounds good (though by no means necessary either).\n"},{"id":"150422","messageId":"7vmxrqjvf6.fsf@alter.siamese.dyndns.org","threadId":"24841","inReplyTo":"E1Ot4NP-0002xn-Nc@chiyo.mc.pp.se","subject":"Re: [PATCH v2 1/2] convert: fix normalization of foreign idents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-10T00:26:21Z","receivedAt":"2010-09-10T00:26:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marcus Comstedt <marcus@mc.pp.se> writes:\n\n> Since ident_to_worktree() does not touch $Id$ tags which contain\n> foreign expansions in the repository, make sure that ident_to_git()\n> does not either.  This fixes the problem that such files show\n> spurious modification upon checkout.\n>\n> There is however one case where we want ident_to_git() to normalize\n> the tag to $Id$ despite the asymmetry:  When committing a modification\n> to a file which has a foreign ident, the foreign ident should be\n> replaced with a regular git ident.  Thus, add a new parameter to\n> convert_to_git() that indicates if we want the foreign idents\n> normalized after all.\n\nWould it be possible that the real culprit is that ident_to_worktree()\ndoes not always touch $Id$ in the first place?  Why isn't \"$Id: garbage$\"\nfirst cleaned and then smudged upon checkout?\n\nIt also smells wrong that this \"sometimes we convert, sometimes we don't\"\nis a special case for \"$Id$\" and for no other conversion.  Why don't\nsmudge/clean filter or CRLF conversion have the same issue that can be\nsolved with the same approach as this patch takes?\n"},{"id":"150559","messageId":"yf9hbhuisla.fsf@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"7vmxrqjvf6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] convert: fix normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-09-12T21:01:53Z","receivedAt":"2010-09-12T21:01:53Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"\nHi Junio.\n\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Would it be possible that the real culprit is that ident_to_worktree()\n> does not always touch $Id$ in the first place?  Why isn't \"$Id: garbage$\"\n> first cleaned and then smudged upon checkout?\n\nPlease see commit 07814d90095b65b4594efd47c69f9f171ef162d4, and\nthe discussion preceeding it.\n\n\n> It also smells wrong that this \"sometimes we convert, sometimes we don't\"\n> is a special case for \"$Id$\" and for no other conversion.  Why don't\n> smudge/clean filter or CRLF conversion have the same issue that can be\n> solved with the same approach as this patch takes?\n\nI gather that this is because nobody has come up with a use case\nfor smudge/clean or CRLF where a (pervasive) non-normalized\nrepresentation in the repository makes sense.\n\nSpecifically, a foreign ident in the repo is not \"garbage\", but\nsomething useful when you migrate a repo from a different VCS for (at\nleast) the following reasons:\n\n* It allows you to check out a historical tree from the git repo which\n  looks exactly like what it would look like if you checked it out\n  from the previous system\n\n* It provides an indication that a version of a file comes directly\n  from the previous VCS, without any modification since the migration\n  to git, and exactly where in the history of the previous VCS it has\n  originated\n\n* Quite frankly, idents generated by other VCSs contain more useful\n  information than those generated by git, so it's a waste to discard\n  them prematurely\n\nThe same effect can be achieved without direct support for foreign\nidents by instead using fine-grained control in .gitattributes to\nforce -ident on any file which still has foreign idents, but there are\ntwo downsides to this approach:\n\n* A commit hook (and probably a pre-receive hook at the \"blessed\"\n  repository) is needed to make sure that no commits are allowed to a\n  file with foreign idents without also flipping the attribute from\n  -ident to +ident\n\n* Either a full enumeration of all files with foreign idents, or\n  of all files with native idents, is needed in .gitattributes, so\n  that a file can be either added to or removed from this list when\n  making the first \"native\" commit to it\n\nSo it's possible, albeit slightly less practical, to do without this\nfeature.  If the decision to include it is reversed, 07814d90095b65\nshould probably be reverted.\n\n\n  // Marcus\n"},{"id":"150561","messageId":"7vd3sid4bo.fsf@alter.siamese.dyndns.org","threadId":"24841","inReplyTo":"yf9hbhuisla.fsf@chiyo.mc.pp.se","subject":"Re: [PATCH v2 1/2] convert: fix normalization of foreign idents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-12T21:44:59Z","receivedAt":"2010-09-12T21:44:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marcus Comstedt <marcus@mc.pp.se> writes:\n\n>> It also smells wrong that this \"sometimes we convert, sometimes we don't\"\n>> is a special case for \"$Id$\" and for no other conversion.  Why don't\n>> smudge/clean filter or CRLF conversion have the same issue that can be\n>> solved with the same approach as this patch takes?\n>\n> I gather that this is because nobody has come up with a use case\n> for smudge/clean or CRLF where a (pervasive) non-normalized\n> representation in the repository makes sense.\n\nThink a bit more about what you just wrote means.\n\nImagine there isn't any \"$Id$\" (or \"$ident$\" as it was originally known)\nexpansion in git.  You can implement it easily using a smudge/clean pair,\nand the smudge and clean should be conditionally applied in the codepath\nyou touched using exactly the same logic as your patch uses, no?\n\nThat is what I meant.  It smells wrong to make this \"sometime we do,\nsometimes we don't\" as a special case for \"$Id$\".  Specifically, the\nparameter name \"normalize_foreign_ident\" feels wrong; the concept that the\nparameter tries to convey covers much wider than just \"foreign ident\", no?\n"},{"id":"150562","messageId":"yf9d3siiplc.fsf@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"7vd3sid4bo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] convert: fix normalization of foreign idents","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-09-12T22:06:39Z","receivedAt":"2010-09-12T22:06:39Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Imagine there isn't any \"$Id$\" (or \"$ident$\" as it was originally known)\n> expansion in git.  You can implement it easily using a smudge/clean pair,\n\nSorry, but here I have to go off a little at a tangent.  Yes, you\ncould implement the ident-expansion currently provided by git as a\nsmudge/clean pair.  However, you could not implement an ident which\nactually puts something more useful (such as the id of the commit\nwhere the file was last modified) into the id string by using\nsmudge/clean.  I know, because I tried to do just that.  ;-)\nThe reason:  smudge/clean do not get the pathname, so they are not\nable to query any information about the file from the repository.\nI might submit a patch adressing this issue later.\n\n\n> and the smudge and clean should be conditionally applied in the codepath\n> you touched using exactly the same logic as your patch uses, no?\n>\n> That is what I meant.  It smells wrong to make this \"sometime we do,\n> sometimes we don't\" as a special case for \"$Id$\".  Specifically, the\n> parameter name \"normalize_foreign_ident\" feels wrong; the concept that the\n> parameter tries to convey covers much wider than just \"foreign ident\", no?\n\nOk, I think I follow where you are going.  _If_ we say that clean\n(and smudge?) should be able to run in different \"modes\", with\ncleaning for a commit being such an mode, then this ought to be\ntriggered by the same parameter, yes.  The parameter name describes\nwhat the parameter does now, but not necessarily what it would do in a\npossible future where such new concepts as modal clean scripts have\nbeen introduced.\n\nGenerally, I'm kind of wondering if the parameters of convert_to_git\nwouldn't be better off just specifying a mode (like the, perhaps also\nslightly mis-named, write_object paremeter to index_mem) rather than\ntrying to micro-manage specific features like they have before.  Was\nthat what you had in mind?\n\n\n  // Marcus\n"},{"id":"150650","messageId":"E1OvHRf-00064G-8q@chiyo.mc.pp.se","threadId":"24841","inReplyTo":"yf9d3siiplc.fsf@chiyo.mc.pp.se","subject":"[PATCH] convert: generalize checksafe parameter","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-09-13T22:00:49Z","receivedAt":"2010-09-13T22:00:49Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"The convert_to_git() function used to have a checksafe parameter,\nwhich could be used to prevent safe_crlf checks by passing 0\ninstead of the value of the global variable safe_crlf.\n\nSince preventing checks is a wider concept than just disabling\nsafe_crlf, generalize the parameter so that it can be used for other\ntypes of checks as well.\n\nSigned-off-by: Marcus Comstedt <marcus@mc.pp.se>\n---\n\nOk, here's a stab at generalizing the parameters which affect\nthe behavior of convert_to_git(), starting with the already\nexisting one.  I'd originally intended to combine it with the\nnew one, but I couldn't seem to find a correspondence between\nthe intended use of the normalized data and whether checks\nare allowed or not (for example, diff allows checks but blame\ndoes not), so I'm keeping that as a separate facet.\n\nI could add this to the current patch series, or rebase that\nagainst this.\n\n builtin/apply.c |    2 +-\n builtin/blame.c |    2 +-\n cache.h         |    7 ++++++-\n combine-diff.c  |    2 +-\n convert.c       |    7 ++++---\n diff.c          |    2 +-\n sha1_file.c     |    2 +-\n 7 files changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 23c18c5..638e7be 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1932,7 +1932,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(\"unable to open or read %s\", path);\n-\t\tconvert_to_git(path, buf->buf, buf->len, buf, 0);\n+\t\tconvert_to_git(path, buf->buf, buf->len, buf, CHECKS_DISALLOWED);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 1015354..850e165 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2095,7 +2095,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \t\tif (strbuf_read(&buf, 0, 0) < 0)\n \t\t\tdie_errno(\"failed to read from stdin\");\n \t}\n-\tconvert_to_git(path, buf.buf, buf.len, &buf, 0);\n+\tconvert_to_git(path, buf.buf, buf.len, &buf, CHECKS_DISALLOWED);\n \torigin->file.ptr = buf.buf;\n \torigin->file.size = buf.len;\n \tpretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);\ndiff --git a/cache.h b/cache.h\nindex 2ef2fa3..250abc1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -581,6 +581,11 @@ enum eol {\n \n extern enum eol eol;\n \n+enum allow_checks {\n+\tCHECKS_DISALLOWED = 0,\n+\tCHECKS_ALLOWED = 1,\n+};\n+\n enum branch_track {\n \tBRANCH_TRACK_UNSPECIFIED = -1,\n \tBRANCH_TRACK_NEVER = 0,\n@@ -1059,7 +1064,7 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);\n /* convert.c */\n /* returns 1 if *dst was used */\n extern int convert_to_git(const char *path, const char *src, size_t len,\n-                          struct strbuf *dst, enum safe_crlf checksafe);\n+                          struct strbuf *dst, enum allow_checks checksallowed);\n extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);\n extern int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst);\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex 655fa89..c7f132d 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -758,7 +758,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\tif (is_file) {\n \t\t\t\tstruct strbuf buf = STRBUF_INIT;\n \n-\t\t\t\tif (convert_to_git(elem->path, result, len, &buf, safe_crlf)) {\n+\t\t\t\tif (convert_to_git(elem->path, result, len, &buf, CHECKS_ALLOWED)) {\n \t\t\t\t\tfree(result);\n \t\t\t\t\tresult = strbuf_detach(&buf, &len);\n \t\t\t\t\tresult_size = len;\ndiff --git a/convert.c b/convert.c\nindex 01de9a8..8050c24 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -706,7 +706,7 @@ static enum action determine_action(enum action text_attr, enum eol eol_attr)\n }\n \n int convert_to_git(const char *path, const char *src, size_t len,\n-                   struct strbuf *dst, enum safe_crlf checksafe)\n+                   struct strbuf *dst, enum allow_checks checksallowed)\n {\n \tstruct git_attr_check check[5];\n \tenum action action = CRLF_GUESS;\n@@ -733,7 +733,8 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \t\tlen = dst->len;\n \t}\n \taction = determine_action(action, eol_attr);\n-\tret |= crlf_to_git(path, src, len, dst, action, checksafe);\n+\tret |= crlf_to_git(path, src, len, dst, action,\n+\t\t\t   (checksallowed? safe_crlf : 0));\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n@@ -796,5 +797,5 @@ int renormalize_buffer(const char *path, const char *src, size_t len, struct str\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\treturn ret | convert_to_git(path, src, len, dst, 0);\n+\treturn ret | convert_to_git(path, src, len, dst, CHECKS_DISALLOWED);\n }\ndiff --git a/diff.c b/diff.c\nindex 9a5c77c..ed74f6b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2375,7 +2375,7 @@ 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\tif (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf)) {\n+\t\tif (convert_to_git(s->path, s->data, s->size, &buf, CHECKS_ALLOWED)) {\n \t\t\tsize_t size = 0;\n \t\t\tmunmap(s->data, s->size);\n \t\t\ts->should_munmap = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 0cd9435..13624a6 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2434,7 +2434,7 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \tif ((type == OBJ_BLOB) && path) {\n \t\tstruct strbuf nbuf = STRBUF_INIT;\n \t\tif (convert_to_git(path, buf, size, &nbuf,\n-\t\t                   write_object ? safe_crlf : 0)) {\n+\t\t                   write_object ? CHECKS_ALLOWED : CHECKS_DISALLOWED)) {\n \t\t\tbuf = strbuf_detach(&nbuf, &size);\n \t\t\tre_allocated = 1;\n \t\t}\n-- \n1.7.2\n"}]}