{"thread":{"id":"24191","subject":"[PATCH v3 0/3] Help merging when text has been normalized","startedAt":"2010-06-24T20:44:29Z","lastAt":"2010-06-25T08:58:54Z","messageCount":9,"participants":["Eyvind Bernhardsen","Johannes Sixt","Finn Arne Gangstad"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"144200","messageId":"cover.1277408598.git.eyvind.bernhardsen@gmail.com","threadId":"24191","inReplyTo":null,"subject":"[PATCH v3 0/3] Help merging when text has been normalized","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind.bernhardsen@gmail.com","sentAt":"2010-06-24T20:44:29Z","receivedAt":"2010-06-24T20:44:29Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"Here's a slightly expanded series based on my ll-merge normalization\npatch.  I've been working in a repository that uses \"* text=auto\" at\n$dayjob, and this series helps a lot when merging older,\npre-normalization branches.\n\nThe first commit is a cleaned-up version of the previous patch.  I\nborrowed Finn Arne's explanation for why it's useful for my commit\nmessage, ASCII art and all.  The idea is pretty simple: normalization\nmight differ between branches, so renormalization is required to prevent\n(or at least reduce) conflicts.\n\nThe second fixes a similar problem, but for delete/modify conflicts.  If\na file is normalized on a branch but deleted in another, merging the\nbranches will cause a conflict even though the actual content of the\nfile hasn't changed at all.\n\nIf the merge-base version of the file and the modified file differ they\nare both normalized, and if either was changed by normalization they are\ncompared.  If they compare equal, the file is considered unmodified and\nis deleted.\n\nThe third patch prevents normalization from expanding and then\ncollapsing CRLFs, saving time and memory when core.autocrlf=true or\ncore.eol=crlf.\n\n- Eyvind\n\nEyvind Bernhardsen (3):\n  Avoid conflicts when merging branches with mixed normalization\n  Try normalizing files to avoid delete/modify conflicts when merging\n  Don't expand CRLFs when normalizing text during merge\n\n cache.h                    |    1 +\n convert.c                  |   27 ++++++++++++++++----\n ll-merge.c                 |   12 +++++++++\n merge-recursive.c          |   42 ++++++++++++++++++++++++++++++-\n t/t6038-merge-text-auto.sh |   58 ++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 133 insertions(+), 7 deletions(-)\n create mode 100755 t/t6038-merge-text-auto.sh\n\n-- \n1.7.1.575.g383de\n"},{"id":"144201","messageId":"c6365533ce03625b362fd432d1c3276d335cba89.1277408598.git.eyvind.bernhardsen@gmail.com","threadId":"24191","inReplyTo":"cover.1277408598.git.eyvind.bernhardsen@gmail.com","subject":"[PATCH v3 1/3] Avoid conflicts when merging branches with mixed normalization","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind.bernhardsen@gmail.com","sentAt":"2010-06-24T20:44:30Z","receivedAt":"2010-06-24T20:44:30Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"Currently, merging across changes in line ending normalization is\npainful since files containing CRLF will conflict with normalized files,\neven if the only difference between the two versions is the line\nendings.  Additionally, any \"real\" merge conflicts that exist are\nobscured because every line in the file has a conflict.\n\nAssume you start out with a repo that has a lot of text files with CRLF\nchecked in (A):\n\n      o---C\n     /     \\\n    A---B---D\n\nB: Add \"* text=auto\" to .gitattributes and normalize all files to\n   LF-only\n\nC: Modify some of the text files\n\nD: Try to merge C\n\nYou will get a ridiculous number of LF/CRLF conflicts when trying to\nmerge C into D, since the repository contents for C are \"wrong\" wrt the\nnew .gitattributes file.\n\nFix ll-merge so that the \"base\", \"theirs\" and \"ours\" stages are passed\nthrough convert_to_worktree() and convert_to_git() before a three-way\nmerge.  This ensures that all three stages are normalized in the same\nway, removing from consideration differences that are only due to\nnormalization.\n\nSigned-off-by: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>\n---\n cache.h                    |    1 +\n convert.c                  |   11 +++++++-\n ll-merge.c                 |   12 +++++++++\n t/t6038-merge-text-auto.sh |   58 ++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 81 insertions(+), 1 deletions(-)\n create mode 100755 t/t6038-merge-text-auto.sh\n\ndiff --git a/cache.h b/cache.h\nindex ff4a7c2..5db89f9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1043,6 +1043,7 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);\n extern int convert_to_git(const char *path, const char *src, size_t len,\n                           struct strbuf *dst, enum safe_crlf checksafe);\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 \n /* add */\n /*\ndiff --git a/convert.c b/convert.c\nindex e41a31e..e1561a9 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -693,7 +693,7 @@ static int git_path_check_ident(const char *path, struct git_attr_check *check)\n \treturn !!ATTR_TRUE(value);\n }\n \n-enum action determine_action(enum action text_attr, enum eol eol_attr) {\n+static enum action determine_action(enum action text_attr, enum eol eol_attr) {\n \tif (text_attr == CRLF_BINARY)\n \t\treturn CRLF_BINARY;\n \tif (eol_attr == EOL_LF)\n@@ -773,3 +773,12 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc\n \t}\n \treturn ret | apply_filter(path, src, len, dst, filter);\n }\n+\n+int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst) {\n+\tint ret = convert_to_working_tree(path, src, len, dst);\n+\tif (ret) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n+\t}\n+\treturn ret | convert_to_git(path, src, len, dst, 0);\n+}\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 3764a1a..bdaa580 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -321,6 +321,15 @@ static int git_path_check_merge(const char *path, struct git_attr_check check[2]\n \treturn git_checkattr(path, 2, check);\n }\n \n+static void normalize_file(mmfile_t *mm, const char *path) {\n+\tstruct strbuf strbuf = STRBUF_INIT;\n+\tif (renormalize_buffer(path, mm->ptr, mm->size, &strbuf)) {\n+\t\tfree(mm->ptr);\n+\t\tmm->size = strbuf.len;\n+\t\tmm->ptr = strbuf_detach(&strbuf, NULL);\n+\t}\n+}\n+\n int ll_merge(mmbuffer_t *result_buf,\n \t     const char *path,\n \t     mmfile_t *ancestor, const char *ancestor_label,\n@@ -334,6 +343,9 @@ int ll_merge(mmbuffer_t *result_buf,\n \tconst struct ll_merge_driver *driver;\n \tint virtual_ancestor = flag & 01;\n \n+\tnormalize_file(ancestor, path);\n+\tnormalize_file(ours, path);\n+\tnormalize_file(theirs, path);\n \tif (!git_path_check_merge(path, check)) {\n \t\tll_driver_name = check[0].value;\n \t\tif (check[1].value) {\ndiff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh\nnew file mode 100755\nindex 0000000..44e6003\n--- /dev/null\n+++ b/t/t6038-merge-text-auto.sh\n@@ -0,0 +1,58 @@\n+#!/bin/sh\n+\n+test_description='CRLF merge conflict across text=auto change'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tgit config core.autocrlf false &&\n+\techo first line | append_cr >file &&\n+\tgit add file &&\n+\tgit commit -m \"Initial\" &&\n+\tgit tag initial &&\n+\tgit branch side &&\n+\techo \"* text=auto\" >.gitattributes &&\n+\ttouch file &&\n+\tgit add .gitattributes file &&\n+\tgit commit -m \"normalize file\" &&\n+\techo same line | append_cr >>file &&\n+\tgit add file &&\n+\tgit commit -m \"add line from a\" &&\n+\tgit tag a &&\n+\tgit rm .gitattributes &&\n+\trm file &&\n+\tgit checkout file &&\n+\tgit commit -m \"remove .gitattributes\" &&\n+\tgit tag c &&\n+\tgit checkout side &&\n+\techo same line | append_cr >>file &&\n+\tgit commit -m \"add line from b\" file &&\n+\tgit tag b &&\n+\tgit checkout master\n+'\n+\n+test_expect_success 'Check merging after setting text=auto' '\n+\tgit reset --hard a &&\n+\tgit merge b &&\n+\tcat file | remove_cr >file.temp &&\n+\ttest_cmp file file.temp\n+'\n+\n+test_expect_success 'Check merging addition of text=auto' '\n+\tgit reset --hard b &&\n+\tgit merge a &&\n+\tcat file | remove_cr >file.temp &&\n+\ttest_cmp file file.temp\n+'\n+\n+test_expect_failure 'Test delete/normalize conflict' '\n+\tgit checkout side &&\n+\tgit reset --hard initial &&\n+\tgit rm file &&\n+\tgit commit -m \"remove file\" &&\n+\tgit checkout master &&\n+\tgit reset --hard a^ &&\n+\tgit merge side\n+'\n+\n+test_done\n-- \n1.7.1.575.g383de\n"},{"id":"144203","messageId":"eeb2d5a91ac0e443e74c5866c9c331639b72b7a4.1277408598.git.eyvind.bernhardsen@gmail.com","threadId":"24191","inReplyTo":"cover.1277408598.git.eyvind.bernhardsen@gmail.com","subject":"[PATCH v3 2/3] Try normalizing files to avoid delete/modify conflicts when merging","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind.bernhardsen@gmail.com","sentAt":"2010-06-24T20:44:31Z","receivedAt":"2010-06-24T20:44:31Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"If a file is modified due to normalization on one branch, and deleted on\nanother, a merge of the two branches will result in a delete/modify\nconflict for that file even if it is otherwise unchanged.\n\nTry to avoid the conflict by normalizing and comparing the \"base\" file\nand the modified file when their sha1s differ.  If they compare equal,\nthe file is considered unmodified and is deleted.\n\nSigned-off-by: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>\n---\n merge-recursive.c          |   42 ++++++++++++++++++++++++++++++++++++++++--\n t/t6038-merge-text-auto.sh |    2 +-\n 2 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 206c103..95edc3e 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1056,6 +1056,42 @@ static unsigned char *stage_sha(const unsigned char *sha, unsigned mode)\n \treturn (is_null_sha1(sha) || mode == 0) ? NULL: (unsigned char *)sha;\n }\n \n+static int read_sha1_strbuf(const unsigned char *sha, const char *path,\n+\t\t\t    struct strbuf *dst) {\n+\tvoid *buf;\n+\tenum object_type type;\n+\tunsigned long size;\n+\tbuf = read_sha1_file(sha, &type, &size);\n+\tif (!buf)\n+\t\treturn 0;\n+\tif (type != OBJ_BLOB) {\n+\t\tfree(buf);\n+\t\treturn 0;\n+\t}\n+\tstrbuf_attach(dst, buf, size, size + 1);\n+\treturn 1;\n+}\n+\n+static int normalized_eq(const unsigned char *a_sha,\n+\t\t\t const unsigned char *b_sha,\n+\t\t\t const char *path) {\n+\tstruct strbuf a = STRBUF_INIT;\n+\tstruct strbuf b = STRBUF_INIT;\n+\tint ret = 0;\n+\tif (a_sha && b_sha &&\n+\t    read_sha1_strbuf(a_sha, path, &a) &&\n+\t    read_sha1_strbuf(b_sha, path, &b)) {\n+\t\t/* Both files must be normalized, so we can't use || */\n+\t\tif ((renormalize_buffer(path, a.buf, a.len, &a) |\n+\t\t     renormalize_buffer(path, b.buf, b.len, &b)) &&\n+\t\t    (a.len == b.len))\n+\t\t\tret = memcmp(a.buf, b.buf, a.len) == 0;\n+\t}\n+\tstrbuf_release(&a);\n+\tstrbuf_release(&b);\n+\treturn ret;\n+}\n+\n /* Per entry merge function */\n static int process_entry(struct merge_options *o,\n \t\t\t const char *path, struct stage_data *entry)\n@@ -1075,8 +1111,10 @@ static int process_entry(struct merge_options *o,\n \tif (o_sha && (!a_sha || !b_sha)) {\n \t\t/* Case A: Deleted in one */\n \t\tif ((!a_sha && !b_sha) ||\n-\t\t    (sha_eq(a_sha, o_sha) && !b_sha) ||\n-\t\t    (!a_sha && sha_eq(b_sha, o_sha))) {\n+\t\t    (!b_sha && sha_eq(a_sha, o_sha) ||\n+\t\t     normalized_eq(a_sha, o_sha, path)) ||\n+\t\t    (!a_sha && sha_eq(b_sha, o_sha) ||\n+\t\t     normalized_eq(b_sha, o_sha, path))) {\n \t\t\t/* Deleted in both or deleted in one and\n \t\t\t * unchanged in the other */\n \t\t\tif (a_sha)\ndiff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh\nindex 44e6003..5e45256 100755\n--- a/t/t6038-merge-text-auto.sh\n+++ b/t/t6038-merge-text-auto.sh\n@@ -45,7 +45,7 @@ test_expect_success 'Check merging addition of text=auto' '\n \ttest_cmp file file.temp\n '\n \n-test_expect_failure 'Test delete/normalize conflict' '\n+test_expect_success 'Test delete/normalize conflict' '\n \tgit checkout side &&\n \tgit reset --hard initial &&\n \tgit rm file &&\n-- \n1.7.1.575.g383de\n"},{"id":"144202","messageId":"90f38b5f5c49f9b9f5427a026e51f867a1121982.1277408598.git.eyvind.bernhardsen@gmail.com","threadId":"24191","inReplyTo":"cover.1277408598.git.eyvind.bernhardsen@gmail.com","subject":"[PATCH v3 3/3] Don't expand CRLFs when normalizing text during merge","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind.bernhardsen@gmail.com","sentAt":"2010-06-24T20:44:32Z","receivedAt":"2010-06-24T20:44:32Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"There's no need to expand CRLFs when convert_to_working_tree() is called\nto normalize text for a merge since the text will be converted back\nimmediately.  Improves performance of merges with conflicting line\nendings when core.eol=crlf or core.autocrlf=true.\n\nSigned-off-by: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>\n---\n convert.c |   22 +++++++++++++++-------\n 1 files changed, 15 insertions(+), 7 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex e1561a9..d45d490 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -739,7 +739,9 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \treturn ret | ident_to_git(path, src, len, dst, ident);\n }\n \n-int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\n+static int convert_to_working_tree_internal(const char *path, const char *src,\n+\t\t\t\t\t    size_t len, struct strbuf *dst,\n+\t\t\t\t\t    int normalizing)\n {\n \tstruct git_attr_check check[5];\n \tenum action action = CRLF_GUESS;\n@@ -765,17 +767,23 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\taction = determine_action(action, eol_attr);\n-\tret |= crlf_to_worktree(path, src, len, dst, action);\n-\tif (ret) {\n-\t\tsrc = dst->buf;\n-\t\tlen = dst->len;\n+\tif (!normalizing) {\n+\t\taction = determine_action(action, eol_attr);\n+\t\tret |= crlf_to_worktree(path, src, len, dst, action);\n+\t\tif (ret) {\n+\t\t\tsrc = dst->buf;\n+\t\t\tlen = dst->len;\n+\t\t}\n \t}\n \treturn ret | apply_filter(path, src, len, dst, filter);\n }\n \n+int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst) {\n+\treturn convert_to_working_tree_internal(path, src, len, dst, 0);\n+}\n+\n int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst) {\n-\tint ret = convert_to_working_tree(path, src, len, dst);\n+\tint ret = convert_to_working_tree_internal(path, src, len, dst, 1);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n-- \n1.7.1.575.g383de\n"},{"id":"144224","messageId":"4C244278.10407@viscovery.net","threadId":"24191","inReplyTo":"90f38b5f5c49f9b9f5427a026e51f867a1121982.1277408598.git.eyvind.bernhardsen@gmail.com","subject":"Re: [PATCH v3 3/3] Don't expand CRLFs when normalizing text during merge","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-06-25T05:45:28Z","receivedAt":"2010-06-25T05:45:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 6/24/2010 22:44, schrieb Eyvind Bernhardsen:\n> There's no need to expand CRLFs when convert_to_working_tree() is called\n> to normalize text for a merge since the text will be converted back\n> immediately.  Improves performance of merges with conflicting line\n> endings when core.eol=crlf or core.autocrlf=true.\n\nPardon me, first you make a big deal about normalization for merges, only\nthat you finally omit it? What am I missing?\n\nBTW, most of the new functions you introduced violate the style: they\nshould have the opening brace on the next line.\n\n-- Hannes\n"},{"id":"144227","messageId":"FB02F1B0-9FE7-4B7B-BA30-5A510F83BCE7@gmail.com","threadId":"24191","inReplyTo":"4C244278.10407@viscovery.net","subject":"Re: [PATCH v3 3/3] Don't expand CRLFs when normalizing text during merge","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind.bernhardsen@gmail.com","sentAt":"2010-06-25T07:58:35Z","receivedAt":"2010-06-25T07:58:35Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"On 25. juni 2010, at 07.45, Johannes Sixt wrote:\n\n> Am 6/24/2010 22:44, schrieb Eyvind Bernhardsen:\n>> There's no need to expand CRLFs when convert_to_working_tree() is called\n>> to normalize text for a merge since the text will be converted back\n>> immediately.  Improves performance of merges with conflicting line\n>> endings when core.eol=crlf or core.autocrlf=true.\n> \n> Pardon me, first you make a big deal about normalization for merges, only\n> that you finally omit it? What am I missing?\n\nSorry, I didn't explain that very well.  I noticed that normalize_buffer() does more work when core.eol=crlf than it does when core.eol=lf: _to_working_tree() converts LF to CRLF, and then _to_git() reverses that conversion.  This patch makes normalization act the same way when core.eol=crlf as it does when core.eol=lf.\n\nThat implies a lack of symmetry in the way the crlf_to_git() and crlf_to_worktree() functions are called, but that asymmetry already exists when core.eol=lf since crlf_to_worktree() returns immediately when no output conversion is required.\n\nI considered temporarily setting the \"eol\" and \"auto_crlf\" globals in normalize_buffer(), but messing with global variables felt wrong and this gives the same result (almost: it also disables conversion when a file has text=crlf, but that is a further optimization).\n\n> BTW, most of the new functions you introduced violate the style: they\n> should have the opening brace on the next line.\n\nAh, will fix.  Thanks.\n-- \nEyvind Bernhardsen\n"},{"id":"144228","messageId":"20100625080043.GB4734@pvv.org","threadId":"24191","inReplyTo":"4C244278.10407@viscovery.net","subject":"Re: [PATCH v3 3/3] Don't expand CRLFs when normalizing text during merge","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2010-06-25T08:00:43Z","receivedAt":"2010-06-25T08:00:43Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Fri, Jun 25, 2010 at 07:45:28AM +0200, Johannes Sixt wrote:\n> Am 6/24/2010 22:44, schrieb Eyvind Bernhardsen:\n> > There's no need to expand CRLFs when convert_to_working_tree() is called\n> > to normalize text for a merge since the text will be converted back\n> > immediately.  Improves performance of merges with conflicting line\n> > endings when core.eol=crlf or core.autocrlf=true.\n> \n> Pardon me, first you make a big deal about normalization for merges, only\n> that you finally omit it? What am I missing?\n\nHe calls convert_to_working_tree and then immediately calls\nconvert_to_git again.  convert_to_git will still convert CRLF to LF\nwhere appropriate, so the end result will be the same. There is no\nreason to go through an \"expensive\" conversion of LF->CRLF in\nconvert_to_working_tree first.\n\n- Finn Arne\n"},{"id":"144229","messageId":"4C246622.9090707@viscovery.net","threadId":"24191","inReplyTo":"FB02F1B0-9FE7-4B7B-BA30-5A510F83BCE7@gmail.com","subject":"Re: [PATCH v3 3/3] Don't expand CRLFs when normalizing text during merge","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-06-25T08:17:38Z","receivedAt":"2010-06-25T08:17:38Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 6/25/2010 9:58, schrieb Eyvind Bernhardsen:\n> On 25. juni 2010, at 07.45, Johannes Sixt wrote:\n> \n>> Am 6/24/2010 22:44, schrieb Eyvind Bernhardsen:\n>>> There's no need to expand CRLFs when convert_to_working_tree() is called\n>>> to normalize text for a merge since the text will be converted back\n>>> immediately.  Improves performance of merges with conflicting line\n>>> endings when core.eol=crlf or core.autocrlf=true.\n>>\n>> Pardon me, first you make a big deal about normalization for merges, only\n>> that you finally omit it? What am I missing?\n> \n> Sorry, I didn't explain that very well.  I noticed that normalize_buffer()\n> does more work when core.eol=crlf than it does when core.eol=lf:\n> _to_working_tree() converts LF to CRLF, and then _to_git() reverses that\n> conversion.  This patch makes normalization act the same way when\n> core.eol=crlf as it does when core.eol=lf.\n\nGot it: I missed that you are omitting the conversion only on the \"way\nout\" but not on the \"way back\".\n\nLooking more closely at your patch, I think that you should make this\noptimization only if you can prove that the subsequent apply_filter() is a\nno-op. Otherwise, you may break a smudge filter that expects CRLFs.\n\n-- Hannes\n"},{"id":"144231","messageId":"EE9F4F92-06D5-4B31-BF6F-1163774C112B@gmail.com","threadId":"24191","inReplyTo":"4C246622.9090707@viscovery.net","subject":"Re: [PATCH v3 3/3] Don't expand CRLFs when normalizing text during merge","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind.bernhardsen@gmail.com","sentAt":"2010-06-25T08:58:54Z","receivedAt":"2010-06-25T08:58:54Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"On 25. juni 2010, at 10.17, Johannes Sixt wrote:\n\n> Am 6/25/2010 9:58, schrieb Eyvind Bernhardsen:\n>> Sorry, I didn't explain that very well.  I noticed that normalize_buffer()\n>> does more work when core.eol=crlf than it does when core.eol=lf:\n>> _to_working_tree() converts LF to CRLF, and then _to_git() reverses that\n>> conversion.  This patch makes normalization act the same way when\n>> core.eol=crlf as it does when core.eol=lf.\n> \n> Got it: I missed that you are omitting the conversion only on the \"way\n> out\" but not on the \"way back\".\n> \n> Looking more closely at your patch, I think that you should make this\n> optimization only if you can prove that the subsequent apply_filter() is a\n> no-op. Otherwise, you may break a smudge filter that expects CRLFs.\n\nSuch a smudge filter would break when core.eol=lf, but you're suggesting that someone might be using a picky smudge filter and has set core.eol=crlf to make sure it gets the line endings it expects?\n\nThat's a very good catch.  I'll add a test so that the optimization is only done if filter is NULL.\n-- \nEyvind Bernhardsen\n"}]}