{"thread":{"id":"53427","subject":"[PATCH] bisect: fix replay of CRLF logs","startedAt":"2020-05-07T21:29:45Z","lastAt":"2020-05-10T10:55:24Z","messageCount":13,"participants":["Christopher Warrington via GitGitGadget","Eric Sunshine","Jeff King","Junio C Hamano","Christopher Warrington (CHRISTOPHER)","brian m. carlson","Achim Gratz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"397360","messageId":"pull.629.git.1588886980377.gitgitgadget@gmail.com","threadId":"53427","inReplyTo":null,"subject":"[PATCH] bisect: fix replay of CRLF logs","fromName":"Christopher Warrington via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-07T21:29:40Z","receivedAt":"2020-05-07T21:29:45Z","isPatch":true,"sender":{"key":"name:Christopher Warrington","avatar":null},"body":"From: Christopher Warrington \n\nSometimes bisect logs have CRLF newlines. (E.g., if they've been edited\non a Windows machine and their LF-only nature wasn't preserved.)\nPreviously, such log files would cause odd failures deep in the guts of\ngit bisect, like \"?? what are you talking about?\" or \"couldn't get the\noid of the rev '...?'\" (notice the trailing ?) as each line's CR ends up\npart of the final value read from the log.\n\nThis commit fixes that by stripping CRs from the log before further\nprocessing.\n\nA regression test that fails without the git-bisect.sh change, \"bisect\nreplay with CRLF log\" has been added as well.\n\nWere anyone to intentionally be using terms/revs with embedded CRs,\nreplaying such bisects will no longer work with this change. I suspect\nthat this is incredibly rare.\n\nSigned-off-by: Christopher Warrington \n---\n [RFC] bisect: fix replay of CRLF logs\n \n I recently ran into a problem replaying a bisect log that was created\n and edited on a Windows machine. During the editing process, the log's\n LFs were converted to CRLFs, which caused git bisect replay to fail. In\n my particular case, the error from git version 2.26.2.windows.1 was\n \"couldn't get the oid of the rev '...?'\" (notice the trailing ?).\n \n I was able to reproduce this problem in the current maint branch,\n af6b65d45e (Git 2.26.2, 2020-04-19) on Ubuntu 18.04 as well.\n \n This patch strips any CRs from the log during git bisect replay to avoid\n these issues.\n \n Is this change something that makes sense to add to Git itself?\n \n I've opted to use tr -d '\\r' to remove all CRs from the bisect log for\n simplicity, but if there's a need to preserve CRs in the middle of log\n lines, something like this will only remove a CR at the end of the line:\n \n sed $(printf 's/^\\\\(.*\\\\)\\r$/\\\\1/') \"$file\" | while read ...\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-629%2Fchwarr%2Fbisect-replay-crlf-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-629/chwarr/bisect-replay-crlf-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/629\n\n git-bisect.sh | 10 ++++++++--\n t/t6030-bisect-porcelain.sh | 7 +++++++\n 2 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex efee12b8b1e..8406a9adc36 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -209,7 +209,11 @@ bisect_replay () {\n \ttest \"$#\" -eq 1 || die \"$(gettext \"No logfile given\")\"\n \ttest -r \"$file\" || die \"$(eval_gettext \"cannot read \\$file for replaying\")\"\n \tgit bisect--helper --bisect-reset || exit\n-\twhile read git bisect command rev\n+\n+\t# We remove any CR in the input to handle bisect log files that have\n+\t# CRLF line endings. The assumption is that CR within bisect\n+\t# commands also don't matter.\n+\ttr -d '\\r' <\"$file\" | while read git bisect command rev\n \tdo\n \t\ttest \"$git $bisect\" = \"git bisect\" || test \"$git\" = \"git-bisect\" || continue\n \t\tif test \"$git\" = \"git-bisect\"\n@@ -231,7 +235,9 @@ bisect_replay () {\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n \t\tesac\n-\tdone <\"$file\"\n+\tdone\n+\n+\tget_terms\n \tbisect_auto_next\n }\n \ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 821a0c88cf0..72c5dbab278 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -792,6 +792,13 @@ test_expect_success 'bisect replay with old and new' '\n \tgit bisect reset\n '\n \n+test_expect_success 'bisect replay with CRLF log' '\n+\tawk 1 \"ORS=\\\\r\\\\n\" log_to_replay_crlf.txt &&\n+\tgit bisect replay log_to_replay_crlf.txt >bisect_result_crlf &&\n+\tgrep \"$HASH2 is the first new commit\" bisect_result_crlf &&\n+\tgit bisect reset\n+'\n+\n test_expect_success 'bisect cannot mix old/new and good/bad' '\n \tgit bisect start &&\n \tgit bisect bad $HASH4 &&\n\nbase-commit: af6b65d45ef179ed52087e80cb089f6b2349f4ec\n-- \ngitgitgadget\n"},{"id":"397362","messageId":"CAPig+cTbvmGPjR85RupL4DZjzCwjeHxBqnbPg1+8Gd-Xmzf7CA@mail.gmail.com","threadId":"53427","inReplyTo":"pull.629.git.1588886980377.gitgitgadget@gmail.com","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-05-07T22:13:22Z","receivedAt":"2020-05-07T22:13:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 7, 2020 at 5:29 PM Christopher Warrington via GitGitGadget\n wrote:\n> Sometimes bisect logs have CRLF newlines. (E.g., if they've been edited\n> on a Windows machine and their LF-only nature wasn't preserved.)\n> Previously, such log files would cause odd failures deep in the guts of\n> git bisect, like \"?? what are you talking about?\" or \"couldn't get the\n> oid of the rev '...?'\" (notice the trailing ?) as each line's CR ends up\n> part of the final value read from the log.\n>\n> This commit fixes that by stripping CRs from the log before further\n> processing.\n> [...]\n> Signed-off-by: Christopher Warrington \n> ---\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> @@ -209,7 +209,11 @@ bisect_replay () {\n> - while read git bisect command rev\n> +\n> + # We remove any CR in the input to handle bisect log files that have\n> + # CRLF line endings. The assumption is that CR within bisect\n> + # commands also don't matter.\n> + tr -d '\\r' <\"$file\" | while read git bisect command rev\n\nDue to portability concerns, I worry about using '\\r' here. Indeed\nthis would be its first use in this codebase. On the other hand,\n'\\015' is heavily used (at least in the tests), so that would likely\nbe a safer alternative.\n\n> @@ -231,7 +235,9 @@ bisect_replay () {\n> - done <\"$file\"\n> + done\n> +\n> + get_terms\n> bisect_auto_next\n\nWhy the new get_terms() invocation? Is that leftover debugging gunk?\n\n> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> @@ -792,6 +792,13 @@ test_expect_success 'bisect replay with old and new' '\n> +test_expect_success 'bisect replay with CRLF log' '\n> + awk 1 \"ORS=\\\\r\\\\n\" log_to_replay_crlf.txt &&\n\nThis would be the first use of awk's ORS in this codebase, which may\ninvite portability problems. In this codebase, the more typical way to\ndo this is via a combination of 'sed' and 'tr', however, even better\nwould be to take advantage of append_cr() from\nt/test-lib-functions.sh:\n\n appenc_cr log_to_replay_crlf.txt &&\n\n> + git bisect replay log_to_replay_crlf.txt >bisect_result_crlf &&\n> + grep \"$HASH2 is the first new commit\" bisect_result_crlf &&\n> + git bisect reset\n> +'\n"},{"id":"397364","messageId":"20200507222510.GA42822@coredump.intra.peff.net","threadId":"53427","inReplyTo":"pull.629.git.1588886980377.gitgitgadget@gmail.com","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-07T22:25:10Z","receivedAt":"2020-05-07T22:25:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 07, 2020 at 09:29:40PM +0000, Christopher Warrington via GitGitGadget wrote:\n\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index efee12b8b1e..8406a9adc36 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -209,7 +209,11 @@ bisect_replay () {\n> \ttest \"$#\" -eq 1 || die \"$(gettext \"No logfile given\")\"\n> \ttest -r \"$file\" || die \"$(eval_gettext \"cannot read \\$file for replaying\")\"\n> \tgit bisect--helper --bisect-reset || exit\n> -\twhile read git bisect command rev\n> +\n> +\t# We remove any CR in the input to handle bisect log files that have\n> +\t# CRLF line endings. The assumption is that CR within bisect\n> +\t# commands also don't matter.\n> +\ttr -d '\\r' <\"$file\" | while read git bisect command rev\n> \tdo\n> \t\ttest \"$git $bisect\" = \"git bisect\" || test \"$git\" = \"git-bisect\" || continue\n> \t\tif test \"$git\" = \"git-bisect\"\n> @@ -231,7 +235,9 @@ bisect_replay () {\n> \t\t*)\n> \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n> \t\tesac\n> -\tdone <\"$file\"\n> +\tdone\n\nThis puts the while-loop on the right-hand side of a pipe, which means\nthat it's not running in the main shell environment any longer. So any\nvariables set will be lost after the loop ends, any calls to exit will\nonly exit the loop and not the whole script, etc.\n\nIt looks like we might call into bisect_start inside the loop, which\ndoes exit. I didn't trace all the way through its sub-functions to see\nif they set variables.\n\nThe simplest fix is probably to clean up \"$file\" into another tempfile,\nand then read from that.\n\n-Peff\n"},{"id":"397369","messageId":"xmqq5zd72vjp.fsf@gitster.c.googlers.com","threadId":"53427","inReplyTo":"20200507222510.GA42822@coredump.intra.peff.net","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-07T23:07:54Z","receivedAt":"2020-05-07T23:07:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The simplest fix is probably to clean up \"$file\" into another tempfile,\n> and then read from that.\n\nOr just tell the users do not break the log file (or they can keep\nboth halves)?\n"},{"id":"397403","messageId":"20200508130831.GB631018@coredump.intra.peff.net","threadId":"53427","inReplyTo":"xmqq5zd72vjp.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-08T13:08:31Z","receivedAt":"2020-05-08T13:08:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 07, 2020 at 04:07:54PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > The simplest fix is probably to clean up \"$file\" into another tempfile,\n> > and then read from that.\n> \n> Or just tell the users do not break the log file (or they can keep\n> both halves)?\n\nI am OK with that, too. :)\n\n-Peff\n"},{"id":"397407","messageId":"xmqqh7wq1n52.fsf@gitster.c.googlers.com","threadId":"53427","inReplyTo":"20200508130831.GB631018@coredump.intra.peff.net","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-08T15:07:05Z","receivedAt":"2020-05-08T15:07:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, May 07, 2020 at 04:07:54PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > The simplest fix is probably to clean up \"$file\" into another tempfile,\n>> > and then read from that.\n>> \n>> Or just tell the users do not break the log file (or they can keep\n>> both halves)?\n>\n> I am OK with that, too. :)\n\nWell, that was tongue-in-cheek.\n\nThe log is designed to be edited and then run via the shell or fed\nto the \"bisect replay\" subcommand, so if a (wide) class of editors\ntend to \"corrupt\" the edited result in a known and recoverable way,\nwe should deal with it.\n\nReplaying is just setting the refs the logged session should have\nknown about (without checking out the revisions at each step) and\ndoing the final checkout, so it should be a fast operation, and\npenalizing majority of users by paying the cost to dos2unix copy the\nfile \"just in case\" feels somewhat ugly. I wish we were dumb and\nchecked out each and every intermediate steps---then the cost for\nsuch a \"just in case\" clean-up would have been dwarfed in the noise.\n\nI wonder if we can add a CR to IFS so that the parsing logic of each\nline would not even see it?\n\n bisect_replay () {\n file=\"$1\"\n test \"$#\" -eq 1 || die \"$(gettext \"No logfile given\")\"\n test -r \"$file\" || die \"$(eval_gettext \"cannot read \\$file for replaying\")\"\n git bisect--helper --bisect-reset || exit\n+ IFS=\"$IFS$(printf \"\\015\")\"\n while read git bisect command rev\n do\n test \"$git $bisect\" = \"git bisect\" ||\n test \"$git\" = \"git-bisect\" || continue\n\n"},{"id":"397419","messageId":"xmqq8si2z8zb.fsf@gitster.c.googlers.com","threadId":"53427","inReplyTo":"xmqqh7wq1n52.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-08T16:28:56Z","receivedAt":"2020-05-08T16:29:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I wonder if we can add a CR to IFS so that the parsing logic of each\n> line would not even see it?\n\nSo I got curious and tried this; it seems to pass Christopher's test\n(corrected with Eric's suggestion).\n\nAs the implementation changed, I ended up rewriting some parts of\nthe log message originally proposed and here is what I tentatively\nqueued.\n\n-- >8 --\nFrom: Christopher Warrington \nSubject: [PATCH] bisect: allow CRLF line endings in \"git bisect replay\" input\n\nWe advertise that the bisect log can be corrected in your editor\nbefore being fed to \"git bisect replay\", but some editors may\nturn the line endings to CRLF.\n\nUpdate the parser of the input lines so that the CR at the end of\nthe line gets ignored.\n\nWere anyone to intentionally be using terms/revs with embedded CRs,\nreplaying such bisects will no longer work with this change. I suspect\nthat this is incredibly rare.\n\nSigned-off-by: Christopher Warrington \nSigned-off-by: Junio C Hamano \n---\n git-bisect.sh | 2 ++\n t/t6030-bisect-porcelain.sh | 7 +++++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex efee12b8b1..56548d4be7 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -209,6 +209,7 @@ bisect_replay () {\n \ttest \"$#\" -eq 1 || die \"$(gettext \"No logfile given\")\"\n \ttest -r \"$file\" || die \"$(eval_gettext \"cannot read \\$file for replaying\")\"\n \tgit bisect--helper --bisect-reset || exit\n+\toIFS=\"$IFS\" IFS=\"$IFS:$(printf '\\015')\"\n \twhile read git bisect command rev\n \tdo\n \t\ttest \"$git $bisect\" = \"git bisect\" || test \"$git\" = \"git-bisect\" || continue\n@@ -232,6 +233,7 @@ bisect_replay () {\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n \t\tesac\n \tdone <\"$file\"\n+\tIFS=\"$oIFS\"\n \tbisect_auto_next\n }\n \ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 821a0c88cf..bb84c8a411 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -792,6 +792,13 @@ test_expect_success 'bisect replay with old and new' '\n \tgit bisect reset\n '\n \n+test_expect_success 'bisect replay with CRLF log' '\n+\tappend_cr log_to_replay_crlf.txt &&\n+\tgit bisect replay log_to_replay_crlf.txt >bisect_result_crlf &&\n+\tgrep \"$HASH2 is the first new commit\" bisect_result_crlf &&\n+\tgit bisect reset\n+'\n+\n test_expect_success 'bisect cannot mix old/new and good/bad' '\n \tgit bisect start &&\n \tgit bisect bad $HASH4 &&\n-- \n2.26.2-561-g07d8ea56f2\n\n"},{"id":"397423","messageId":"20200508171232.GA637136@coredump.intra.peff.net","threadId":"53427","inReplyTo":"xmqq8si2z8zb.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-08T17:12:32Z","receivedAt":"2020-05-08T17:12:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 08, 2020 at 09:28:56AM -0700, Junio C Hamano wrote:\n\n> -- >8 --\n> From: Christopher Warrington \n> Subject: [PATCH] bisect: allow CRLF line endings in \"git bisect replay\" input\n> \n> We advertise that the bisect log can be corrected in your editor\n> before being fed to \"git bisect replay\", but some editors may\n> turn the line endings to CRLF.\n> \n> Update the parser of the input lines so that the CR at the end of\n> the line gets ignored.\n\nI'm a little surprised that bash \"read\" on Windows doesn't eat CRLFs\nalready. But I often find myself confused by line ending decisions in\ngeneral, as well as the difference between cygwin versus msys versus\npure windows binaries, etc.\n\nAt any rate, munging IFS seems much nicer than having an extra call to\ntr.\n\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index efee12b8b1..56548d4be7 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -209,6 +209,7 @@ bisect_replay () {\n> \ttest \"$#\" -eq 1 || die \"$(gettext \"No logfile given\")\"\n> \ttest -r \"$file\" || die \"$(eval_gettext \"cannot read \\$file for replaying\")\"\n> \tgit bisect--helper --bisect-reset || exit\n> +\toIFS=\"$IFS\" IFS=\"$IFS:$(printf '\\015')\"\n\nThere's no \":\" separator in IFS, so here you're treating colon as\nend-of-line. I think you just want:\n\n IFS=\"$IFS$(printf '\\015')\"\n\n-Peff\n"},{"id":"397430","messageId":"xmqqv9l6xqhf.fsf@gitster.c.googlers.com","threadId":"53427","inReplyTo":"20200508171232.GA637136@coredump.intra.peff.net","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-08T17:53:48Z","receivedAt":"2020-05-08T17:53:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> +\toIFS=\"$IFS\" IFS=\"$IFS:$(printf '\\015')\"\n>\n> There's no \":\" separator in IFS, so here you're treating colon as\n> end-of-line. I think you just want:\n>\n> IFS=\"$IFS$(printf '\\015')\"\n\nYup. Thanks for spotting ;-)\n"},{"id":"397442","messageId":"SN6PR00MB044634979E0EA55CCAAF8B469BA20@SN6PR00MB0446.namprd00.prod.outlook.com","threadId":"53427","inReplyTo":"xmqq8si2z8zb.fsf@gitster.c.googlers.com","subject":"RE: [EXTERNAL] Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Christopher Warrington (CHRISTOPHER)","fromEmail":"christopher.warrington@microsoft.com","sentAt":"2020-05-08T22:59:19Z","receivedAt":"2020-05-08T22:59:33Z","isPatch":true,"sender":{"key":"christopher.warrington@microsoft.com","avatar":null},"body":"On 2020-05-08 09:31-07:00, Junio C Hamano wrote:\n\n>> I wonder if we can add a CR to IFS so that the parsing logic of each line\n>> would not even see it?\n\n> So I got curious and tried this; it seems to pass Christopher's test\n> (corrected with Eric's suggestion).\n\n> As the implementation changed, I ended up rewriting some parts of the log\n> message originally proposed and here is what I tentatively queued.\n\nThis approach is much cleaner. Thank you, Eric, Junio, and Peff.\n\nI can confirm 6c722cbe5a (bisect: allow CRLF line endings in \"git bisect\nreplay\" input, 2020-05-07) works on a CRLFed bisect log when I apply it to\ngit version 2.26.2.windows.1.\n\n-- \nChristopher Warrington \nMicrosoft Corp.\n\n(Apologies about the [EXTERNAL] in the subject. It gets added on the way in,\nand if I remove it, the References: and In-Reply-To: headers get \"helpfully\"\nstripped out.)\n"},{"id":"397456","messageId":"xmqqblmxvzs7.fsf@gitster.c.googlers.com","threadId":"53427","inReplyTo":"SN6PR00MB044634979E0EA55CCAAF8B469BA20@SN6PR00MB0446.namprd00.prod.outlook.com","subject":"Re: [EXTERNAL] Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-09T16:28:08Z","receivedAt":"2020-05-09T16:28:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Christopher Warrington (CHRISTOPHER)\"\n writes:\n\n> This approach is much cleaner. Thank you, Eric, Junio, and Peff.\n>\n> I can confirm 6c722cbe5a (bisect: allow CRLF line endings in \"git bisect\n> replay\" input, 2020-05-07) works on a CRLFed bisect log when I apply it to\n> git version 2.26.2.windows.1.\n\nThanks.\n"},{"id":"397474","messageId":"20200509221730.GG7234@camp.crustytoothpaste.net","threadId":"53427","inReplyTo":"20200508171232.GA637136@coredump.intra.peff.net","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-05-09T22:17:30Z","receivedAt":"2020-05-09T22:17:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-05-08 at 17:12:32, Jeff King wrote:\n> On Fri, May 08, 2020 at 09:28:56AM -0700, Junio C Hamano wrote:\n> \n> > -- >8 --\n> > From: Christopher Warrington \n> > Subject: [PATCH] bisect: allow CRLF line endings in \"git bisect replay\" input\n> > \n> > We advertise that the bisect log can be corrected in your editor\n> > before being fed to \"git bisect replay\", but some editors may\n> > turn the line endings to CRLF.\n> > \n> > Update the parser of the input lines so that the CR at the end of\n> > the line gets ignored.\n> \n> I'm a little surprised that bash \"read\" on Windows doesn't eat CRLFs\n> already. But I often find myself confused by line ending decisions in\n> general, as well as the difference between cygwin versus msys versus\n> pure windows binaries, etc.\n\nI was surprised by that as well, but I believe at least the bash in Git\nfor Windows is LF-only and doesn't do anything special with CR. In fact,\nISTR it chokes on shell scripts with CR line endings just as every shell\non Unix does. The commits from Dscho and Stolee to our current\n.gitattributes file explain the situation quite well.\n\nCygwin allows configurable behavior for line endings, but I don't know\nwhether any configuration it allows handles CR in the shell. I'm sure\nCygwin will do the right thing with LF only lines regardless, so it's\nprobably safe to just assume LF-only behavior in the shell.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"397492","messageId":"87tv0o5abw.fsf@Rainer.invalid","threadId":"53427","inReplyTo":"20200509221730.GG7234@camp.crustytoothpaste.net","subject":"Re: [PATCH] bisect: fix replay of CRLF logs","fromName":"Achim Gratz","fromEmail":"stromeko@nexgo.de","sentAt":"2020-05-10T10:54:43Z","receivedAt":"2020-05-10T10:55:24Z","isPatch":true,"sender":{"key":"stromeko@nexgo.de","avatar":null},"body":"brian m. carlson writes:\n> Cygwin allows configurable behavior for line endings, but I don't know\n\nThat's restricted to handling files in text mode based on a mount\noption.\n\nhttps://cygwin.com/faq.html#faq.api.cr-lf\n\n> whether any configuration it allows handles CR in the shell. I'm sure\n> Cygwin will do the right thing with LF only lines regardless, so it's\n> probably safe to just assume LF-only behavior in the shell.\n\nA handful of tools had patches to recognize and deal with CRLF in input,\nbut most of these patches have been dropped a long time ago. The better\nassumption is that Cygwin behaves like Linux, so CR only input/output on\ntext.\n\n\nRegards,\nAchim.\n-- \n+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+\n\nSD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:\nhttp://Synth.Stromeko.net/Downloads.html#WaldorfSDada\n\n"}]}