{"thread":{"id":"13872","subject":"[PATCH] Add testcase for merging in a CRLF repo, showing that conflict file is in LF only","startedAt":"2008-06-09T11:40:32Z","lastAt":"2008-06-12T20:50:34Z","messageCount":26,"participants":["Marius Storm-Olsen","Johannes Sixt","Johannes Schindelin","Junio C Hamano","Jakub Narebski","J. Bruce Fields","Jon Loeliger"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"79219","messageId":"26299.4828321554$1213013668@news.gmane.org","threadId":"13872","inReplyTo":"\"Storm-Olsen*\"@MHS","subject":"[PATCH] Add testcase for merging in a CRLF repo, showing that conflict file is in LF only","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-09T11:40:32Z","receivedAt":"2008-06-09T11:40:32Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"An LF only conflict file results in the resolved file being in LF,\nthe commit is in LF and a warning saying that LF will be replaced\nby CRLF, and the working dir ends up with a mix of CRLF and LF files.\n\nSigned-off-by: Marius Storm-Olsen \n---\n (Resend due to \"git reset --hard initial\" instead of \"git reset\n --hard a\", in the first testcase)\n \n Sorry, no patch to actually *fix* the problem.\n Someone who knows the code in question will probably find the solution in a\n fraction of the time that I would.\n Also note that :1:file, :2:file and :3:file all are also in LF format, and not\n CRLF, which you would want if core.autocrlf == true.\n\n t/t6033-merge-crlf.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 52 insertions(+), 0 deletions(-)\n create mode 100755 t/t6033-merge-crlf.sh\n\ndiff --git a/t/t6033-merge-crlf.sh b/t/t6033-merge-crlf.sh\nnew file mode 100755\nindex 0000000..8bff2f4\n--- /dev/null\n+++ b/t/t6033-merge-crlf.sh\n@@ -0,0 +1,52 @@\n+#!/bin/sh\n+\n+append_cr () {\n+\tsed -e 's/$/Q/' | tr Q '\\015'\n+}\n+\n+remove_cr () {\n+\ttr '\\015' Q | sed -e 's/Q$//'\n+}\n+\n+test_description='merge conflict in crlf repo\n+\n+\t\tb---M\n+\t / /\n+\tinitial---a\n+\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tgit config core.autocrlf true &&\n+\techo foo | append_cr >file &&\n+\tgit add file &&\n+\tgit commit -m \"Initial\" &&\n+\tgit tag initial &&\n+\tgit branch side &&\n+\techo line from a | append_cr >file &&\n+\tgit commit -m \"add line from a\" file &&\n+\tgit tag a &&\n+\tgit checkout side &&\n+\techo line from b | 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 \"ours\" is CRLF' '\n+\tgit reset --hard a &&\n+\tgit merge side -s ours &&\n+\tcat file | remove_cr | append_cr >file.temp &&\n+\ttest_cmp file file.temp\n+'\n+\n+test_expect_success 'Check that conflict file is CRLF' '\n+\tgit reset --hard a &&\n+\t! git merge side &&\n+\tcat file | remove_cr | append_cr >file.temp &&\n+\ttest_cmp file file.temp\n+'\n+\n+test_done\n-- \n1.5.6.rc0.162.gaeac2.dirty\n"},{"id":"79223","messageId":"484D3225.3020900@viscovery.net","threadId":"13872","inReplyTo":"26299.4828321554$1213013668@news.gmane.org","subject":"Re: [PATCH] Add testcase for merging in a CRLF repo, showing that conflict file is in LF only","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-06-09T13:37:41Z","receivedAt":"2008-06-09T13:37:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Marius Storm-Olsen schrieb:\n> An LF only conflict file results in the resolved file being in LF,\n> the commit is in LF and a warning saying that LF will be replaced\n> by CRLF, and the working dir ends up with a mix of CRLF and LF files.\n\nAfter reading these 3 lines I've no idea what you are talking about. Can\nyou translate this to English, please? ;-)\n\n> Sorry, no patch to actually *fix* the problem.\n\nThen you should use test_expect_failure instead of test_expect_success.\nAnd maybe also mention it in the commit message.\n\n> +test_expect_success 'Check that conflict file is CRLF' '\n> +\tgit reset --hard a &&\n> +\t! git merge side &&\n\n\ttest_must_fail git merge side &&\n\n> +\tcat file | remove_cr | append_cr >file.temp &&\n> +\ttest_cmp file file.temp\n> +'\n\n-- Hannes\n"},{"id":"79229","messageId":"484D424C.3010002@trolltech.com","threadId":"13872","inReplyTo":"484D3225.3020900@viscovery.net","subject":"Re: [PATCH] Add testcase for merging in a CRLF repo, showing that conflict file is in LF only","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-09T14:46:36Z","receivedAt":"2008-06-09T14:46:36Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Johannes Sixt said the following on 09.06.2008 15:37:\n> Marius Storm-Olsen schrieb:\n>> An LF only conflict file results in the resolved file being in LF,\n>> the commit is in LF and a warning saying that LF will be replaced\n>> by CRLF, and the working dir ends up with a mix of CRLF and LF files.\n> \n> After reading these 3 lines I've no idea what you are talking about. Can\n> you translate this to English, please? ;-)\n\nCertainly :-)\nIt means that if you work on a repo with core.autocrlf == true, you'd \nexpect every text file to have CRLF EOLs. However, if you by some \noperation, get a conflict, then the conflicted file has LF EOLs.\nNow, of course you'd go about resolving the files conflict, and then \n'git add '. When you do that, you'll get the warning saying that \nLF will be replaced by CRLF. Then you commit. The end result is that \nyou have a workingdir with a mix of LF and CRLF files, which after \nsome more operations may trigger a \"whole file changed\" diff, due to \nthe workingdir file now having LF EOLs.\n\n>> Sorry, no patch to actually *fix* the problem.\n> \n> Then you should use test_expect_failure instead of test_expect_success.\n> And maybe also mention it in the commit message.\n\nWell, the test case is written in a way that it *should* pass (iow, it \n_expects_ a success), but it currently doesn't. So, the goal is that \nsomeone, who is more intimate with the code, can just run the testcase \nuntil it passes (fixing in between each run, of course ;-)\n\n>> +test_expect_success 'Check that conflict file is CRLF' '\n>> +\tgit reset --hard a &&\n>> +\t! git merge side &&\n> \n> \ttest_must_fail git merge side &&\n\nAh, I checked a few other testcases, where I saw the ! construct. I \ndon't mind changing it, if it's important. Does it add 'feature' to \nthe testcase by using test_must_fail, instead of '!' ?\n\n>> +\tcat file | remove_cr | append_cr >file.temp &&\n>> +\ttest_cmp file file.temp\n>> +'\n> \n> -- Hannes\n\nThanks\n\n--\n.marius\n"},{"id":"79230","messageId":"484D46D6.9040900@viscovery.net","threadId":"13872","inReplyTo":"484D424C.3010002@trolltech.com","subject":"Re: [PATCH] Add testcase for merging in a CRLF repo, showing that conflict file is in LF only","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-06-09T15:05:58Z","receivedAt":"2008-06-09T15:05:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Marius Storm-Olsen schrieb:\n> Johannes Sixt said the following on 09.06.2008 15:37:\n>> Marius Storm-Olsen schrieb:\n>>> An LF only conflict file results in the resolved file being in LF,\n>>> the commit is in LF and a warning saying that LF will be replaced\n>>> by CRLF, and the working dir ends up with a mix of CRLF and LF files.\n>>\n>> After reading these 3 lines I've no idea what you are talking about. Can\n>> you translate this to English, please? ;-)\n> \n> Certainly :-)\n> It means that if you work on a repo with core.autocrlf == true, you'd\n> expect every text file to have CRLF EOLs. However, if you by some\n> operation, get a conflict, then the conflicted file has LF EOLs.\n> Now, of course you'd go about resolving the files conflict, and then\n> 'git add '. When you do that, you'll get the warning saying that\n> LF will be replaced by CRLF. Then you commit. The end result is that you\n> have a workingdir with a mix of LF and CRLF files, which after some more\n> operations may trigger a \"whole file changed\" diff, due to the\n> workingdir file now having LF EOLs.\n\nAha! Care to write it this way in the commit message in the next round? ;)\n\n>>> Sorry, no patch to actually *fix* the problem.\n>>\n>> Then you should use test_expect_failure instead of test_expect_success.\n>> And maybe also mention it in the commit message.\n> \n> Well, the test case is written in a way that it *should* pass (iow, it\n> _expects_ a success), but it currently doesn't. So, the goal is that\n> someone, who is more intimate with the code, can just run the testcase\n> until it passes (fixing in between each run, of course ;-)\n\ntest_expect_failure has changed its meaning. It's now used to say precisly\nwhat you describe here.\n\nIt means: \"We should expect this command sequence to complete\nsuccessfully, but we know that there is a bug in a git command, and hence\nwe must expect failure until it is fixed.\"\n\nSuch a test is marked as \"still broken\", and the test run is not\ninterrupted. If the bug is fixed, the test is marked as \"FIXED\" until the\n'test_expect_failure' is turned into 'test_expect_success'.\n\n>>> +test_expect_success 'Check that conflict file is CRLF' '\n>>> + git reset --hard a &&\n>>> + ! git merge side &&\n>>\n>> test_must_fail git merge side &&\n> \n> Ah, I checked a few other testcases, where I saw the ! construct. I\n> don't mind changing it, if it's important. Does it add 'feature' to the\n> testcase by using test_must_fail, instead of '!' ?\n\n'! git cmd' says that any unusual exit is ok, even a segfault and\nincorrect usage. 'test_must_fail git cmd' says that only deliberate error\nexits are ok.\n\n-- Hannes\n"},{"id":"79254","messageId":"484D881B.1070400@trolltech.com","threadId":"13872","inReplyTo":"484D424C.3010002@trolltech.com","subject":"Re: [PATCH] Add testcase for merging in a CRLF repo, showing that conflict file is in LF only","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-09T19:44:27Z","receivedAt":"2008-06-09T19:44:27Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Marius Storm-Olsen said the following on 09.06.2008 16:46:\n> ... Then you commit. The end result is that \n> you have a workingdir with a mix of LF and CRLF files, which after \n> some more operations may trigger a \"whole file changed\" diff, due to \n> the workingdir file now having LF EOLs.\n\n..actually, it's more like applying patches and cherry-picking then \nbreaks when the touch the same file, if I recall correctly.\n\nThanks for all the pointers, I'll send an updated patch tomorrow.\n\n--\n.marius\n"},{"id":"79273","messageId":"alpine.DEB.1.00.0806092221420.1783@racer","threadId":"13872","inReplyTo":"484D3225.3020900@viscovery.net","subject":"[PATCH 1/2] Add testcase for merging in a CRLF repo","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T21:22:37Z","receivedAt":"2008-06-09T21:22:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nFrom: Marius Storm-Olsen \n\nIf you work on a repo with core.autocrlf == true, you would expect\nevery text file to have CRLF EOLs. However, if you by some operation,\nget a conflict, then the conflicted file has LF EOLs.\n\nNow, of course you'd go about resolving the files conflict, and then 'git\nadd '. When you do that, you'll get the warning saying that LF will\nbe replaced by CRLF. Then you commit. The end result is that you have a\nworkingdir with a mix of LF and CRLF files, which after some more\noperations may trigger a \"whole file changed\" diff, due to the workingdir\nfile now having LF EOLs.\n\nAn LF only conflict file results in the resolved file being in LF,\nthe commit is in LF and a warning saying that LF will be replaced\nby CRLF, and the working dir ends up with a mix of CRLF and LF files.\n\nSigned-off-by: Marius Storm-Olsen \nSigned-off-by: Johannes Schindelin \n---\n t/t6033-merge-crlf.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 52 insertions(+), 0 deletions(-)\n create mode 100755 t/t6033-merge-crlf.sh\n\ndiff --git a/t/t6033-merge-crlf.sh b/t/t6033-merge-crlf.sh\nnew file mode 100755\nindex 0000000..ea22837\n--- /dev/null\n+++ b/t/t6033-merge-crlf.sh\n@@ -0,0 +1,52 @@\n+#!/bin/sh\n+\n+append_cr () {\n+\tsed -e 's/$/Q/' | tr Q '\\015'\n+}\n+\n+remove_cr () {\n+\ttr '\\015' Q | sed -e 's/Q$//'\n+}\n+\n+test_description='merge conflict in crlf repo\n+\n+\t\tb---M\n+\t / /\n+\tinitial---a\n+\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tgit config core.autocrlf true &&\n+\techo foo | append_cr >file &&\n+\tgit add file &&\n+\tgit commit -m \"Initial\" &&\n+\tgit tag initial &&\n+\tgit branch side &&\n+\techo line from a | append_cr >file &&\n+\tgit commit -m \"add line from a\" file &&\n+\tgit tag a &&\n+\tgit checkout side &&\n+\techo line from b | 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 \"ours\" is CRLF' '\n+\tgit reset --hard initial &&\n+\tgit merge side -s ours &&\n+\tcat file | remove_cr | append_cr >file.temp &&\n+\ttest_cmp file file.temp\n+'\n+\n+test_expect_failure 'Check that conflict file is CRLF' '\n+\tgit reset --hard a &&\n+\ttest_must_fail git merge side &&\n+\tcat file | remove_cr | append_cr >file.temp &&\n+\ttest_cmp file file.temp\n+'\n+\n+test_done\n-- \n1.5.6.rc1.181.gb439d\n"},{"id":"79274","messageId":"alpine.DEB.1.00.0806092223010.1783@racer","threadId":"13872","inReplyTo":"alpine.DEB.1.00.0806092221420.1783@racer","subject":"[PATCH 2/2] merge-recursive: respect core.autocrlf","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T21:23:16Z","receivedAt":"2008-06-09T21:23:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSigned-off-by: Johannes Schindelin \n---\n builtin-merge-recursive.c | 8 ++++++++\n t/t6033-merge-crlf.sh | 2 +-\n 2 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 7643f17..edd023f 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -525,6 +525,7 @@ static void update_file_flags(const unsigned char *sha,\n \t\tenum object_type type;\n \t\tvoid *buf;\n \t\tunsigned long size;\n+\t\tstruct strbuf strbuf;\n \n \t\tif (S_ISGITLINK(mode))\n \t\t\tdie(\"cannot read object %s '%s': It is a submodule!\",\n@@ -535,6 +536,12 @@ static void update_file_flags(const unsigned char *sha,\n \t\t\tdie(\"cannot read object %s '%s'\", sha1_to_hex(sha), path);\n \t\tif (type != OBJ_BLOB)\n \t\t\tdie(\"blob expected for %s '%s'\", sha1_to_hex(sha), path);\n+\t\tstrbuf_init(&strbuf, 0);\n+\t\tif (convert_to_working_tree(path, buf, size, &strbuf)) {\n+\t\t\tfree(buf);\n+\t\t\tsize = strbuf.len;\n+\t\t\tbuf = strbuf_detach(&strbuf, NULL);\n+\t\t}\n \n \t\tif (make_room_for_path(path) < 0) {\n \t\t\tupdate_wd = 0;\n@@ -560,6 +567,7 @@ static void update_file_flags(const unsigned char *sha,\n \t\t} else\n \t\t\tdie(\"do not know what to do with %06o %s '%s'\",\n \t\t\t mode, sha1_to_hex(sha), path);\n+\t\tfree(buf);\n \t}\n update_index:\n \tif (update_cache)\ndiff --git a/t/t6033-merge-crlf.sh b/t/t6033-merge-crlf.sh\nindex ea22837..75d9602 100755\n--- a/t/t6033-merge-crlf.sh\n+++ b/t/t6033-merge-crlf.sh\n@@ -42,7 +42,7 @@ test_expect_success 'Check \"ours\" is CRLF' '\n \ttest_cmp file file.temp\n '\n \n-test_expect_failure 'Check that conflict file is CRLF' '\n+test_expect_success 'Check that conflict file is CRLF' '\n \tgit reset --hard a &&\n \ttest_must_fail git merge side &&\n \tcat file | remove_cr | append_cr >file.temp &&\n-- \n1.5.6.rc1.181.gb439d\n"},{"id":"79276","messageId":"7vod6affz6.fsf@gitster.siamese.dyndns.org","threadId":"13872","inReplyTo":"alpine.DEB.1.00.0806092223010.1783@racer","subject":"Re: [PATCH 2/2] merge-recursive: respect core.autocrlf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-09T21:36:29Z","receivedAt":"2008-06-09T21:36:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin-merge-recursive.c | 8 ++++++++\n> t/t6033-merge-crlf.sh | 2 +-\n> 2 files changed, 9 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\n> index 7643f17..edd023f 100644\n> --- a/builtin-merge-recursive.c\n> +++ b/builtin-merge-recursive.c\n> @@ -525,6 +525,7 @@ static void update_file_flags(const unsigned char *sha,\n> \t\tenum object_type type;\n> \t\tvoid *buf;\n> \t\tunsigned long size;\n> +\t\tstruct strbuf strbuf;\n> \n> \t\tif (S_ISGITLINK(mode))\n> \t\t\tdie(\"cannot read object %s '%s': It is a submodule!\",\n> @@ -535,6 +536,12 @@ static void update_file_flags(const unsigned char *sha,\n> \t\t\tdie(\"cannot read object %s '%s'\", sha1_to_hex(sha), path);\n> \t\tif (type != OBJ_BLOB)\n> \t\t\tdie(\"blob expected for %s '%s'\", sha1_to_hex(sha), path);\n> +\t\tstrbuf_init(&strbuf, 0);\n> +\t\tif (convert_to_working_tree(path, buf, size, &strbuf)) {\n> +\t\t\tfree(buf);\n> +\t\t\tsize = strbuf.len;\n> +\t\t\tbuf = strbuf_detach(&strbuf, NULL);\n> +\t\t}\n> \n> \t\tif (make_room_for_path(path) < 0) {\n> \t\t\tupdate_wd = 0;\n\nFairly straightforward fix, except that I suspect this needs to be done\nonly for regular files and not symlinks.\n\nI think entry.c:write_entry() shows how this should be done.\n"},{"id":"79288","messageId":"alpine.DEB.1.00.0806092305430.1783@racer","threadId":"13872","inReplyTo":"7vod6affz6.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] merge-recursive: respect core.autocrlf","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T22:59:31Z","receivedAt":"2008-06-09T22:59:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Mon, 9 Jun 2008, Junio C Hamano wrote:\n\n\t> Fairly straightforward fix, except that I suspect this needs to \n\t> be done only for regular files and not symlinks.\n\t> \n\t> I think entry.c:write_entry() shows how this should be done.\n\n\tRight. And the relevant clause is actually already there. D'oh.\n\n builtin-merge-recursive.c | 12 +++++++++++-\n t/t6033-merge-crlf.sh | 2 +-\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 7643f17..1fbff3a 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -535,13 +535,22 @@ static void update_file_flags(const unsigned char *sha,\n \t\t\tdie(\"cannot read object %s '%s'\", sha1_to_hex(sha), path);\n \t\tif (type != OBJ_BLOB)\n \t\t\tdie(\"blob expected for %s '%s'\", sha1_to_hex(sha), path);\n-\n \t\tif (make_room_for_path(path) < 0) {\n \t\t\tupdate_wd = 0;\n \t\t\tgoto update_index;\n \t\t}\n \t\tif (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {\n \t\t\tint fd;\n+\t\t\tstruct strbuf strbuf;\n+\n+\t\t\tstrbuf_init(&strbuf, 0);\n+\t\t\tif (convert_to_working_tree(path, buf, size, &strbuf)) {\n+\t\t\t\tsize_t newsize = 0;\n+\t\t\t\tfree(buf);\n+\t\t\t\tbuf = strbuf_detach(&strbuf, &newsize);\n+\t\t\t\tsize = newsize;\n+\t\t\t}\n+\n \t\t\tif (mode & 0100)\n \t\t\t\tmode = 0777;\n \t\t\telse\n@@ -560,6 +569,7 @@ static void update_file_flags(const unsigned char *sha,\n \t\t} else\n \t\t\tdie(\"do not know what to do with %06o %s '%s'\",\n \t\t\t mode, sha1_to_hex(sha), path);\n+\t\tfree(buf);\n \t}\n update_index:\n \tif (update_cache)\ndiff --git a/t/t6033-merge-crlf.sh b/t/t6033-merge-crlf.sh\nindex ea22837..75d9602 100755\n--- a/t/t6033-merge-crlf.sh\n+++ b/t/t6033-merge-crlf.sh\n@@ -42,7 +42,7 @@ test_expect_success 'Check \"ours\" is CRLF' '\n \ttest_cmp file file.temp\n '\n \n-test_expect_failure 'Check that conflict file is CRLF' '\n+test_expect_success 'Check that conflict file is CRLF' '\n \tgit reset --hard a &&\n \ttest_must_fail git merge side &&\n \tcat file | remove_cr | append_cr >file.temp &&\n-- \n1.5.6.rc1.181.gb439d\n"},{"id":"79292","messageId":"7vprqqdwh7.fsf@gitster.siamese.dyndns.org","threadId":"13872","inReplyTo":"alpine.DEB.1.00.0806092305430.1783@racer","subject":"Re: [PATCH v2] merge-recursive: respect core.autocrlf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-09T23:23:00Z","receivedAt":"2008-06-09T23:23:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Signed-off-by: Johannes Schindelin \n> ---\n>\n> \tOn Mon, 9 Jun 2008, Junio C Hamano wrote:\n>\n> \t> Fairly straightforward fix, except that I suspect this needs to \n> \t> be done only for regular files and not symlinks.\n> \t> \n> \t> I think entry.c:write_entry() shows how this should be done.\n>\n> \tRight. And the relevant clause is actually already there. D'oh.\n\nWell, you actually have \"double d'oh\". \"This ought to be a symlink but\nthe filesystem is lacking, so we instead write out what the readlink from\nsuch a symlink would return\" codepath should not convert_to_worktree().\n\nI'll fix it up, no need to resend. Thanks for the fix.\n"},{"id":"79296","messageId":"alpine.DEB.1.00.0806100033350.1783@racer","threadId":"13872","inReplyTo":"7vprqqdwh7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] merge-recursive: respect core.autocrlf","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T23:35:34Z","receivedAt":"2008-06-09T23:35:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Signed-off-by: Johannes Schindelin \n> > ---\n> >\n> > \tOn Mon, 9 Jun 2008, Junio C Hamano wrote:\n> >\n> > \t> Fairly straightforward fix, except that I suspect this needs to \n> > \t> be done only for regular files and not symlinks.\n> > \t> \n> > \t> I think entry.c:write_entry() shows how this should be done.\n> >\n> > \tRight. And the relevant clause is actually already there. D'oh.\n> \n> Well, you actually have \"double d'oh\". \"This ought to be a symlink but \n> the filesystem is lacking, so we instead write out what the readlink \n> from such a symlink would return\" codepath should not \n> convert_to_worktree().\n\nI actually thought about that a bit, and just assumed that the rest of the \nGit code respects autocrlf for \"fake\" symlinks.\n\nIMO it makes no sense at all to write the textual symlink files without \nCR/LF when the user clearly asked for it with autocrlf = true. After all, \nit _is_ a text file then.\n\nBut yes, I tried to save some time and did not check.\n\nCiao,\nDscho\n"},{"id":"79322","messageId":"2e371261399563d49665f26eb06dd19fd3b071cb.1213084587.git.marius@trolltech.com","threadId":"13872","inReplyTo":"cover.1213084587.git.marius@trolltech.com","subject":"[PATCH 1/2] Add testcases for verifying that staged files in a conflict are CRLF, when core.autocrlf = true","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-10T07:40:07Z","receivedAt":"2008-06-10T07:40:07Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"When you 'git show :2:' in a conflict, the file should have CRLF EOLs,\nif the repo is configured with core.autocrlf = true.\n\nSigned-off-by: Marius Storm-Olsen \n---\n t/t6033-merge-crlf.sh | 18 ++++++++++++++++++\n 1 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6033-merge-crlf.sh b/t/t6033-merge-crlf.sh\nindex 75d9602..f161b40 100755\n--- a/t/t6033-merge-crlf.sh\n+++ b/t/t6033-merge-crlf.sh\n@@ -49,4 +49,22 @@ test_expect_success 'Check that conflict file is CRLF' '\n \ttest_cmp file file.temp\n '\n \n+test_expect_failure 'Check that staged file :1: is CRLF' '\n+\tgit show :1:file >staged.temp1 &&\n+\tgit show :1:file | remove_cr | append_cr >staged.temp2 &&\n+\ttest_cmp staged.temp1 staged.temp2\n+'\n+\n+test_expect_failure 'Check that staged file :2: is CRLF' '\n+\tgit show :2:file >staged.temp1 &&\n+\tgit show :2:file | remove_cr | append_cr >staged.temp2 &&\n+\ttest_cmp staged.temp1 staged.temp2\n+'\n+\n+test_expect_failure 'Check that staged file :3: is CRLF' '\n+\tgit show :3:file >staged.temp1 &&\n+\tgit show :3:file | remove_cr | append_cr >staged.temp2 &&\n+\ttest_cmp staged.temp1 staged.temp2\n+'\n+\n test_done\n-- \n1.5.6.rc2.158.g3478\n"},{"id":"79323","messageId":"3478a0d599f41fad9d8509dc264cbe34772446e2.1213084587.git.marius@trolltech.com","threadId":"13872","inReplyTo":"cover.1213084587.git.marius@trolltech.com","subject":"[PATCH 2/2] Ensure that objects shown in a core.autocrlf = true repo have CRLF EOLs","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-10T07:55:09Z","receivedAt":"2008-06-10T07:55:09Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"When you show an object, it should be shown with the EOLs which the repo\nis configured for, and not how it's stored internally in the object store.\n\nSigned-off-by: Marius Storm-Olsen \n---\n builtin-log.c | 19 ++++++++++++++-----\n t/t6033-merge-crlf.sh | 6 +++---\n 2 files changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 9817d6f..94367f6 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -287,8 +287,8 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)\n \t show_date(date, tz, rev->date_mode));\n }\n \n-static int show_object(const unsigned char *sha1, int show_tag_object,\n-\tstruct rev_info *rev)\n+static int show_object(const unsigned char *sha1, const char *name,\n+\tint show_tag_object, struct rev_info *rev)\n {\n \tunsigned long size;\n \tenum object_type type;\n@@ -309,8 +309,17 @@ static int show_object(const unsigned char *sha1, int show_tag_object,\n \t\t\toffset = new_offset;\n \t\t}\n \n-\tif (offset < size)\n+\tif (offset < size) {\n+\t\tstruct strbuf strbuf;\n+\t\tstrbuf_init(&strbuf, 0);\n+\t\tif (convert_to_working_tree(name, buf + offset, size - offset, &strbuf)) {\n+\t\t\tfree(buf);\n+\t\t\toffset = 0;\n+\t\t\tsize = strbuf.len;\n+\t\t\tbuf = strbuf_detach(&strbuf, NULL);\n+\t\t}\n \t\tfwrite(buf + offset, size - offset, 1, stdout);\n+\t}\n \tfree(buf);\n \treturn 0;\n }\n@@ -350,7 +359,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \t\tconst char *name = objects[i].name;\n \t\tswitch (o->type) {\n \t\tcase OBJ_BLOB:\n-\t\t\tret = show_object(o->sha1, 0, NULL);\n+\t\t\tret = show_object(o->sha1, name, 0, NULL);\n \t\t\tbreak;\n \t\tcase OBJ_TAG: {\n \t\t\tstruct tag *t = (struct tag *)o;\n@@ -359,7 +368,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tdiff_get_color_opt(&rev.diffopt, DIFF_COMMIT),\n \t\t\t\t\tt->tag,\n \t\t\t\t\tdiff_get_color_opt(&rev.diffopt, DIFF_RESET));\n-\t\t\tret = show_object(o->sha1, 1, &rev);\n+\t\t\tret = show_object(o->sha1, name, 1, &rev);\n \t\t\tobjects[i].item = (struct object *)t->tagged;\n \t\t\ti--;\n \t\t\tbreak;\ndiff --git a/t/t6033-merge-crlf.sh b/t/t6033-merge-crlf.sh\nindex f161b40..d1d1dcb 100755\n--- a/t/t6033-merge-crlf.sh\n+++ b/t/t6033-merge-crlf.sh\n@@ -49,19 +49,19 @@ test_expect_success 'Check that conflict file is CRLF' '\n \ttest_cmp file file.temp\n '\n \n-test_expect_failure 'Check that staged file :1: is CRLF' '\n+test_expect_success 'Check that staged file :1: is CRLF' '\n \tgit show :1:file >staged.temp1 &&\n \tgit show :1:file | remove_cr | append_cr >staged.temp2 &&\n \ttest_cmp staged.temp1 staged.temp2\n '\n \n-test_expect_failure 'Check that staged file :2: is CRLF' '\n+test_expect_success 'Check that staged file :2: is CRLF' '\n \tgit show :2:file >staged.temp1 &&\n \tgit show :2:file | remove_cr | append_cr >staged.temp2 &&\n \ttest_cmp staged.temp1 staged.temp2\n '\n \n-test_expect_failure 'Check that staged file :3: is CRLF' '\n+test_expect_success 'Check that staged file :3: is CRLF' '\n \tgit show :3:file >staged.temp1 &&\n \tgit show :3:file | remove_cr | append_cr >staged.temp2 &&\n \ttest_cmp staged.temp1 staged.temp2\n-- \n1.5.6.rc2.158.g3478\n"},{"id":"79321","messageId":"cover.1213084587.git.marius@trolltech.com","threadId":"13872","inReplyTo":"7vprqqdwh7.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-10T08:10:27Z","receivedAt":"2008-06-10T08:10:27Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"When you use 'git show :' or 'git show ::',\nthe objects are shows as they are in the object store, ignoring the\ncore.autocrlf configuration.\n\nThis series adds testcases which checks the stage files in a merge\nconflict, and a fix for the problem.\n\nRunning all testcases before and after the fix reveals no regressions:\n\nBefore patch series:\n ./aggregate-results.sh test-results/t*-*\n fixed 1\n success 3374\n failed 0\n broken 2\n total 3377\n \nAfter patch series: \n ./aggregate-results.sh test-results/t*-*\n fixed 1\n success 3377\n failed 0\n broken 2\n total 3380\n rm -f -r 'trash directory' test-results\n\nMarius Storm-Olsen (2):\n Add testcases for verifying that staged files in a conflict are CRLF,\n when core.autocrlf = true\n Ensure that objects shown in a core.autocrlf = true repo have CRLF\n EOLs\n\n builtin-log.c | 19 ++++++++++++++-----\n t/t6033-merge-crlf.sh | 18 ++++++++++++++++++\n 2 files changed, 32 insertions(+), 5 deletions(-)\n"},{"id":"79361","messageId":"alpine.DEB.1.00.0806101632570.1783@racer","threadId":"13872","inReplyTo":"cover.1213084587.git.marius@trolltech.com","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-10T15:34:15Z","receivedAt":"2008-06-10T15:34:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Jun 2008, Marius Storm-Olsen wrote:\n\n> When you use 'git show :' or 'git show ::', the \n> objects are shows as they are in the object store, ignoring the \n> core.autocrlf configuration.\n\nI think this is the correct behaviour: inside the object repository, the \nfiles are supposed to be LF clean.\n\nLikewise, things in the unmerged stages are in the index, which again is \nnot the working directory, so they should be LF clean.\n\n_Only_ when writing a file to the working directory, it should get \nclobbered.\n\nCiao,\nDscho\n"},{"id":"79395","messageId":"7vk5gxc4gz.fsf@gitster.siamese.dyndns.org","threadId":"13872","inReplyTo":"alpine.DEB.1.00.0806101632570.1783@racer","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-10T22:25:32Z","receivedAt":"2008-06-10T22:25:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Tue, 10 Jun 2008, Marius Storm-Olsen wrote:\n>\n>> When you use 'git show :' or 'git show ::', the \n>> objects are shows as they are in the object store, ignoring the \n>> core.autocrlf configuration.\n>\n> I think this is the correct behaviour: inside the object repository, the \n> files are supposed to be LF clean.\n>\n> Likewise, things in the unmerged stages are in the index, which again is \n> not the working directory, so they should be LF clean.\n>\n> _Only_ when writing a file to the working directory, it should get \n> clobbered.\n\nI'd agree with your argument on general principle, but it might make sense\nto give an option to let you say \"here is a blob contents, and use the\nattribute for this path to munge it out to the filesystem.\" I am not sure\nif that belongs to \"git show\" Porcelain, though. It _could_ be more like:\n\n git checkout-blob $blob_sha1 $path\n\nthat (1) reads the blob object specified by its object name, (2)\ngrabs attribute for the $path, and (3) applies convert_to_worktree()\nfiltering given that attribute and deposits the results to $path.\n\nAlternatively, the interface could be:\n\n git cat-file blob $blob_sha1 |\n git filter-blob --use-attr-for=$path >$path.old\n\nso that you can then do:\n\n\tgit diff --no-index $path.old $path\n\nI dunno.\n"},{"id":"79432","messageId":"484F6A27.1040602@trolltech.com","threadId":"13872","inReplyTo":"7vk5gxc4gz.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-11T06:01:11Z","receivedAt":"2008-06-11T06:01:11Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Junio C Hamano said the following on 11.06.2008 00:25:\n> Johannes Schindelin writes:\n>> On Tue, 10 Jun 2008, Marius Storm-Olsen wrote:\n>>> When you use 'git show :' or 'git show ::', the \n>>> objects are shows as they are in the object store, ignoring the \n>>> core.autocrlf configuration.\n>> I think this is the correct behaviour: inside the object repository, the \n>> files are supposed to be LF clean.\n>>\n>> Likewise, things in the unmerged stages are in the index, which again is \n>> not the working directory, so they should be LF clean.\n>>\n>> _Only_ when writing a file to the working directory, it should get \n>> clobbered.\n> \n> I'd agree with your argument on general principle, but it might make sense\n> to give an option to let you say \"here is a blob contents, and use the\n> attribute for this path to munge it out to the filesystem.\" I am not sure\n> if that belongs to \"git show\" Porcelain, though. It _could_ be more like:\n> \n> git checkout-blob $blob_sha1 $path\n> \n> that (1) reads the blob object specified by its object name, (2)\n> grabs attribute for the $path, and (3) applies convert_to_worktree()\n> filtering given that attribute and deposits the results to $path.\n> \n> Alternatively, the interface could be:\n> \n> git cat-file blob $blob_sha1 |\n> git filter-blob --use-attr-for=$path >$path.old\n> \n> so that you can then do:\n> \n> \tgit diff --no-index $path.old $path\n> \n> I dunno.\n\nWell, consider this:\nSay you are merging two branches, and know that you want to just use \nthe parts which conflict from the branch being merged in. Then you \nsimply do:\n\tgit merge side\n\tgit show :3:file.txt > file.txt\n\tgit add file.txt\n\tgit commit ...blah blah...\n\nNow, with the current behavior, this workflow breaks for \ncore.autocrlf=true repos. Given that 'git show' *is* porcelain, I'd \nexpect it to work 'naturally' in my workflow, and not dump raw object \nstore content.\nHowever, I also see that it can be useful at times. Almost makes me \nconsider a --raw option to 'git show' for those seldom cases. IMO, \n'git show' *should* care about autocrlf. Not doing so is just \nconfusing to the end-user.\n\nThe fact that the stage files are in the index doesn't matter. I'd \nwant CRLF files from 'git show v1.5.6-rc0:builtin-log.c' as well.\n\n-- \n.marius [@trolltech.com]\n'if you know what you're doing, it's not research'\n\n"},{"id":"79441","messageId":"m3hcc0s7f5.fsf@localhost.localdomain","threadId":"13872","inReplyTo":"484F6A27.1040602@trolltech.com","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-11T08:25:57Z","receivedAt":"2008-06-11T08:25:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Marius Storm-Olsen writes:\n\n> However, I also see that it can be useful at times. Almost makes me\n> consider a --raw option to 'git show' for those seldom cases. IMO,\n> 'git show' *should* care about autocrlf. Not doing so is just\n> confusing to the end-user.\n\nThere is always \"git cat-file -p\" which is porcelain, and should not\ncare about attributes (perhaps with the exception of explicitely told\nso with some command option).\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"79482","messageId":"alpine.DEB.1.00.0806112000400.1783@racer","threadId":"13872","inReplyTo":"484F6A27.1040602@trolltech.com","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-11T19:06:08Z","receivedAt":"2008-06-11T19:06:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 11 Jun 2008, Marius Storm-Olsen wrote:\n\n> Well, consider this:\n>\n> Say you are merging two branches, and know that you want to just use the \n> parts which conflict from the branch being merged in. Then you simply \n> do:\n>\n> \tgit merge side\n> \tgit show :3:file.txt > file.txt\n\nThis is not really how I would do things. I would do\n\n\tgit checkout side file.txt here.\n\nThe _point_ is: \"git show\" is supposed to show you the contents _in the \nrepository_. For example, no smudge/clean filters will be heeded, and \nneither other attributes.\n\nFurther, \"git show\" will work without any problems in any bare repository.\n\nIn other words: \"git show\" is _not_ an operation on a working directory.\n\n\"git checkout\" is. So use that instead.\n\n> Given that 'git show' *is* porcelain, I'd expect it to work 'naturally' \n> in my workflow, and not dump raw object store content.\n\nDo not confuse porcelain with \"works on the working directory\".\n\n> The fact that the stage files are in the index doesn't matter. I'd want \n> CRLF files from 'git show v1.5.6-rc0:builtin-log.c' as well.\n\nBut it _does_ matter!\n\nThe index works on raw objects, not on smudged files. Period.\n\nCiao,\nDscho\n"},{"id":"79578","messageId":"4850E647.7050602@trolltech.com","threadId":"13872","inReplyTo":"alpine.DEB.1.00.0806112000400.1783@racer","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-12T09:03:03Z","receivedAt":"2008-06-12T09:03:03Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Johannes Schindelin said the following on 11.06.2008 21:06:\n> On Wed, 11 Jun 2008, Marius Storm-Olsen wrote:\n>> Well, consider this:\n>>\n>> Say you are merging two branches, and know that you want to just use the \n>> parts which conflict from the branch being merged in. Then you simply \n>> do:\n>>\n>> \tgit merge side\n>> \tgit show :3:file.txt > file.txt\n> \n> This is not really how I would do things. I would do\n> \n> \tgit checkout side file.txt here.\n\nUhm, 'git checkout side file.txt' is not the same file content \n(ignoring EOLs please) as 'git show :3:file.txt'.\nRef: user-manual.html#conflict-resolution\n\n> The _point_ is: \"git show\" is supposed to show you the contents _in the \n> repository_. For example, no smudge/clean filters will be heeded, and \n> neither other attributes.\n\nYou are describing \"git cat-file\".\nIMO, \"git show\" should have more consideration towards the repo \nsettings. I doubt anyone, excluding yourself and a few more \nold-timers, think the content they get out from \"git show \" is \n*not* the content they'll get when they decide to \"git checkout \n\". For most people the commands a mostly the same, except that \n\"show\" just stdout-dumps the content, while \"checkout\" writes it to \ndisk. The subtle difference there is simply just confusing, and is \nwhat we need to fix so people won't find Git so hard to use. It's all \nabout usability. Let \"git cat-file\" do raw dumps, and \"git show\" what \nmost people would expect.\n\nSeen another way: If you \"git show\" any object, they are formated in a \nnice way for the user to see the output; not raw dumps. There's no \nreason why the user should even consider that when they show a plain \nblob, *then* it's raw (in the sense that EOLs are not handled properly).\n\nThe \"show\" command is too nice and convenient for it to have such a \ndisrespect for the user.\n\n> Further, \"git show\" will work without any problems in any bare repository.\n\nSure, it writes to stdout, and not to file. People understand that.\n\n> In other words: \"git show\" is _not_ an operation on a working directory.\n\nSee above. Nobody expect it to touch files. However, any repo (even \nbare) still has a config file though, and \"git show\" should respect \nits settings.\n\n> \"git checkout\" is. So use that instead.\n\n\"git checkout\" doesn't munge ::, which is what the \ndocumentation is referring to when it comes to conflict resolution.\n\n>> Given that 'git show' *is* porcelain, I'd expect it to work 'naturally' \n>> in my workflow, and not dump raw object store content.\n> \n> Do not confuse porcelain with \"works on the working directory\".\n\nI don't. But I'm trying to see the workflow from a non-git-master POV, \nyou're obviously not.\n\n>> The fact that the stage files are in the index doesn't matter. I'd want \n>> CRLF files from 'git show v1.5.6-rc0:builtin-log.c' as well.\n> \n> But it _does_ matter!\n> The index works on raw objects, not on smudged files. Period.\n\nYou misunderstood me. I don't smudged files in the index.\n\n-- \n.marius [@trolltech.com]\n'if you know what you're doing, it's not research'\n\n"},{"id":"79618","messageId":"7vtzfy8n4i.fsf@gitster.siamese.dyndns.org","threadId":"13872","inReplyTo":"4850E647.7050602@trolltech.com","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-12T19:33:01Z","receivedAt":"2008-06-12T19:33:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marius Storm-Olsen writes:\n\n> Johannes Schindelin said the following on 11.06.2008 21:06:\n>> On Wed, 11 Jun 2008, Marius Storm-Olsen wrote:\n>>> Well, consider this:\n>>>\n>>> Say you are merging two branches, and know that you want to just\n>>> use the parts which conflict from the branch being merged in. Then\n>>> you simply do:\n>>>\n>>> \tgit merge side\n>>> \tgit show :3:file.txt > file.txt\n>>\n>> This is not really how I would do things. I would do\n>>\n>> \tgit checkout side file.txt here.\n>\n> Uhm, 'git checkout side file.txt' is not the same file content\n> (ignoring EOLs please) as 'git show :3:file.txt'.\n> Ref: user-manual.html#conflict-resolution\n\nBruce, I think the comment in this quoted section is wrong.\n\nTrue, the combined diff can show only the interesting differences between\nthe three, but that is not because we munge stage #2 and #3. They contain\nverbatim copies from the HEAD and the MERGE_HEAD respectively, and the\ncombining logic runs three-way diffs between the three stages to discard\nthe hunks that the result comes solely from either stage #2 or stage #3.\n\nSo perhaps something like this is in order...\n\n---\n\n Documentation/user-manual.txt | 15 +++++++--------\n 1 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt\nindex bfde507..64a820b 100644\n--- a/Documentation/user-manual.txt\n+++ b/Documentation/user-manual.txt\n@@ -1254,16 +1254,15 @@ these three \"file stages\" represents a different version of the file:\n \n -------------------------------------------------\n $ git show :1:file.txt\t# the file in a common ancestor of both branches\n-$ git show :2:file.txt\t# the version from HEAD, but including any\n-\t\t\t# nonconflicting changes from MERGE_HEAD\n-$ git show :3:file.txt\t# the version from MERGE_HEAD, but including any\n-\t\t\t# nonconflicting changes from HEAD.\n+$ git show :2:file.txt\t# the version from HEAD.\n+$ git show :3:file.txt\t# the version from MERGE_HEAD.\n -------------------------------------------------\n \n-Since the stage 2 and stage 3 versions have already been updated with\n-nonconflicting changes, the only remaining differences between them are\n-the important ones; thus linkgit:git-diff[1] can use the information in\n-the index to show only those conflicts.\n+When you ask linkgit:git-diff[1] to show the conflicts, it runs a\n+three-way diff between the conflicted merge results in the work tree with\n+stages 2 and 3 to show only hunks whose contents come from both sides,\n+mixed (in other words, when a hunk's merge results come only from stage 2,\n+that part is not conflicting and is not shown. Same for stage 3).\n \n The diff above shows the differences between the working-tree version of\n file.txt and the stage 2 and stage 3 versions. So instead of preceding\n"},{"id":"79619","messageId":"20080612195553.GK13626@fieldses.org","threadId":"13872","inReplyTo":"7vtzfy8n4i.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2008-06-12T19:55:53Z","receivedAt":"2008-06-12T19:55:53Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Thu, Jun 12, 2008 at 12:33:01PM -0700, Junio C Hamano wrote:\n> Marius Storm-Olsen writes:\n> \n> > Johannes Schindelin said the following on 11.06.2008 21:06:\n> >> On Wed, 11 Jun 2008, Marius Storm-Olsen wrote:\n> >>> Well, consider this:\n> >>>\n> >>> Say you are merging two branches, and know that you want to just\n> >>> use the parts which conflict from the branch being merged in. Then\n> >>> you simply do:\n> >>>\n> >>> \tgit merge side\n> >>> \tgit show :3:file.txt > file.txt\n> >>\n> >> This is not really how I would do things. I would do\n> >>\n> >> \tgit checkout side file.txt here.\n> >\n> > Uhm, 'git checkout side file.txt' is not the same file content\n> > (ignoring EOLs please) as 'git show :3:file.txt'.\n> > Ref: user-manual.html#conflict-resolution\n> \n> Bruce, I think the comment in this quoted section is wrong.\n> \n> True, the combined diff can show only the interesting differences between\n> the three, but that is not because we munge stage #2 and #3. They contain\n> verbatim copies from the HEAD and the MERGE_HEAD respectively, and the\n> combining logic runs three-way diffs between the three stages to discard\n> the hunks that the result comes solely from either stage #2 or stage #3.\n\nOops, thanks for catching that! I don't know how I got the idea it\nworked that way.\n\n(Is there any advantage, then, to the :n:filename syntax to a user?\nIs it useful in any cases when they couldn't use HEAD or MERGE_HEAD\ninstead? If not I might be tempted to cut this bit entirely (or\npostpone it till later.)\n\n--b.\n\n> \n> So perhaps something like this is in order...\n> \n> ---\n> \n> Documentation/user-manual.txt | 15 +++++++--------\n> 1 files changed, 7 insertions(+), 8 deletions(-)\n> \n> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt\n> index bfde507..64a820b 100644\n> --- a/Documentation/user-manual.txt\n> +++ b/Documentation/user-manual.txt\n> @@ -1254,16 +1254,15 @@ these three \"file stages\" represents a different version of the file:\n> \n> -------------------------------------------------\n> $ git show :1:file.txt\t# the file in a common ancestor of both branches\n> -$ git show :2:file.txt\t# the version from HEAD, but including any\n> -\t\t\t# nonconflicting changes from MERGE_HEAD\n> -$ git show :3:file.txt\t# the version from MERGE_HEAD, but including any\n> -\t\t\t# nonconflicting changes from HEAD.\n> +$ git show :2:file.txt\t# the version from HEAD.\n> +$ git show :3:file.txt\t# the version from MERGE_HEAD.\n> -------------------------------------------------\n> \n> -Since the stage 2 and stage 3 versions have already been updated with\n> -nonconflicting changes, the only remaining differences between them are\n> -the important ones; thus linkgit:git-diff[1] can use the information in\n> -the index to show only those conflicts.\n> +When you ask linkgit:git-diff[1] to show the conflicts, it runs a\n> +three-way diff between the conflicted merge results in the work tree with\n> +stages 2 and 3 to show only hunks whose contents come from both sides,\n> +mixed (in other words, when a hunk's merge results come only from stage 2,\n> +that part is not conflicting and is not shown. Same for stage 3).\n> \n> The diff above shows the differences between the working-tree version of\n> file.txt and the stage 2 and stage 3 versions. So instead of preceding\n"},{"id":"79623","messageId":"48518418.2010007@trolltech.com","threadId":"13872","inReplyTo":"7vtzfy8n4i.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2008-06-12T20:16:24Z","receivedAt":"2008-06-12T20:16:24Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Marius Storm-Olsen writes:\n>> Uhm, 'git checkout side file.txt' is not the same file content \n>> (ignoring EOLs please) as 'git show :3:file.txt'. Ref:\n>> user-manual.html#conflict-resolution\n> \n> Bruce, I think the comment in this quoted section is wrong.\n> \n> True, the combined diff can show only the interesting differences\n> between the three, but that is not because we munge stage #2 and #3.\n> They contain verbatim copies from the HEAD and the MERGE_HEAD\n> respectively, and the combining logic runs three-way diffs between\n> the three stages to discard the hunks that the result comes solely\n> from either stage #2 or stage #3.\n...\n> -Since the stage 2 and stage 3 versions have already been updated\n> with -nonconflicting changes, the only remaining differences between\n> them are -the important ones; thus linkgit:git-diff[1] can use the\n> information in -the index to show only those conflicts. +When you ask\n> linkgit:git-diff[1] to show the conflicts, it runs a +three-way diff\n> between the conflicted merge results in the work tree with +stages 2\n> and 3 to show only hunks whose contents come from both sides, +mixed\n> (in other words, when a hunk's merge results come only from stage 2, \n> +that part is not conflicting and is not shown. Same for stage 3).\n\nAah, that certainly clears things up a bit. A good patch I'd say.\n\nHowever, it doesn't change the fact that IMO \"git show\" should respect \ncore.autocrlf, while \"git cat-file\" shouldn't.\n\nI think many would consider\n git show MERGE_HEAD:file.txt > file.txt\nbefore\n git checkout MERGE_HEAD file.txt\nif only because they'd be scared to do a \"checkout\" in the middle of a \nmerge conflict.\n\nPersonally I think the latter is nice, short and sweet, but that doesn't \nmean that it's less scary for people staring out on git. The fact that \nthe two commands above are *not* identical in result, are the kind of \nthings that we need to iron out, to make git more accessible to the \ngeneral public.\n\n--\n.marius\n"},{"id":"79626","messageId":"m3mylqqu05.fsf@localhost.localdomain","threadId":"13872","inReplyTo":"20080612195553.GK13626@fieldses.org","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-12T20:27:15Z","receivedAt":"2008-06-12T20:27:15Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"\"J. Bruce Fields\" writes:\n\n> (Is there any advantage, then, to the :n:filename syntax to a user?\n> Is it useful in any cases when they couldn't use HEAD or MERGE_HEAD\n> instead? If not I might be tempted to cut this bit entirely (or\n> postpone it till later.)\n\nI'm not sure, but I think that while HEAD and MERGE_HEAD vs :n:\ndiffer in the tree represented (in the index trivial / tree conflicts\nare resolved) they have the same file contents for conflicting files.\n\nI think that :n: syntax is just shorter, especially for the ancestor\n(c.f. $(git merge-base HEAD MERGE_HEAD)).\n\nAnd of course there is octopus merge to be considered...\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"79633","messageId":"7vprqmz8kj.fsf@gitster.siamese.dyndns.org","threadId":"13872","inReplyTo":"20080612195553.GK13626@fieldses.org","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-12T20:45:16Z","receivedAt":"2008-06-12T20:45:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"J. Bruce Fields\" writes:\n\n> (Is there any advantage, then, to the :n:filename syntax to a user?\n> Is it useful in any cases when they couldn't use HEAD or MERGE_HEAD\n> instead? If not I might be tempted to cut this bit entirely (or\n> postpone it till later.)\n\nI am somewhat torn between the two.\n\nThis section is only about merge conflicts, so using \"checkout HEAD path\"\nwould be a good substitute. The text flows better that way, because the\nprevious paragraph talks about HEAD and MERGE_HEAD.\n\nWhen people run \"am -3\", however, they may wish that they learned how the\nnotation to name blob objects in the index (e.g. :2:path) can be used to\nexamine and resolve the conflict, as there is no HEAD/MERGE_HEAD in that\nusage context.\n"},{"id":"79636","messageId":"1213303835.5327.24.camel@ld0161-tx32","threadId":"13872","inReplyTo":"7vprqmz8kj.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/2] Respecting core.autocrlf when showing objects","fromName":"Jon Loeliger","fromEmail":"jdl@freescale.com","sentAt":"2008-06-12T20:50:34Z","receivedAt":"2008-06-12T20:50:34Z","isPatch":true,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"On Thu, 2008-06-12 at 13:45 -0700, Junio C Hamano wrote:\n> \"J. Bruce Fields\" writes:\n> \n> > (Is there any advantage, then, to the :n:filename syntax to a user?\n> > Is it useful in any cases when they couldn't use HEAD or MERGE_HEAD\n> > instead? If not I might be tempted to cut this bit entirely (or\n> > postpone it till later.)\n> \n> I am somewhat torn between the two.\n> \n> This section is only about merge conflicts, so using \"checkout HEAD path\"\n> would be a good substitute. The text flows better that way, because the\n> previous paragraph talks about HEAD and MERGE_HEAD.\n> \n> When people run \"am -3\", however, they may wish that they learned how the\n> notation to name blob objects in the index (e.g. :2:path) can be used to\n> examine and resolve the conflict, as there is no HEAD/MERGE_HEAD in that\n> usage context.\n\n\nHi Junio,\n\nI was planning on specifically pointing out the :n: forms as well.\nSo I'm watching this one a bit carefully and would appreciate a\nbit of long-term guidance on the issue here.\n\nThanks,\njdl\n"}]}