{"thread":{"id":"19490","subject":"Slightly obscure merge-file inconsistency","startedAt":"2009-05-25T00:21:12Z","lastAt":"2009-05-25T07:52:05Z","messageCount":5,"participants":["Charles Bailey","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"114615","messageId":"1243210874-18451-1-git-send-email-charles@hashpling.org","threadId":"19490","inReplyTo":null,"subject":"Slightly obscure merge-file inconsistency","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-05-25T00:21:12Z","receivedAt":"2009-05-25T00:21:12Z","isPatch":false,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"This was spotted by someone on stackoverflow.com.\n\nIt seems that git merge-file will wipe the merged file in the\ndegenerate case when all the inputs have identical contents. As\nfar as I can tell this is a bug, albeit one which will never be\ntriggered in practice because merge-file won't ever be called on\nfiles that are identical in base, local and remote versions.\n\nI think that the problem is just an overzealous null check in\nxdl_merge. Certainly with the fix, the test suite still passes but\nthere may be some use cases of xdl_merge that I'm not aware of\nthat rely on the current behaviour.\n\nhttp://stackoverflow.com/questions/903966/\n"},{"id":"114617","messageId":"1243210874-18451-2-git-send-email-charles@hashpling.org","threadId":"19490","inReplyTo":"1243210874-18451-1-git-send-email-charles@hashpling.org","subject":"[PATCH 1/2] merge-file fails to output anything for a degenerate merge","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-05-25T00:21:13Z","receivedAt":"2009-05-25T00:21:13Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"In the case that merge-file is passed three files with identical\ncontents it wipes the contents of the output file instead of\nleaving it unchanged.\n\nAlthought merge-file is porcelain and this will never happen in\nnormal usage, it is still wrong.\n\nSigned-off-by: Charles Bailey \n---\n t/t6023-merge-file.sh | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex f8942bc..44141af 100755\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -54,6 +54,11 @@ deduxit me super semitas jusitiae,\n EOF\n printf \"propter nomen suum.\" >> new4.txt\n \n+cp orig.txt test.txt\n+test_expect_success \"merge with no changes\" \\\n+\t\"git merge-file test.txt orig.txt orig.txt &&\n+\t test_cmp test.txt orig.txt\"\n+\n cp new1.txt test.txt\n test_expect_success \"merge without conflict\" \\\n \t\"git merge-file test.txt orig.txt new2.txt\"\n-- \n1.6.3.1.72.gbd1ec\n"},{"id":"114616","messageId":"1243210874-18451-3-git-send-email-charles@hashpling.org","threadId":"19490","inReplyTo":"1243210874-18451-2-git-send-email-charles@hashpling.org","subject":"[PATCH 2/2] Change xdl_merge to generate output even for null merges","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-05-25T00:21:14Z","receivedAt":"2009-05-25T00:21:14Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"xdl_merge used to have a check to ensure that there was at least\nsome change in one or other side being merged but this suppressed\noutput for the degenerate case when base, local and remote\ncontents were all identical.\n\nRemoving this check enables correct output in the degenerate case\nand xdl_free_script handles freeing NULL scripts so there is no\nneed to have the check for these calls.\n\nSigned-off-by: Charles Bailey \n---\n xdiff/xmerge.c | 32 ++++++++++++++++----------------\n 1 files changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex d9737f0..9cdc3ac 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -563,23 +563,23 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,\n \t\treturn -1;\n \t}\n \tstatus = 0;\n-\tif (xscr1 || xscr2) {\n-\t\tif (!xscr1) {\n-\t\t\tresult->ptr = xdl_malloc(mf2->size);\n-\t\t\tmemcpy(result->ptr, mf2->ptr, mf2->size);\n-\t\t\tresult->size = mf2->size;\n-\t\t} else if (!xscr2) {\n-\t\t\tresult->ptr = xdl_malloc(mf1->size);\n-\t\t\tmemcpy(result->ptr, mf1->ptr, mf1->size);\n-\t\t\tresult->size = mf1->size;\n-\t\t} else {\n-\t\t\tstatus = xdl_do_merge(&xe1, xscr1, name1,\n-\t\t\t\t\t &xe2, xscr2, name2,\n-\t\t\t\t\t flags, xpp, result);\n-\t\t}\n-\t\txdl_free_script(xscr1);\n-\t\txdl_free_script(xscr2);\n+\n+\tif (!xscr1) {\n+\t\tresult->ptr = xdl_malloc(mf2->size);\n+\t\tmemcpy(result->ptr, mf2->ptr, mf2->size);\n+\t\tresult->size = mf2->size;\n+\t} else if (!xscr2) {\n+\t\tresult->ptr = xdl_malloc(mf1->size);\n+\t\tmemcpy(result->ptr, mf1->ptr, mf1->size);\n+\t\tresult->size = mf1->size;\n+\t} else {\n+\t\tstatus = xdl_do_merge(&xe1, xscr1, name1,\n+\t\t\t\t\t &xe2, xscr2, name2,\n+\t\t\t\t\t flags, xpp, result);\n \t}\n+\txdl_free_script(xscr1);\n+\txdl_free_script(xscr2);\n+\n \txdl_free_env(&xe1);\n \txdl_free_env(&xe2);\n \n-- \n1.6.3.1.72.gbd1ec\n"},{"id":"114633","messageId":"alpine.DEB.1.00.0905250948420.26154@pacific.mpi-cbg.de","threadId":"19490","inReplyTo":"1243210874-18451-3-git-send-email-charles@hashpling.org","subject":"Re: [PATCH 2/2] Change xdl_merge to generate output even for null merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-25T07:51:05Z","receivedAt":"2009-05-25T07:51:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 May 2009, Charles Bailey wrote:\n\n> xdl_merge used to have a check to ensure that there was at least\n> some change in one or other side being merged but this suppressed\n> output for the degenerate case when base, local and remote\n> contents were all identical.\n> \n> Removing this check enables correct output in the degenerate case\n> and xdl_free_script handles freeing NULL scripts so there is no\n> need to have the check for these calls.\n> \n> Signed-off-by: Charles Bailey \n\nMaybe mention here that it just so happens that when there were no changes \nin base->local, it does not matter what changes (_if any_) were made to \nbase->remote, we should output the remote version.\n\nTook me more than 2 seconds to find that out, so let's spare the \noccasional reader this time (2 seconds is my attention span).\n\nYour fix is correct.\n\nCiao,\nDscho\n"},{"id":"114634","messageId":"alpine.DEB.1.00.0905250951120.26154@pacific.mpi-cbg.de","threadId":"19490","inReplyTo":"1243210874-18451-2-git-send-email-charles@hashpling.org","subject":"Re: [PATCH 1/2] merge-file fails to output anything for a degenerate merge","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-25T07:52:05Z","receivedAt":"2009-05-25T07:52:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 May 2009, Charles Bailey wrote:\n\n> In the case that merge-file is passed three files with identical\n> contents it wipes the contents of the output file instead of\n> leaving it unchanged.\n> \n> Althought merge-file is porcelain and this will never happen in\n> normal usage, it is still wrong.\n> \n> Signed-off-by: Charles Bailey \n\nYour patch is good, but I'd like to have it mention in the commit subject \nthat this is not a fix, but a patch to the test suite. A prefix \"t6023:\" \nwould suffice, methinks.\n\nThanks,\nDscho\n"}]}