Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Am 28.04.2015 um 21:52 schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: I set none of these. But I do commit CRLF and expect to get CRLF back. Am I commiting binary files? Am I doing something that Git does not support? Am I on [my] own? I think these specific sentences are merely uninformed opinions; if I ignore and re-read what people said in the discussion, I think the thread as a whole makes sense. Thanks for the clarification. Following the thread only superficially, I feared some behavior change (or even just a redefinition of what is supported) is about to surface that impacts established workflows. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Hello Hannes, let me correct my previous statement: On Mon, Apr 27, 2015 at 08:58:05PM +0200, Johannes Sixt wrote: When I commit my C source code files with CRLF into the repository (because I do not set any line ending options or configurations or any 'text' attributes or similar), do I then commit binary files or text files? Should I expect not to see any diffs? Of course, you can see diffs. The files are not binary in that sense. Johannes Sixt j...@kdbg.org writes: I set none of these. But I do commit CRLF and expect to get CRLF back. [...] That works. You will not encounter any problem. (Supposing you do not change the line ending options, of course.) Finally, let me explain my previous statement: Am 27.04.2015 um 08:11 schrieb Stepan Kasal: Git does not support CRLF as the internal line separator. I'm often asked: How do I set up git so that it uses CRLF in text files in the repository and checks them out with CRLF on Windows and with LF on unixy systems? My answer to that question always was that you cannot configure the internal line separator in git repo, it is always LF. Your only chance to support both line endings is to have LF in the repo and configure the Windows client to do the conversion. If you commit file in binary mode with CRLF, you are on your own. OK, scratch the word binary. The files in the repo are actually text files. But each text line is contains one more char than you would think. From time to time, this lurks: 1) Does git grep ';$' HEAD find anything? 2) What about git grep ';.$' HEAD ? Or git grep `printf ';\r$' HEAD ? 3) If you try things like git diff HEAD^^..HEAD^ outfile.diff and then open outfile.diff with a suitable editor (e.g. vim), you can see an extra ^M at the end of some lines (the content ones). This is why I tell users that they are on their own if they decide to use the setup you described. Have a nice day, Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Am 27.04.2015 um 21:45 schrieb Torsten Bögershausen: On 04/27/2015 08:58 PM, Johannes Sixt wrote: Am 27.04.2015 um 08:11 schrieb Stepan Kasal: Git does not support CRLF as the internal line separator. If you commit file in binary mode with CRLF, you are on your own. When I commit my C source code files with CRLF into the repository (because I do not set any line ending options or configurations or any 'text' attributes or similar), do I then commit binary files or text files? Should I expect not to see any diffs? -- Hannes You commit files with CRLF in the repo. If you have CRLF in the working tree, things are as follows: core.autocrlf=false : Same as binary, no changes core.autocrlf=true: Normalization is suppressed, (CRLF in repo), and therefore no changes. core.autocrlf=input : Normalization wanted, (CRLF in repo), normalization will be done (and should be committed as soon as possible) I set none of these. But I do commit CRLF and expect to get CRLF back. Am I commiting binary files? Am I doing something that Git does not support? Am I on [my] own? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Torsten Bögershausen tbo...@web.de writes: What do you think about the following test cases for a V2 patch ? test_expect_success 'create blamerepo' ' test_create_repo blamerepo ( cd blamerepo printf testcase\r\n crlffile git -c core.autocrlf=false add crlffile git commit -m add files git -c core.autocrlf=false blame crlffile crlfclean.txt ) ' test_expect_success 'blaming files with CRLF newlines in repo, core.autoclrf=input' ' ( cd blamerepo git -c core.autocrlf=input blame crlffile actual grep Not Committed Yet actual Are you interested in seeing just some of the lines to show up as Not commited yet, or all of them? I think it would be the latter, so perhaps ! grep -v Not Committed Yet actual or something? ) ' Two blank lines only here? test_expect_success 'blaming files with CRLF newlines core.autocrlf=true' ' ( cd blamerepo git -c core.autocrlf=true blame crlffile actual test_cmp crlfclean.txt actual ) ' OK test_expect_success 'blaming files with CRLF newlines core.autocrlf=false' ' ( cd blamerepo git -c core.autocrlf=false blame crlffile actual test_cmp crlfclean.txt actual ) ' Hmm, how's this blame invocation any different from the one done in the set-up step at the very beginning? In other words, I am not sure what kind of breakage could cause this step to fail. I see there is no git blame HEAD crlffile that bypasses the fake latest commit altogether. Wouldn't that be the most appropriate thing to compare against (i.e. how to create crlfclean.txt in the set-up step)? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Johannes Sixt j...@kdbg.org writes: I set none of these. But I do commit CRLF and expect to get CRLF back. Am I commiting binary files? Am I doing something that Git does not support? Am I on [my] own? I think these specific sentences are merely uninformed opinions; if I ignore and re-read what people said in the discussion, I think the thread as a whole makes sense. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
On 28/04/15 09:28, Junio C Hamano wrote: Torsten Bögershausentbo...@web.de writes: What do you think about the following test cases for a V2 patch ? test_expect_success 'create blamerepo' ' test_create_repo blamerepo ( cd blamerepo printf testcase\r\n crlffile git -c core.autocrlf=false add crlffile git commit -m add files git -c core.autocrlf=false blame crlffile crlfclean.txt ) ' test_expect_success 'blaming files with CRLF newlines in repo, core.autoclrf=input' ' ( cd blamerepo git -c core.autocrlf=input blame crlffile actual grep Not Committed Yet actual Are you interested in seeing just some of the lines to show up as Not commited yet, or all of them? I think it would be the latter, so perhaps ! grep -v Not Committed Yet actual or something? ) ' Two blank lines only here? test_expect_success 'blaming files with CRLF newlines core.autocrlf=true' ' ( cd blamerepo git -c core.autocrlf=true blame crlffile actual test_cmp crlfclean.txt actual ) ' OK Interestingly this test doesn't pass on one of my systems, after having stripped t8003 to contain to only have the corner cases. When core.autocrlf is true, the converting should be suppressed: convert.c/has_cr_in_index() should return 1, but doesn't. data = read_blob_data_from_cache(path, sz); and data is NULL. Some more digging has to be done here. On the other hand we want to test blame on a file with LF in the repo and CRLF in the workspace as well. So all in all I need to send a V2. test_expect_success 'blaming files with CRLF newlines core.autocrlf=false' ' ( cd blamerepo git -c core.autocrlf=false blame crlffile actual test_cmp crlfclean.txt actual ) ' Hmm, how's this blame invocation any different from the one done in the set-up step at the very beginning? In other words, I am not sure what kind of breakage could cause this step to fail. I see there is no git blame HEAD crlffile that bypasses the fake latest commit altogether. Wouldn't that be the most appropriate thing to compare against (i.e. how to create crlfclean.txt in the set-up step)? Jepp, There is room for improvements. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
On 04/27/2015 07:47 PM, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: I suspect (I haven't looked very carefully for this round yet to be sure, though) that it may turn out that the commit you are proposing to revert was a misguided attempt to fix a non issue, or to break the behaviour to match a mistaken expectation. If that is the case then definitely the reversion is a good idea, and you should argue along that line of justification. We'd just be fixing an old misguided and bad change in such a case. The original says this: blame: correctly handle files regardless of autocrlf If a file contained CRLF line endings in a repository with core.autocrlf=input, then blame always marked lines as Not Committed Yet, even if they were unmodified. Don't attempt to convert the line endings when creating the fake commit so that blame works correctly regardless of the autocrlf setting. Reported-by: Ephrim Khong dr.kh...@gmail.com Signed-off-by: brian m. carlson sand...@crustytoothpaste.net Signed-off-by: Junio C Hamano gits...@pobox.com But if autocrlf=input, then the end-user expectation is to keep the in-repository data with LF line endings. If your tip-of-the-tree commit incorrectly has CRLF line endings, and if you were going to commit what is in the working tree on top, you would be correcting that mistake by turning the in-repository data into a text file with LF line endings, so Not Committed Yet _is_ the correct behaviour. So I think that the reverting that change is the right thing to do. It really was a change to break the behaviour to match a mistaken expectation, I would have to say. Besides a better commit message (suggestions welcome), What do you think about the following test cases for a V2 patch ? test_expect_success 'create blamerepo' ' test_create_repo blamerepo ( cd blamerepo printf testcase\r\n crlffile git -c core.autocrlf=false add crlffile git commit -m add files git -c core.autocrlf=false blame crlffile crlfclean.txt ) ' test_expect_success 'blaming files with CRLF newlines in repo, core.autoclrf=input' ' ( cd blamerepo git -c core.autocrlf=input blame crlffile actual grep Not Committed Yet actual ) ' test_expect_success 'blaming files with CRLF newlines core.autocrlf=true' ' ( cd blamerepo git -c core.autocrlf=true blame crlffile actual test_cmp crlfclean.txt actual ) ' test_expect_success 'blaming files with CRLF newlines core.autocrlf=false' ' ( cd blamerepo git -c core.autocrlf=false blame crlffile actual test_cmp crlfclean.txt actual ) ' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
On 04/27/2015 08:58 PM, Johannes Sixt wrote: Am 27.04.2015 um 08:11 schrieb Stepan Kasal: Git does not support CRLF as the internal line separator. If you commit file in binary mode with CRLF, you are on your own. When I commit my C source code files with CRLF into the repository (because I do not set any line ending options or configurations or any 'text' attributes or similar), do I then commit binary files or text files? Should I expect not to see any diffs? -- Hannes You commit files with CRLF in the repo. If you have CRLF in the working tree, things are as follows: core.autocrlf=false : Same as binary, no changes core.autocrlf=true: Normalization is suppressed, (CRLF in repo), and therefore no changes. core.autocrlf=input : Normalization wanted, (CRLF in repo), normalization will be done (and should be committed as soon as possible) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Hello, On Sun, Apr 26, 2015 at 10:31:11PM -0700, Junio C Hamano wrote: [...] the commit you are proposing to revert [4d4813a5] was a misguided attempt to fix a non issue, [...] yes, it was this. So I propose to remove the whole commit, including the test case and add two new test cases. Details: Git does not support CRLF as the internal line separator. If you commit file in binary mode with CRLF, you are on your own. If you then recode the file in the working tree to use LF, no wonder things break. If you do it indirectly, by setting the file mode to text, things break exactly the same way. And that is the case that 4d4813a5 wanted to fix, cf the test case in it. OTOH, the commit has broken the most recommended scenario for Windows: LF in the repo, CRLF in the work tree. Thanks, Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
On Mon, Apr 27, 2015 at 10:47:29AM -0700, Junio C Hamano wrote: The original says this: blame: correctly handle files regardless of autocrlf If a file contained CRLF line endings in a repository with core.autocrlf=input, then blame always marked lines as Not Committed Yet, even if they were unmodified. Don't attempt to convert the line endings when creating the fake commit so that blame works correctly regardless of the autocrlf setting. Reported-by: Ephrim Khong dr.kh...@gmail.com Signed-off-by: brian m. carlson sand...@crustytoothpaste.net Signed-off-by: Junio C Hamano gits...@pobox.com But if autocrlf=input, then the end-user expectation is to keep the in-repository data with LF line endings. If your tip-of-the-tree commit incorrectly has CRLF line endings, and if you were going to commit what is in the working tree on top, you would be correcting that mistake by turning the in-repository data into a text file with LF line endings, so Not Committed Yet _is_ the correct behaviour. So I think that the reverting that change is the right thing to do. It really was a change to break the behaviour to match a mistaken expectation, I would have to say. I don't have a strong opinion on whether or not this should be reverted, since I don't use Windows and therefore don't use CRLF or the respective options anywhere, nor am I very familiar with how they are supposed to function. Junio has articulated a good rationale for why it's broken, and I'm willing to go along with that. I will say that perhaps it's worthwhile to write some documentation to explain how the CRLF translation works, as it seems that there's a lot of misunderstanding about it. I am, for the aforementioned reasons, not a good choice to write it. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Junio C Hamano gits...@pobox.com writes: I suspect (I haven't looked very carefully for this round yet to be sure, though) that it may turn out that the commit you are proposing to revert was a misguided attempt to fix a non issue, or to break the behaviour to match a mistaken expectation. If that is the case then definitely the reversion is a good idea, and you should argue along that line of justification. We'd just be fixing an old misguided and bad change in such a case. The original says this: blame: correctly handle files regardless of autocrlf If a file contained CRLF line endings in a repository with core.autocrlf=input, then blame always marked lines as Not Committed Yet, even if they were unmodified. Don't attempt to convert the line endings when creating the fake commit so that blame works correctly regardless of the autocrlf setting. Reported-by: Ephrim Khong dr.kh...@gmail.com Signed-off-by: brian m. carlson sand...@crustytoothpaste.net Signed-off-by: Junio C Hamano gits...@pobox.com But if autocrlf=input, then the end-user expectation is to keep the in-repository data with LF line endings. If your tip-of-the-tree commit incorrectly has CRLF line endings, and if you were going to commit what is in the working tree on top, you would be correcting that mistake by turning the in-repository data into a text file with LF line endings, so Not Committed Yet _is_ the correct behaviour. So I think that the reverting that change is the right thing to do. It really was a change to break the behaviour to match a mistaken expectation, I would have to say. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Am 27.04.2015 um 08:11 schrieb Stepan Kasal: Git does not support CRLF as the internal line separator. If you commit file in binary mode with CRLF, you are on your own. When I commit my C source code files with CRLF into the repository (because I do not set any line ending options or configurations or any 'text' attributes or similar), do I then commit binary files or text files? Should I expect not to see any diffs? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
On Sun, Apr 26, 2015 at 8:02 AM, Torsten Bögershausen tbo...@web.de wrote: A typicall setup under Windows: s/typicall/typical/ core.eol is CRLF and a file is marked as text in .gitattributes. After 4d4813a5 git blame no longer works as expected, every line is annotated as Not Committed Yet, even though the working directory is clean. commit 4d4813a5 removed the conversion in blame.c for all files, with or without CRLF in the repo. Having files with CRLF in the repo and core.autocrlf=input is a temporary situation, the files should be normalized in the repo. Blaming them with Not Committed Yet is OK. The solution is to revert commit 4d4813a5. Reported-By: Stepan Kasal ka...@ucw.cz Signed-off-by: Torsten Bögershausen tbo...@web.de -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Torsten Bögershausen tbo...@web.de writes: Although the intention of 4d4813a5 is good, it breaks the usual EOL-handling for Windows. Until we have a better solution, we suggest to revert it. That makes it sound like you are proposing to rob Peter to pay Paul, but that is not how we do things around here. If both the case 4d4813a5 tried to solve and the issue reported by Stepan need to be satisfied, the current code will stay as-is until you can find a good solution to make both happy. Having said that. I suspect (I haven't looked very carefully for this round yet to be sure, though) that it may turn out that the commit you are proposing to revert was a misguided attempt to fix a non issue, or to break the behaviour to match a mistaken expectation. If that is the case then definitely the reversion is a good idea, and you should argue along that line of justification. We'd just be fixing an old misguided and bad change in such a case. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo
Hello, thank you Torsten for the patch [I'm the reporter, but could not do it myself] -test_expect_success 'blaming files with CRLF newlines' ' +test_expect_failure 'blaming files with CRLF newlines in repo, core.autoclrf=input' ' Shouldn't the old test be rather removed? It deals with an invalid situation. I thought that having crlf in the repo is incorrect, so no wonder that it fails if the files in the working tree are changed to LF. And changing the autocrlf transformation is effectively the same, no matter that the files _physically_ are the same as the files in the repo. Have a nice day, Stepan Kasal -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
