git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: tboegi@web.de
Cc: git@vger.kernel.org
Subject: Re: [PATCH v1 1/1] t6038; use crlf on all platforms
Date: Tue, 17 May 2016 11:39:35 -0700	[thread overview]
Message-ID: <xmqqa8jowmu0.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1463501398-8608-1-git-send-email-tboegi@web.de> (tboegi@web.de's message of "Tue, 17 May 2016 18:09:58 +0200")

tboegi@web.de writes:

> From: Torsten Bögershausen <tboegi@web.de>
>
> t6038 uses different code, depending if NATIVE_CRLF is set ot not.
> When the native line endings are LF, merge.renormalize is not tested very well.
> Change the test to always use CRLF by setting core.eol=crlf.
> ---
> Broke the 10/10 series into smaller pieces, this is the update of t6038

Thanks.

 - Missing sign-off.

 - "... is not tested very well", which implies "with this change,
   it will be tested", is a secondary benefit.  The reader need to
   agree that, whether the platform native line ending is CRLF or
   LF, 'git merge' should behave identically on any platform, as
   long as the line ending convention used in the repository is
   explicitly set in the same way, before agreeing that this is a
   good thing to do in general.  And that is a bigger benefit, no?

 - But doesn't the same principle apply in the other direction?
   When forced to do core.eol=lf, a platform with NATIVE_CRLF should
   behave identically to how a platform without NATIVE_CRLF would?

   With this patch, we lose test coverage for core.eol=lf case,
   which used to be tested on non-NATIVE_CRLF platforms.  Isn't that
   a concern to us?

>  t/t6038-merge-text-auto.sh | 37 +++++++++++--------------------------
>  1 file changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
> index 85c10b0..4dc8c1a 100755
> --- a/t/t6038-merge-text-auto.sh
> +++ b/t/t6038-merge-text-auto.sh
> @@ -18,6 +18,7 @@ test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
>  
>  test_expect_success setup '
>  	git config core.autocrlf false &&
> +	git config core.eol crlf &&
>  
>  	echo first line | append_cr >file &&
>  	echo first line >control_file &&
> @@ -72,10 +73,8 @@ test_expect_success 'Merge after setting text=auto' '
>  	same line
>  	EOF
>  
> -	if test_have_prereq NATIVE_CRLF; then
> -		append_cr <expected >expected.temp &&
> -		mv expected.temp expected
> -	fi &&
> +	append_cr <expected >expected.temp &&
> +	mv expected.temp expected &&
>  	git config merge.renormalize true &&
>  	git rm -fr . &&
>  	rm -f .gitattributes &&
> @@ -90,10 +89,8 @@ test_expect_success 'Merge addition of text=auto' '
>  	same line
>  	EOF
>  
> -	if test_have_prereq NATIVE_CRLF; then
> -		append_cr <expected >expected.temp &&
> -		mv expected.temp expected
> -	fi &&
> +	append_cr <expected >expected.temp &&
> +	mv expected.temp expected &&
>  	git config merge.renormalize true &&
>  	git rm -fr . &&
>  	rm -f .gitattributes &&
> @@ -104,15 +101,9 @@ test_expect_success 'Merge addition of text=auto' '
>  
>  test_expect_success 'Detect CRLF/LF conflict after setting text=auto' '
>  	echo "<<<<<<<" >expected &&
> -	if test_have_prereq NATIVE_CRLF; then
> -		echo first line | append_cr >>expected &&
> -		echo same line | append_cr >>expected &&
> -		echo ======= | append_cr >>expected
> -	else
> -		echo first line >>expected &&
> -		echo same line >>expected &&
> -		echo ======= >>expected
> -	fi &&
> +	echo first line | append_cr >>expected &&
> +	echo same line | append_cr >>expected &&
> +	echo ======= | append_cr >>expected &&
>  	echo first line | append_cr >>expected &&
>  	echo same line | append_cr >>expected &&
>  	echo ">>>>>>>" >>expected &&
> @@ -128,15 +119,9 @@ test_expect_success 'Detect LF/CRLF conflict from addition of text=auto' '
>  	echo "<<<<<<<" >expected &&
>  	echo first line | append_cr >>expected &&
>  	echo same line | append_cr >>expected &&
> -	if test_have_prereq NATIVE_CRLF; then
> -		echo ======= | append_cr >>expected &&
> -		echo first line | append_cr >>expected &&
> -		echo same line | append_cr >>expected
> -	else
> -		echo ======= >>expected &&
> -		echo first line >>expected &&
> -		echo same line >>expected
> -	fi &&
> +	echo ======= | append_cr >>expected &&
> +	echo first line | append_cr >>expected &&
> +	echo same line | append_cr >>expected &&
>  	echo ">>>>>>>" >>expected &&
>  	git config merge.renormalize false &&
>  	rm -f .gitattributes &&

  reply	other threads:[~2016-05-17 18:39 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-13 13:49 Bug report: Duplicate CRLF rewrite warnings on commit Adam Dinwoodie
2016-05-13 16:43 ` Junio C Hamano
2016-05-14  5:40   ` Torsten Bögershausen
2016-05-14 11:17     ` Adam Dinwoodie
2016-05-13 18:12 ` Jeff King
2016-05-13 19:46   ` Junio C Hamano
2016-05-13 19:53     ` Jeff King
2016-05-15  6:08 ` [PATCH/RFC v1 0/1] Quickfix ?No duplicate " tboegi
2016-05-15  6:08 ` [PATCH/RFC v1 1/1] No " tboegi
2016-05-15  6:15   ` Eric Sunshine
2016-05-15  6:37 ` [PATCH v1 0/3] CRLF-Handling: bug fix around ce_compare_data() tboegi
2016-05-15  6:38 ` [PATCH v1 1/3] t6038; use crlf on all platforms tboegi
2016-05-15  6:42   ` Eric Sunshine
2016-05-15  6:38 ` [PATCH v1 2/3] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-15  6:45   ` Eric Sunshine
2016-05-15  6:38 ` [PATCH v1 3/3] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-15  6:52   ` Eric Sunshine
2016-05-15 22:14   ` Junio C Hamano
2016-05-16 15:51     ` [PATCH v3 0/1] CRLF-Handling: bug fix around ce_compare_data() tboegi
2016-05-16 16:13       ` Junio C Hamano
2016-05-17  4:08         ` Torsten Bögershausen
2016-05-17 16:09           ` [PATCH v1 1/1] t6038; use crlf on all platforms tboegi
2016-05-17 18:39             ` Junio C Hamano [this message]
2016-05-17 16:41           ` [PATCH v4 0/2] CRLF: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-17 16:41           ` [PATCH v4 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-17 16:41           ` [PATCH v4 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-17 18:58             ` Junio C Hamano
2016-05-18  4:26               ` Torsten Bögershausen
2016-05-18 15:10                 ` Torsten Bögershausen
2016-05-19 14:21           ` [PATCH v5 0/2] CRLF: " tboegi
2016-05-19 23:10             ` Junio C Hamano
2016-05-19 14:21           ` [PATCH v5 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-19 14:21           ` [PATCH v5 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-19 23:03             ` Junio C Hamano
2016-05-20 17:12               ` [PATCH v6 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-20 17:12               ` [PATCH v6 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-20 17:46                 ` Junio C Hamano
2016-05-21 10:01               ` [PATCH v7 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-21 10:01               ` [PATCH v7 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-24 18:36                 ` Junio C Hamano
2016-05-16 15:51     ` [PATCH v3 1/1] " tboegi
2016-05-30 17:00     ` [PATCH v1 0/1] t6038-merge-text-auto.sh tboegi
2016-05-30 18:00       ` Junio C Hamano
2016-05-30 18:48         ` Junio C Hamano
2016-05-30 17:00     ` [PATCH v1 1/1] t6038: different eol for "Merge addition of text=auto" tboegi
2016-06-07 15:20     ` [PATCH v2 0/3] unified auto CRLF handling, V2 tboegi
2016-06-07 15:20     ` [PATCH v2 1/3] convert: unify the "auto" handling of CRLF tboegi
2016-06-07 15:20     ` [PATCH v2 2/3] read-cache: factor out get_sha1_from_index() helper tboegi
2016-06-07 15:20     ` [PATCH v2 3/3] Correct ce_compare_data() in a middle of a merge tboegi
2016-05-15 13:02 ` [PATCH v2 0/3] CRLF-Handling: bug fix around ce_compare_data() tboegi
2016-05-15 13:02 ` [PATCH v2 1/3] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-15 13:02 ` [PATCH v2 2/3] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-15 13:02 ` [PATCH v2 3/3] t6038; use crlf on all platforms tboegi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa8jowmu0.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).