Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 26, 2015, 20:07 UTC
- Message-ID
- <xmqqbnblgzrk.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <xmqq8u6piir8.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 16 quoted lines
> Something along the line of the following would be tolerable, even > though I think in the longer term, not just in Git land but in the > larger ecosystem to use POSIXy tools on Windows, the best solution > is to fix the shell so that it matches the expectation of the users > of its platform. > > I say "something along the line of" here because I do not know how > the problematic shell behaves on the assignment command that stuffs > a lone CR into a variable. It _might_ need a similar protection > against the shell feature "the last EOL is removed from the result > of command expansion", as I did in the above example, depending on > how incoherent the implementation is. The implementation seems to > accept CRLF and LF in shell scripts proper just fine, but apparently > its implementation of "read" does not honor such platform EOL > convention, which caused this issue, and I don't know what it does > in the codepath that implements command expansion.
I still have to say "something along the lines of" as I do not have (and I do not wish to have, to be honest) an access to test this with the problematic shell implementation, but here is an updated patch.
-- >8 -- Subject: [PATCH] rebase-i: work around Windows CRLF line endings
Editors on Windows can and do save text files with CRLF line endings, which is the convention on the platform. We are seeing reports that the "read" command in a port of bash to the environment however does not strip the CRLF at the end, not adjusting for the same convention on the platform.
This breaks the recently added sanity checks for the insn sheet fed to "rebase -i"; instead of an empty line (hence nothing in $command), the script was getting a lone CR in there.
Special case a lone CR and treat it the same way as an empty line to work this around.
The test was stolen from Dscho.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- git-rebase--interactive.sh | 13 +++++++++++++ t/t3404-rebase-interactive.sh | 12 ++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index c42ba34..3911711 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -77,6 +77,10 @@ amend="$state_dir"/amend rewritten_list="$state_dir"/rewritten-list rewritten_pending="$state_dir"/rewritten-pending +# Work around a Windows port of shell that does not strip +# the newline at the end of a line correctly. +cr=$(printf "\015") + strategy_args= if test -n "$do_merge" then @@ -518,6 +522,11 @@ do_next () { "$comment_char"*|''|noop|drop|d) mark_action_done ;; + "$cr") + # Windows port of shell not stripping the newline + # at the end of an empty line correctly. + mark_action_done + ;; pick|p) comment_for_reflog pick @@ -888,6 +897,10 @@ check_bad_cmd_and_sha () { "$comment_char"*|''|noop|x|exec) # Doesn't expect a SHA-1 ;; + "$cr") + # Windows port of shell not stripping the newline + # at the end of an empty line correctly. + ;; pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f) if ! check_commit_sha "${rest%%[ ]*}" "$lineno" "$1" then diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 88d7d53..2f97a3d 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1240,4 +1240,16 @@ test_expect_success 'static check of bad SHA-1' ' test E = $(git cat-file commit HEAD | sed -ne \$p) ' +test_expect_success 'editor saves as CR/LF' ' + git checkout -b with-crlf && + write_script add-crs.sh <<-\EOF && + sed -e "s/\$/Q/" <"$1" | tr Q "\\015" >"$1".new && + mv -f "$1".new "$1" + EOF + ( + test_set_editor "$(pwd)/add-crs.sh" && + git rebase -i HEAD^ + ) +' + test_done
-- 2.6.2-405-g6877da6