[PATCH] bisect: fix replay of CRLF logs

["Christopher Warrington via GitGitGadget" <gitgitgadget@gmail.com>]

From: Christopher Warrington <chwarr@microsoft.com>

Sometimes bisect logs have CRLF newlines. (E.g., if they've been edited
on a Windows machine and their LF-only nature wasn't preserved.)
Previously, such log files would cause odd failures deep in the guts of
git bisect, like "?? what are you talking about?" or "couldn't get the
oid of the rev '...?'" (notice the trailing ?) as each line's CR ends up
part of the final value read from the log.

This commit fixes that by stripping CRs from the log before further
processing.

A regression test that fails without the git-bisect.sh change, "bisect
replay with CRLF log" has been added as well.

Were anyone to intentionally be using terms/revs with embedded CRs,
replaying such bisects will no longer work with this change. I suspect
that this is incredibly rare.

Signed-off-by: Christopher Warrington <chwarr@microsoft.com>
---
    [RFC] bisect: fix replay of CRLF logs
    
    I recently ran into a problem replaying a bisect log that was created
    and edited on a Windows machine. During the editing process, the log's
    LFs were converted to CRLFs, which caused git bisect replay to fail. In
    my particular case, the error from git version 2.26.2.windows.1 was
    "couldn't get the oid of the rev '...?'" (notice the trailing ?).
    
    I was able to reproduce this problem in the current maint branch,
    af6b65d45e (Git 2.26.2, 2020-04-19) on Ubuntu 18.04 as well.
    
    This patch strips any CRs from the log during git bisect replay to avoid
    these issues.
    
    Is this change something that makes sense to add to Git itself?
    
    I've opted to use tr -d '\r' to remove all CRs from the bisect log for
    simplicity, but if there's a need to preserve CRs in the middle of log
    lines, something like this will only remove a CR at the end of the line:
    
    sed $(printf 's/^\\(.*\\)\r$/\\1/') "$file" | while read ...

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-629%2Fchwarr%2Fbisect-replay-crlf-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-629/chwarr/bisect-replay-crlf-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/629

 git-bisect.sh               | 10 ++++++++--
 t/t6030-bisect-porcelain.sh |  7 +++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index efee12b8b1e..8406a9adc36 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -209,7 +209,11 @@ bisect_replay () {
 	test "$#" -eq 1 || die "$(gettext "No logfile given")"
 	test -r "$file" || die "$(eval_gettext "cannot read \$file for replaying")"
 	git bisect--helper --bisect-reset || exit
-	while read git bisect command rev
+
+	# We remove any CR in the input to handle bisect log files that have
+	# CRLF line endings. The assumption is that CR within bisect
+	# commands also don't matter.
+	tr -d '\r' <"$file" | while read git bisect command rev
 	do
 		test "$git $bisect" = "git bisect" || test "$git" = "git-bisect" || continue
 		if test "$git" = "git-bisect"
@@ -231,7 +235,9 @@ bisect_replay () {
 		*)
 			die "$(gettext "?? what are you talking about?")" ;;
 		esac
-	done <"$file"
+	done
+
+	get_terms
 	bisect_auto_next
 }
 
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 821a0c88cf0..72c5dbab278 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -792,6 +792,13 @@ test_expect_success 'bisect replay with old and new' '
 	git bisect reset
 '
 
+test_expect_success 'bisect replay with CRLF log' '
+	awk 1 "ORS=\\r\\n" <log_to_replay.txt >log_to_replay_crlf.txt &&
+	git bisect replay log_to_replay_crlf.txt >bisect_result_crlf &&
+	grep "$HASH2 is the first new commit" bisect_result_crlf &&
+	git bisect reset
+'
+
 test_expect_success 'bisect cannot mix old/new and good/bad' '
 	git bisect start &&
 	git bisect bad $HASH4 &&

base-commit: af6b65d45ef179ed52087e80cb089f6b2349f4ec
-- 
gitgitgadget