{"thread":{"id":"17284","subject":"git diff, git mergetool and CRLF conversion","startedAt":"2009-01-21T16:55:34Z","lastAt":"2009-01-27T13:58:21Z","messageCount":12,"participants":["Hannu Koivisto","Charles Bailey","Theodore Tso","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"101405","messageId":"83k58ofvjt.fsf@kalahari.s2.org","threadId":"17284","inReplyTo":null,"subject":"git diff, git mergetool and CRLF conversion","fromName":"Hannu Koivisto","fromEmail":"azure@iki.fi","sentAt":"2009-01-21T16:55:34Z","receivedAt":"2009-01-21T16:55:34Z","isPatch":false,"sender":{"key":"azure@iki.fi","avatar":null},"body":"Hi,\n\nSuppose I have core.autocrlf set to true and and due to that a\nversion controlled file in a working tree with CRLF line endings.\nIf I modify such a file and then say \"git diff\", I get a patch with\nLF line endings.\n\nAlso, if get a merge conflict with a file to which CRLF conversion\nis applied and run e.g. \"git mergetool -t emerge\", the temporary\nfiles representing stage2 and stage3 versions seem to have LF line\nendings.\n\nIs this intended behaviour?  I'm using 1.6.1 on Cygwin.\n\n-- \nHannu\n"},{"id":"101408","messageId":"20090121172351.GB21727@hashpling.org","threadId":"17284","inReplyTo":"83k58ofvjt.fsf@kalahari.s2.org","subject":"Re: git diff, git mergetool and CRLF conversion","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-01-21T17:23:51Z","receivedAt":"2009-01-21T17:23:51Z","isPatch":false,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Wed, Jan 21, 2009 at 06:55:34PM +0200, Hannu Koivisto wrote:\n> Hi,\n> \n> Suppose I have core.autocrlf set to true and and due to that a\n> version controlled file in a working tree with CRLF line endings.\n> If I modify such a file and then say \"git diff\", I get a patch with\n> LF line endings.\n> \n> Also, if get a merge conflict with a file to which CRLF conversion\n> is applied and run e.g. \"git mergetool -t emerge\", the temporary\n> files representing stage2 and stage3 versions seem to have LF line\n> endings.\n> \n> Is this intended behaviour?  I'm using 1.6.1 on Cygwin.\n\nSpeaking for mergetool, I believe that it's simply because mergetool\nuses git cat-file which just outputs the raw contents of a blob and\ndoesn't do any line ending conversion.\n\nIMHO, I think that it should probably perform the 'convert to working\ntree format' change when preparing the temporary files. I'm not sure\nhow best to do that, but perhaps it should be using git checkout-index\nwith the --temp option instead of cat-file.\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"101463","messageId":"20090121210348.GD9088@mit.edu","threadId":"17284","inReplyTo":"20090121172351.GB21727@hashpling.org","subject":"Re: git diff, git mergetool and CRLF conversion","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2009-01-21T21:03:48Z","receivedAt":"2009-01-21T21:03:48Z","isPatch":false,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"On Wed, Jan 21, 2009 at 05:23:51PM +0000, Charles Bailey wrote:\n> > Is this intended behaviour?  I'm using 1.6.1 on Cygwin.\n> \n> Speaking for mergetool, I believe that it's simply because mergetool\n> uses git cat-file which just outputs the raw contents of a blob and\n> doesn't do any line ending conversion.\n> \n> IMHO, I think that it should probably perform the 'convert to working\n> tree format' change when preparing the temporary files. I'm not sure\n> how best to do that, but perhaps it should be using git checkout-index\n> with the --temp option instead of cat-file.\n\nYes, I agree, that would probably be better.\n\n\t\t\t\t\t\t- Ted\n"},{"id":"101474","messageId":"1232578668-2203-1-git-send-email-charles@hashpling.org","threadId":"17284","inReplyTo":"20090121210348.GD9088@mit.edu","subject":"[PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-01-21T22:57:48Z","receivedAt":"2009-01-21T22:57:48Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"Previously, git mergetool used cat-file which does not perform git to\nworktree conversion. This changes mergetool to use git checkout-index\ninstead which means that the temporary files used for mergetool use the\ncorrect line endings for the platform.\n\nSigned-off-by: Charles Bailey <charles@hashpling.org>\n---\n git-mergetool.sh     |   14 +++++++++++---\n t/t7610-mergetool.sh |   15 +++++++++++++--\n 2 files changed, 24 insertions(+), 5 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 00e1337..a4855d9 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -127,6 +127,14 @@ check_unchanged () {\n     fi\n }\n \n+checkout_staged_file () {\n+    tmpfile=$(expr \"$(git checkout-index --temp --stage=\"$1\" \"$2\")\" : '\\([^\t]*\\)\t')\n+\n+    if test $? -eq 0 -a -n \"$tmpfile\" ; then\n+\tmv -- \"$tmpfile\" \"$3\"\n+    fi\n+}\n+\n merge_file () {\n     MERGED=\"$1\"\n \n@@ -153,9 +161,9 @@ merge_file () {\n     local_mode=`git ls-files -u -- \"$MERGED\" | awk '{if ($3==2) print $1;}'`\n     remote_mode=`git ls-files -u -- \"$MERGED\" | awk '{if ($3==3) print $1;}'`\n \n-    base_present   && git cat-file blob \":1:$prefix$MERGED\" >\"$BASE\" 2>/dev/null\n-    local_present  && git cat-file blob \":2:$prefix$MERGED\" >\"$LOCAL\" 2>/dev/null\n-    remote_present && git cat-file blob \":3:$prefix$MERGED\" >\"$REMOTE\" 2>/dev/null\n+    base_present   && checkout_staged_file 1 \"$prefix$MERGED\" \"$BASE\"\n+    local_present  && checkout_staged_file 2 \"$prefix$MERGED\" \"$LOCAL\"\n+    remote_present && checkout_staged_file 3 \"$prefix$MERGED\" \"$REMOTE\"\n \n     if test -z \"$local_mode\" -o -z \"$remote_mode\"; then\n \techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 09fa5f1..edb6a57 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -34,13 +34,24 @@ test_expect_success 'custom mergetool' '\n     git config merge.tool mytool &&\n     git config mergetool.mytool.cmd \"cat \\\"\\$REMOTE\\\" >\\\"\\$MERGED\\\"\" &&\n     git config mergetool.mytool.trustExitCode true &&\n-\tgit checkout branch1 &&\n+    git checkout branch1 &&\n     test_must_fail git merge master >/dev/null 2>&1 &&\n     ( yes \"\" | git mergetool file1>/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool file2>/dev/null 2>&1 ) &&\n     test \"$(cat file1)\" = \"master updated\" &&\n     test \"$(cat file2)\" = \"master new\" &&\n-\tgit commit -m \"branch1 resolved with mergetool\"\n+    git commit -m \"branch1 resolved with mergetool\"\n+'\n+\n+test_expect_success 'mergetool crlf' '\n+    git config core.autocrlf true &&\n+    git reset --hard HEAD^\n+    test_must_fail git merge master >/dev/null 2>&1 &&\n+    ( yes \"\" | git mergetool file1>/dev/null 2>&1 ) &&\n+    ( yes \"\" | git mergetool file2>/dev/null 2>&1 ) &&\n+    test \"$(printf x | cat file1 -)\" = \"$(printf \"master updated\\r\\nx\")\" &&\n+    test \"$(printf x | cat file2 -)\" = \"$(printf \"master new\\r\\nx\")\" &&\n+    git commit -m \"branch1 resolved with mergetool - autocrlf\"\n '\n \n test_done\n-- \n1.6.1.235.gc9d403\n"},{"id":"101667","messageId":"7v1vuuvt11.fsf@gitster.siamese.dyndns.org","threadId":"17284","inReplyTo":"1232578668-2203-1-git-send-email-charles@hashpling.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-23T17:20:10Z","receivedAt":"2009-01-23T17:20:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charles Bailey <charles@hashpling.org> writes:\n\n> Previously, git mergetool used cat-file which does not perform git to\n> worktree conversion. This changes mergetool to use git checkout-index\n> instead which means that the temporary files used for mergetool use the\n> correct line endings for the platform.\n>\n> Signed-off-by: Charles Bailey <charles@hashpling.org>\n\nSounds like the right thing to do and from a cursory review it looks Ok to\nme.\n\nBut I do not use mergetool myself, so an Ack from Ted and a Thanks from\nwhoever reported the breakage would be encouraging ;-).\n\n> +checkout_staged_file () {\n> +    tmpfile=$(expr \"$(git checkout-index --temp --stage=\"$1\" \"$2\")\" : '\\([^\t]*\\)\t')\n> +\n> +    if test $? -eq 0 -a -n \"$tmpfile\" ; then\n> +\tmv -- \"$tmpfile\" \"$3\"\n\nThe original redirects into the final destination but this moves.  This\nwill lose the perm bits of the original and obey the perm bits\ncheckout-index gives you.  It will also behave differently when the path\nis a symlink.  These two differences _may_ well be improvements and/or\nbugfixes, but if that is the case please describe them as such.\n"},{"id":"101674","messageId":"20090123181800.GA20177@hashpling.org","threadId":"17284","inReplyTo":"7v1vuuvt11.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-01-23T18:18:00Z","receivedAt":"2009-01-23T18:18:00Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Fri, Jan 23, 2009 at 09:20:10AM -0800, Junio C Hamano wrote:\n> Charles Bailey <charles@hashpling.org> writes:\n> \n> > Previously, git mergetool used cat-file which does not perform git to\n> > worktree conversion. This changes mergetool to use git checkout-index\n> > instead which means that the temporary files used for mergetool use the\n> > correct line endings for the platform.\n> >\n> > Signed-off-by: Charles Bailey <charles@hashpling.org>\n> \n> Sounds like the right thing to do and from a cursory review it looks Ok to\n> me.\n> \n> But I do not use mergetool myself, so an Ack from Ted and a Thanks from\n> whoever reported the breakage would be encouraging ;-).\n\nYes, please!\n\nI had wondered why I hadn't really noticed about this 'issue' before\nas I've used git mergetool on windows with autocrlf set to true quite\na bit. I think that if your mergetool handles LF endings it doesn't\nreally matter as it's only the temporary files that are affected and\nif the mergetool generates LF output files in response to LF input\nfiles then this is resolved to the correct format at the time it is\nadded to the index in any case.\n\n> > +checkout_staged_file () {\n> > +    tmpfile=$(expr \"$(git checkout-index --temp --stage=\"$1\" \"$2\")\" : '\\([^\t]*\\)\t')\n> > +\n> > +    if test $? -eq 0 -a -n \"$tmpfile\" ; then\n> > +\tmv -- \"$tmpfile\" \"$3\"\n> \n> The original redirects into the final destination but this moves.  This\n> will lose the perm bits of the original and obey the perm bits\n> checkout-index gives you.  It will also behave differently when the path\n> is a symlink.  These two differences _may_ well be improvements and/or\n> bugfixes, but if that is the case please describe them as such.\n\nI hadn't actually thought about the perms thing that much but now that\nI do...\n\nThis code, and the code that it replaces, only affects the temporary\nfiles that form the basis for the merge. The result/destination file\nis as generated by the merge (or rebase).\n\nThe redirect version (as is) will not set permissions from the index -\neffectively losing information, the new version should (I think - I'm\nnot an expert in checkout-index) get the 'correct' repository\npermissions. I would say that this is, if anything, an improvement.\nThe ultimate effect really depends on the mergetool and whether the\nsource file permissions affect the permissions that it sets on the\ntarget file. In the vast majority of cases I would think that it\ndoesn't have any effect.\n\nNote that symlinks in the repository are not resolved by this code\npath so they aren't affected.\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"101995","messageId":"83skn6doxm.fsf@kalahari.s2.org","threadId":"17284","inReplyTo":"7v1vuuvt11.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Hannu Koivisto","fromEmail":"azure@iki.fi","sentAt":"2009-01-26T16:15:01Z","receivedAt":"2009-01-26T16:15:01Z","isPatch":true,"sender":{"key":"azure@iki.fi","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Charles Bailey <charles@hashpling.org> writes:\n>\n>> Previously, git mergetool used cat-file which does not perform git to\n>> worktree conversion. This changes mergetool to use git checkout-index\n>> instead which means that the temporary files used for mergetool use the\n>> correct line endings for the platform.\n>>\n>> Signed-off-by: Charles Bailey <charles@hashpling.org>\n>\n> Sounds like the right thing to do and from a cursory review it looks Ok to\n> me.\n>\n> But I do not use mergetool myself, so an Ack from Ted and a Thanks from\n> whoever reported the breakage would be encouraging ;-).\n\nI apologize for not being able to test this earlier and I'm\ncertainly thankful for the patch, although admittedly I reported\nthe issue mainly to help improve git, not because mergetool is part\nof my normal use of git (that may change when I find time to study it\nand see if I can easily add ediff support to it in addition to\nemerge).\n\nNow that I tried the patch, I observed that while the stage2 and\nstage3 temporary files have CRLF line endings, the merge result\nbuffer/file has LF line endings.  I'm again using Cygwin git,\nmergetool -t emerge and native Windows Emacs.  So when I quit the\nmergetool, I get\n\n...\nHit return to start merge resolution tool (emerge):\nwarning: LF will be replaced by CRLF in kala.txt\nwarning: LF will be replaced by CRLF in kala.txt\n\nand indeed hexdump proves that the file in my worktree now has LF\nline endings even though it had CRLF line endings before invoking\nmergetool.\n\nI wonder why I didn't notice this the first time.  I can certainly\nreproduce it now without Charles' patch as well so I suppose this\nis a separate issue and the patch does what it is supposed to do.\n\n-- \nHannu\n"},{"id":"102001","messageId":"20090126163114.GD32604@hashpling.org","threadId":"17284","inReplyTo":"83skn6doxm.fsf@kalahari.s2.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-01-26T16:31:14Z","receivedAt":"2009-01-26T16:31:14Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Mon, Jan 26, 2009 at 06:15:01PM +0200, Hannu Koivisto wrote:\n>\n> Now that I tried the patch, I observed that while the stage2 and\n> stage3 temporary files have CRLF line endings, the merge result\n> buffer/file has LF line endings.  I'm again using Cygwin git,\n> mergetool -t emerge and native Windows Emacs.  So when I quit the\n> mergetool, I get\n> \n> ...\n> Hit return to start merge resolution tool (emerge):\n> warning: LF will be replaced by CRLF in kala.txt\n> warning: LF will be replaced by CRLF in kala.txt\n> \n> and indeed hexdump proves that the file in my worktree now has LF\n> line endings even though it had CRLF line endings before invoking\n> mergetool.\n> \n> I wonder why I didn't notice this the first time.  I can certainly\n> reproduce it now without Charles' patch as well so I suppose this\n> is a separate issue and the patch does what it is supposed to do.\n\nmergetool doesn't touch the destination file, it just asks the merge\ntool to overwrite it.\n\nI suspect that the LF endings in the file is due to the fact that in\nbuiltin-merge-file.c, the file is opened (fopen) in binary mode\n(\"wb\"), but xdl_merge terminates all lines with a raw '\\n'.\n\nThe obvious fix would be to change fopen in builtin-file-merge.c to\nuse \"w\" instead, but this doesn't work in a number of scenarios. In\nparticular, it is wrong for repositories on windows with core.autocrlf\nset to false, and would not fix non-windows repositories with\ncore.autocrlf set to true.\n\nCurrently, I've no idea as to what the solution should be.\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"102060","messageId":"7v7i4h4v19.fsf@gitster.siamese.dyndns.org","threadId":"17284","inReplyTo":"20090126163114.GD32604@hashpling.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-26T21:28:02Z","receivedAt":"2009-01-26T21:28:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charles Bailey <charles@hashpling.org> writes:\n\n> I suspect that the LF endings in the file is due to the fact that in\n> builtin-merge-file.c, the file is opened (fopen) in binary mode\n> (\"wb\"), but xdl_merge terminates all lines with a raw '\\n'.\n>\n> The obvious fix would be to change fopen in builtin-file-merge.c to\n> use \"w\" instead, but this doesn't work in a number of scenarios. In\n> particular, it is wrong for repositories on windows with core.autocrlf\n> set to false, and would not fix non-windows repositories with\n> core.autocrlf set to true.\n>\n> Currently, I've no idea as to what the solution should be.\n\n\"git file-merge\" is designed to be a replacement for stock RCS merge, and\nunfortunately it does not call convert_to_working_tree(), nor has any way\nto know for which path it should take the attributes to apply to affect\nwhat convert_to_working_tree() should do even if it were to call it.\n\nI think we would need a new option to the command that says \"pretend this\nis about merging this path, and use the gitattributes specified for it\nwhen writing out the result.\"\n"},{"id":"102064","messageId":"7vskn53em1.fsf@gitster.siamese.dyndns.org","threadId":"17284","inReplyTo":"7v7i4h4v19.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-26T22:08:06Z","receivedAt":"2009-01-26T22:08:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Charles Bailey <charles@hashpling.org> writes:\n>\n>> I suspect that the LF endings in the file is due to the fact that in\n>> builtin-merge-file.c, the file is opened (fopen) in binary mode\n>> (\"wb\"), but xdl_merge terminates all lines with a raw '\\n'.\n>>\n>> The obvious fix would be to change fopen in builtin-file-merge.c to\n>> use \"w\" instead, but this doesn't work in a number of scenarios. In\n>> particular, it is wrong for repositories on windows with core.autocrlf\n>> set to false, and would not fix non-windows repositories with\n>> core.autocrlf set to true.\n>>\n>> Currently, I've no idea as to what the solution should be.\n>\n> \"git file-merge\" is designed to be a replacement for stock RCS merge, and\n> unfortunately it does not call convert_to_working_tree(), nor has any way\n> to know for which path it should take the attributes to apply to affect\n> what convert_to_working_tree() should do even if it were to call it.\n>\n> I think we would need a new option to the command that says \"pretend this\n> is about merging this path, and use the gitattributes specified for it\n> when writing out the result.\"\n\nPerhaps something along this line to teach\n\n    $ git merge-file --attribute-path=frotz.c file1 orig_file file2\n\nto merge what happened since orig_file to file2 into file1, and deposit\nthe result after converting it appropriately for path \"frotz.c\" obeying\ncore.autocrlf and gitattribute rules.\n\nI see rerere.c::merge() has the exact same issue, but its breakage is half\nhidden by its use of fopen(path, \"w\").  It should explicitly use\nconvert_to_working_tree() like this patch does, and write the results out\nin binary mode.\n\n builtin-merge-file.c |   18 +++++++++++++++++-\n 1 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git i/builtin-merge-file.c w/builtin-merge-file.c\nindex 96edb97..61d1092 100644\n--- i/builtin-merge-file.c\n+++ w/builtin-merge-file.c\n@@ -5,7 +5,7 @@\n #include \"parse-options.h\"\n \n static const char *const merge_file_usage[] = {\n-\t\"git merge-file [options] [-L name1 [-L orig [-L name2]]] file1 orig_file file2\",\n+\t\"git merge-file [options] [-L name1 [-L orig [-L name2]]] [--attribute-path path] file1 orig_file file2\",\n \tNULL\n };\n \n@@ -30,10 +30,13 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tint merge_level = XDL_MERGE_ZEALOUS_ALNUM;\n \tint merge_style = 0, quiet = 0;\n \tint nongit;\n+\tchar *attribute_path = NULL;\n \n \tstruct option options[] = {\n \t\tOPT_BOOLEAN('p', \"stdout\", &to_stdout, \"send results to standard output\"),\n \t\tOPT_SET_INT(0, \"diff3\", &merge_style, \"use a diff3 based merge\", XDL_MERGE_DIFF3),\n+\t\tOPT_STRING('a', \"attribute-path\", &attribute_path, \"path\",\n+\t\t\t   \"apply work-tree conversion for the path\"),\n \t\tOPT__QUIET(&quiet),\n \t\tOPT_CALLBACK('L', NULL, names, \"name\",\n \t\t\t     \"set labels for file1/orig_file/file2\", &label_cb),\n@@ -73,6 +76,19 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfs[i].ptr);\n \n+\tif (ret >= 0 && attribute_path) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tret = convert_to_working_tree(attribute_path,\n+\t\t\t\t\t      result.ptr, result.size,\n+\t\t\t\t\t      &buf);\n+\t\tfree(result.ptr);\n+\t\tif (!ret) {\n+\t\t\tsize_t len;\n+\t\t\tresult.ptr = strbuf_detach(&buf, &len);\n+\t\t\tresult.size = len;\n+\t\t}\n+\t}\n+\n \tif (ret >= 0) {\n \t\tconst char *filename = argv[0];\n \t\tFILE *f = to_stdout ? stdout : fopen(filename, \"wb\");\n"},{"id":"102067","messageId":"7vocxt3bsc.fsf@gitster.siamese.dyndns.org","threadId":"17284","inReplyTo":"7vskn53em1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-26T23:09:07Z","receivedAt":"2009-01-26T23:09:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Perhaps something along this line to teach\n>\n>     $ git merge-file --attribute-path=frotz.c file1 orig_file file2\n>\n> to merge what happened since orig_file to file2 into file1, and deposit\n> the result after converting it appropriately for path \"frotz.c\" obeying\n> core.autocrlf and gitattribute rules.\n>\n> I see rerere.c::merge() has the exact same issue, but its breakage is half\n> hidden by its use of fopen(path, \"w\").  It should explicitly use\n> convert_to_working_tree() like this patch does, and write the results out\n> in binary mode.\n\nSecond try.  I forgot how convert_* worked X-<.\n\n builtin-merge-file.c |   20 +++++++++++++++++++-\n 1 files changed, 19 insertions(+), 1 deletions(-)\n\ndiff --git i/builtin-merge-file.c w/builtin-merge-file.c\nindex 96edb97..edee815 100644\n--- i/builtin-merge-file.c\n+++ w/builtin-merge-file.c\n@@ -5,7 +5,7 @@\n #include \"parse-options.h\"\n \n static const char *const merge_file_usage[] = {\n-\t\"git merge-file [options] [-L name1 [-L orig [-L name2]]] file1 orig_file file2\",\n+\t\"git merge-file [options] [-L name1 [-L orig [-L name2]]] [--attribute-path path] file1 orig_file file2\",\n \tNULL\n };\n \n@@ -30,10 +30,13 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tint merge_level = XDL_MERGE_ZEALOUS_ALNUM;\n \tint merge_style = 0, quiet = 0;\n \tint nongit;\n+\tchar *attribute_path = NULL;\n \n \tstruct option options[] = {\n \t\tOPT_BOOLEAN('p', \"stdout\", &to_stdout, \"send results to standard output\"),\n \t\tOPT_SET_INT(0, \"diff3\", &merge_style, \"use a diff3 based merge\", XDL_MERGE_DIFF3),\n+\t\tOPT_STRING('a', \"attribute-path\", &attribute_path, \"path\",\n+\t\t\t   \"apply work-tree conversion for the path\"),\n \t\tOPT__QUIET(&quiet),\n \t\tOPT_CALLBACK('L', NULL, names, \"name\",\n \t\t\t     \"set labels for file1/orig_file/file2\", &label_cb),\n@@ -73,6 +76,21 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfs[i].ptr);\n \n+\tif (ret >= 0 && attribute_path) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tint st;\n+\t\tst = convert_to_working_tree(attribute_path,\n+\t\t\t\t\t     result.ptr, result.size,\n+\t\t\t\t\t     &buf);\n+\t\tif (st) {\n+\t\t\tsize_t len;\n+\n+\t\t\tfree(result.ptr);\n+\t\t\tresult.ptr = strbuf_detach(&buf, &len);\n+\t\t\tresult.size = len;\n+\t\t}\n+\t}\n+\n \tif (ret >= 0) {\n \t\tconst char *filename = argv[0];\n \t\tFILE *f = to_stdout ? stdout : fopen(filename, \"wb\");\n"},{"id":"102119","messageId":"83ocxsetqa.fsf@kalahari.s2.org","threadId":"17284","inReplyTo":"7vocxt3bsc.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] mergetool: respect autocrlf by using checkout-index","fromName":"Hannu Koivisto","fromEmail":"azure@iki.fi","sentAt":"2009-01-27T13:58:21Z","receivedAt":"2009-01-27T13:58:21Z","isPatch":true,"sender":{"key":"azure@iki.fi","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Perhaps something along this line to teach\n>>\n>>     $ git merge-file --attribute-path=frotz.c file1 orig_file file2\n>>\n>> to merge what happened since orig_file to file2 into file1, and deposit\n>> the result after converting it appropriately for path \"frotz.c\" obeying\n>> core.autocrlf and gitattribute rules.\n>>\n>> I see rerere.c::merge() has the exact same issue, but its breakage is half\n>> hidden by its use of fopen(path, \"w\").  It should explicitly use\n>> convert_to_working_tree() like this patch does, and write the results out\n>> in binary mode.\n>\n> Second try.  I forgot how convert_* worked X-<.\n\nArgh, it seems I have wasted your time.  That patch may do\nsomething useful but in this case with _or without_ it all the\nfiles seem to be correct in the filesystem before I save the merge\nresult in emerge.  I.e. it seems that for some reason Emacs detects\nthe coding system of the result file incorrectly.  I'll investigate\nthat at some point especially if ediff suffers from the same\nproblem.\n\n-- \nHannu\n"}]}