Re: [PATCH 1/1] cvsexportcommit: force crlf translation

From: Junio C Hamano <gitster@pobox.com>
Date: May 7, 2019, 09:22 UTC
Message-ID: <xmqqtve6lito.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <9b508af11be125ce5fd80c40c732dbcaf45067fb.1556575126.git.gitgitgadget@gmail.com>

"Dustin Spicuzza via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Dustin Spicuzza <dustin@virtualroadside.com>
>
> When using cvsnt + msys + git, it seems like the output of cvs status
> had \r\n in it, and caused the command to fail.

This is a bit under-explained in that it does not make it clear where the right place to fix would be. From "X did Y which caused the command to fail", a possible right fix could be "so fix it by telling X not to do Y", but of course a patch to fix cvsnt won't come to this list ;-)

I haven't thought things through, so let's think it aloud and enumerate what should have been explained in the log message here.

 - With binmode(":crlf"), (i.e. if the platform uses CRLF convert
   external CRLF into internal '\n'), the change hopes not to affect
   platforms that do not use CRLF.

 - "it SEEMS LIKE the output of cvs status had CRLF in it", i.e. it
   is uncertain if everybody on the platform has the same issue.
   But by using binmode(":crlf"), if some other implementations of
   "cvs status" on the platform used LF, they won't get negatively
   affected by this change, either.

So, I guess the change is safe enough that it does not hurt other people.

Show 21 quoted lines

>
> This fixes that.
>
> Signed-off-by: Dustin Spicuzza <dustin@virtualroadside.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  git-cvsexportcommit.perl | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
> index d13f02da95..fc00d5946a 100755
> --- a/git-cvsexportcommit.perl
> +++ b/git-cvsexportcommit.perl
> @@ -431,6 +431,7 @@ END
>  sub safe_pipe_capture {
>      my @output;
>      if (my $pid = open my $child, '-|') {
> +	binmode($child, ":crlf");
>  	@output = (<$child>);
>  	close $child or die join(' ',@_).": $! $?";
>      } else {