Re: [PATCH] revisions --stdin: accept CRLF line terminators

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Sixt <j6t@xxxxxxxx> writes:

> On Windows, 'git rebase -i' with rebase.missingCommitsCheck set to
> warn or error reports:
>
>    Dropped commits (newer to older):
>    'atal: bad revision '410dee56...
>
> The error comes from the git rev-list --stdin invocation in
> git-rebase--interactive.sh (function check_todo_list). It is caused by
> CRs that end up in the file "$todo".miss, because many tools of the MSYS
> toolset force LF to CRLF conversion when regular files are written via
> stdout.
>
> To fix the error, permit CRLF line terminators when revisions and
> pathspec are read using the --stdin option.
>
> Signed-off-by: Johannes Sixt <j6t@xxxxxxxx>
> ---
>  This fixes a new failure in the test suite (t3404.8[67]) on Windows, but
>  I got around to debug it only now.
>
>  revision.c                |  4 ++++
>  t/t6017-rev-list-stdin.sh | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/revision.c b/revision.c
> index cf60c5d..4efedeb 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1641,6 +1641,8 @@ static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
>  		int len = sb->len;
>  		if (len && sb->buf[len - 1] == '\n')
>  			sb->buf[--len] = '\0';
> +		if (len && sb->buf[len - 1] == '\r')
> +			sb->buf[--len] = '\0';
>  		ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
>  		prune->path[prune->nr++] = xstrdup(sb->buf);
>  	}
> @@ -1661,6 +1663,8 @@ static void read_revisions_from_stdin(struct rev_info *revs,
>  		int len = sb.len;
>  		if (len && sb.buf[len - 1] == '\n')
>  			sb.buf[--len] = '\0';
> +		if (len && sb.buf[len - 1] == '\r')
> +			sb.buf[--len] = '\0';

This will strip lone CR at the end of line, in addition to CRLF at
the end of line (which you want to handle) and LF at the end of line
(we always have removed), making it work even on ancient MacOS.

If we really cared, I guess we could write it like this:

	if (len && sb.buf[len - 1] == '\n') {
        	len--;
                if (len && sb.buf[len - 1] == '\r')
                	len--;
		sb.buf[len] = '\0';
	}

but your version should be fine as-is.

Thanks.

> diff --git a/t/t6017-rev-list-stdin.sh b/t/t6017-rev-list-stdin.sh
> index 667b375..34c43cf 100755
> --- a/t/t6017-rev-list-stdin.sh
> +++ b/t/t6017-rev-list-stdin.sh
> @@ -75,4 +75,20 @@ test_expect_success 'not only --stdin' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'accept CRLF line terminators' '
> +	cat >expect <<-\EOF &&
> +	7
> +
> +	file-2
> +	EOF
> +	q_to_cr >input <<-\EOF &&
> +	masterQ
> +	^master^Q
> +	--Q
> +	file-2Q
> +	EOF
> +	git log --pretty=tformat:%s --name-only --stdin <input >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
> -- 
> 2.3.2.245.gb5bf9d3
>
> -- 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]