{"thread":{"id":"37019","subject":"[PATCH 0/2] git-merge-file: do not add LF at EOF while applying unrelated change","startedAt":"2014-06-28T19:37:21Z","lastAt":"2014-06-28T19:37:23Z","messageCount":3,"participants":["Max Kirillov"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"245135","messageId":"1403984243-22717-1-git-send-email-max@max630.net","threadId":"37019","inReplyTo":null,"subject":"[PATCH 0/2] git-merge-file: do not add LF at EOF while applying unrelated change","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-06-28T19:37:21Z","receivedAt":"2014-06-28T19:37:21Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Hi.\n\nI have noticed that cherry-pick adds trailing newlines when it is not\nexpected to - the change does not contain its addition. Here is the fix\nfor it.\n\nThe fix is quite debugging-driven, without detailed analysis of how\nexactly this \"add_nl\" parameter works in all cases. But it passes all\ntests. I have added some more where I felt there were not enough.\n\nAlso I have noticed that some tests in the t6023, which related to the\nbehavior contain a mistake which makes them meaningless. I have fixed it\nand marked the tests as expected failure, because they are failing after\nthat. Hopely they will be fixed some day.\n\nMax Kirillov (2):\n  t6023-merge-file.sh: fix and mark as broken invalid tests\n  git-merge-file: do not add LF at EOF while applying unrelated change\n\n t/t6023-merge-file.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++++---\n xdiff/xmerge.c        |  4 +--\n 2 files changed, 71 insertions(+), 5 deletions(-)\n\n-- \n2.0.0.526.g5318336\n"},{"id":"245137","messageId":"1403984243-22717-2-git-send-email-max@max630.net","threadId":"37019","inReplyTo":"1403984243-22717-1-git-send-email-max@max630.net","subject":"[PATCH 1/2] t6023-merge-file.sh: fix and mark as broken invalid tests","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-06-28T19:37:22Z","receivedAt":"2014-06-28T19:37:22Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Tests \"merge without conflict (missing LF at EOF\" and \"merge result\nadded missing LF\" are meaningless - the first one is identical to\n\"merge without conflict\" and the second compares results of those\nidentical tests, which are always same.\n\nThis has been so since their addition in ba1f5f3537. Probably \"new4.txt\"\nwas meant to be used instead of \"new2.txt\". Unfortunately, the current\nmerge-file breaks with new4 - conflict is reported. They also fail at\nthat revision if fixed.\n\nFix the file reference to \"new4.txt\" and mark the tests as failing -\nthey look like legitimate expectations, just not satisfied at time\nbeing.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\n t/t6023-merge-file.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex d9f3439..6da921c 100755\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -77,10 +77,10 @@ test_expect_success \"merge without conflict (--quiet)\" \\\n \t\"git merge-file --quiet test.txt orig.txt new2.txt\"\n \n cp new1.txt test2.txt\n-test_expect_success \"merge without conflict (missing LF at EOF)\" \\\n-\t\"git merge-file test2.txt orig.txt new2.txt\"\n+test_expect_failure \"merge without conflict (missing LF at EOF)\" \\\n+\t\"git merge-file test2.txt orig.txt new4.txt\"\n \n-test_expect_success \"merge result added missing LF\" \\\n+test_expect_failure \"merge result added missing LF\" \\\n \t\"test_cmp test.txt test2.txt\"\n \n cp test.txt backup.txt\n-- \n2.0.0.526.g5318336\n"},{"id":"245136","messageId":"1403984243-22717-3-git-send-email-max@max630.net","threadId":"37019","inReplyTo":"1403984243-22717-1-git-send-email-max@max630.net","subject":"[PATCH 2/2] git-merge-file: do not add LF at EOF while applying unrelated change","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-06-28T19:37:23Z","receivedAt":"2014-06-28T19:37:23Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"If 'current-file' does not contain LF at EOF, and change between\n'base-file' and 'other-file' does not change any line close to EOF, the\n3-way merge should not add LF to EOF.  This is what 'diff3 -m' does, and\nseems to be a reasonable expectation.\n\nThe change which introduced the behavior is cd1d61c44f and was about\nunion merge. The union merges are tested in the t6026-merge-attr, and\nmerge-file in general are tested in t6023-merge-file. Add tests\n\"merge conflicting with --..\" there, to verify that the fix does not\nbreak anything.\n\nAlso add tests \"merge without conflict (missing LF at EOF, away from\nchange in the other file)\" and \"merge does not add LF away of change\",\nto demonstrate the changed behavior.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\n t/t6023-merge-file.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++\n xdiff/xmerge.c        |  4 ++--\n 2 files changed, 68 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex 6da921c..21d1b85 100755\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -83,6 +83,23 @@ test_expect_failure \"merge without conflict (missing LF at EOF)\" \\\n test_expect_failure \"merge result added missing LF\" \\\n \t\"test_cmp test.txt test2.txt\"\n \n+cp new4.txt test3.txt\n+test_expect_success \"merge without conflict (missing LF at EOF, away from change in the other file)\" \\\n+\t\"git merge-file --quiet test3.txt new2.txt new3.txt\"\n+\n+cat > expect.txt << EOF\n+DOMINUS regit me,\n+et nihil mihi deerit.\n+In loco pascuae ibi me collocavit,\n+super aquam refectionis educavit me;\n+animam meam convertit,\n+deduxit me super semitas jusitiae,\n+EOF\n+printf \"propter nomen suum.\" >> expect.txt\n+\n+test_expect_success \"merge does not add LF away of change\" \\\n+\t\"test_cmp test3.txt expect.txt\"\n+\n cp test.txt backup.txt\n test_expect_success \"merge with conflicts\" \\\n \t\"test_must_fail git merge-file test.txt orig.txt new3.txt\"\n@@ -107,6 +124,55 @@ EOF\n test_expect_success \"expected conflict markers\" \"test_cmp test.txt expect.txt\"\n \n cp backup.txt test.txt\n+\n+cat > expect.txt << EOF\n+Dominus regit me, et nihil mihi deerit.\n+In loco pascuae ibi me collocavit,\n+super aquam refectionis educavit me;\n+animam meam convertit,\n+deduxit me super semitas jusitiae,\n+propter nomen suum.\n+Nam et si ambulavero in medio umbrae mortis,\n+non timebo mala, quoniam tu mecum es:\n+virga tua et baculus tuus ipsa me consolata sunt.\n+EOF\n+test_expect_success \"merge conflicting with --ours\" \\\n+\t\"git merge-file --ours test.txt orig.txt new3.txt && test_cmp test.txt expect.txt\"\n+cp backup.txt test.txt\n+\n+cat > expect.txt << EOF\n+DOMINUS regit me,\n+et nihil mihi deerit.\n+In loco pascuae ibi me collocavit,\n+super aquam refectionis educavit me;\n+animam meam convertit,\n+deduxit me super semitas jusitiae,\n+propter nomen suum.\n+Nam et si ambulavero in medio umbrae mortis,\n+non timebo mala, quoniam tu mecum es:\n+virga tua et baculus tuus ipsa me consolata sunt.\n+EOF\n+test_expect_success \"merge conflicting with --theirs\" \\\n+\t\"git merge-file --theirs test.txt orig.txt new3.txt && test_cmp test.txt expect.txt\"\n+cp backup.txt test.txt\n+\n+cat > expect.txt << EOF\n+Dominus regit me, et nihil mihi deerit.\n+DOMINUS regit me,\n+et nihil mihi deerit.\n+In loco pascuae ibi me collocavit,\n+super aquam refectionis educavit me;\n+animam meam convertit,\n+deduxit me super semitas jusitiae,\n+propter nomen suum.\n+Nam et si ambulavero in medio umbrae mortis,\n+non timebo mala, quoniam tu mecum es:\n+virga tua et baculus tuus ipsa me consolata sunt.\n+EOF\n+test_expect_success \"merge conflicting with --union\" \\\n+\t\"git merge-file --union test.txt orig.txt new3.txt && test_cmp test.txt expect.txt\"\n+cp backup.txt test.txt\n+\n test_expect_success \"merge with conflicts, using -L\" \\\n \t\"test_must_fail git merge-file -L 1 -L 2 test.txt orig.txt new3.txt\"\n \ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 9e13b25..5f7a95a 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -245,11 +245,11 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,\n \t\t\t\t\t      dest ? dest + size : NULL);\n \t\t\t/* Postimage from side #1 */\n \t\t\tif (m->mode & 1)\n-\t\t\t\tsize += xdl_recs_copy(xe1, m->i1, m->chg1, 1,\n+\t\t\t\tsize += xdl_recs_copy(xe1, m->i1, m->chg1, 0,\n \t\t\t\t\t\t      dest ? dest + size : NULL);\n \t\t\t/* Postimage from side #2 */\n \t\t\tif (m->mode & 2)\n-\t\t\t\tsize += xdl_recs_copy(xe2, m->i2, m->chg2, 1,\n+\t\t\t\tsize += xdl_recs_copy(xe2, m->i2, m->chg2, 0,\n \t\t\t\t\t\t      dest ? dest + size : NULL);\n \t\t} else\n \t\t\tcontinue;\n-- \n2.0.0.526.g5318336\n"}]}