git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: tboegi@web.de
To: git@vger.kernel.org
Cc: peff@peff.net, Johannes.Schindelin@gmx.de,
	"Torsten Bögershausen" <tboegi@web.de>
Subject: [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF
Date: Fri, 12 Aug 2016 18:51:04 +0200	[thread overview]
Message-ID: <1471020664-20784-1-git-send-email-tboegi@web.de> (raw)
In-Reply-To: <20160809114938.pcrvirrzrh6ldmnr@sigill.intra.peff.net>

From: Torsten Bögershausen <tboegi@web.de>

When the "new safer autocrlf-handling" was introduced in c4805393, the logic
around check_safe_crlf() should have been changed:
Once a file has been commited with CRLF (and core.autocrlf is true),
line endings are no longer converted by "git add" or "git checkout".
The line endings will not normalized by Git.
The user may run e.g. dos2unix and commit the file.

Factor out will_convert_lf_to_crlf() which returns when LF are converted
into CRLF at checkout.

Update the logic around check_safe_crlf() and do the possible CRLF-LF
conversion in "git add".
Simulate the checkout (and a possible LF-CRLF conversion) with help of
will_convert_lf_to_crlf().
---
 convert.c            | 97 ++++++++++++++++++++++++++++++----------------------
 t/t0027-auto-crlf.sh |  6 ++--
 2 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/convert.c b/convert.c
index 67d69b5..077f5e6 100644
--- a/convert.c
+++ b/convert.c
@@ -189,33 +189,25 @@ static enum eol output_eol(enum crlf_action crlf_action)
 }
 
 static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
-                            struct text_stat *stats, enum safe_crlf checksafe)
+			    struct text_stat *old_stats, struct text_stat *new_stats,
+			    enum safe_crlf checksafe)
 {
-	if (!checksafe)
-		return;
-
-	if (output_eol(crlf_action) == EOL_LF) {
+	if (old_stats->crlf && !new_stats->crlf ) {
 		/*
-		 * CRLFs would not be restored by checkout:
-		 * check if we'd remove CRLFs
+		 * CRLFs would not be restored by checkout
 		 */
-		if (stats->crlf) {
-			if (checksafe == SAFE_CRLF_WARN)
-				warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
-			else /* i.e. SAFE_CRLF_FAIL */
-				die("CRLF would be replaced by LF in %s.", path);
-		}
-	} else if (output_eol(crlf_action) == EOL_CRLF) {
+		if (checksafe == SAFE_CRLF_WARN)
+			warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
+		else /* i.e. SAFE_CRLF_FAIL */
+			die("CRLF would be replaced by LF in %s.", path);
+	} else if (old_stats->lonelf && !new_stats->lonelf ) {
 		/*
-		 * CRLFs would be added by checkout:
-		 * check if we have "naked" LFs
+		 * CRLFs would be added by checkout
 		 */
-		if (stats->lonelf) {
-			if (checksafe == SAFE_CRLF_WARN)
-				warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
-			else /* i.e. SAFE_CRLF_FAIL */
-				die("LF would be replaced by CRLF in %s", path);
-		}
+		if (checksafe == SAFE_CRLF_WARN)
+			warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
+		else /* i.e. SAFE_CRLF_FAIL */
+			die("LF would be replaced by CRLF in %s", path);
 	}
 }
 
@@ -233,12 +225,35 @@ static int has_cr_in_index(const char *path)
 	return has_cr;
 }
 
+static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
+				   enum crlf_action crlf_action)
+{
+	if (output_eol(crlf_action) != EOL_CRLF)
+		return 0;
+	/* No "naked" LF? Nothing to convert, regardless. */
+	if (!stats->lonelf)
+		return 0;
+
+	if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
+		/* If we have any CR or CRLF line endings, we do not touch it */
+		/* This is the new safer autocrlf-handling */
+		if (stats->lonecr || stats->crlf)
+			return 0;
+
+		if (convert_is_binary(len, stats))
+			return 0;
+	}
+	return 1;
+
+}
+
 static int crlf_to_git(const char *path, const char *src, size_t len,
 		       struct strbuf *buf,
 		       enum crlf_action crlf_action, enum safe_crlf checksafe)
 {
 	struct text_stat stats;
 	char *dst;
+	int convert_crlf_into_lf;
 
 	if (crlf_action == CRLF_BINARY ||
 	    (src && !len))
@@ -252,6 +267,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 		return 1;
 
 	gather_stats(src, len, &stats);
+	/* Optimization: No CRLF? Nothing to convert, regardless. */
+	convert_crlf_into_lf = !!stats.crlf;
 
 	if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
 		if (convert_is_binary(len, &stats))
@@ -263,12 +280,24 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 		if (checksafe == SAFE_CRLF_RENORMALIZE)
 			checksafe = SAFE_CRLF_FALSE;
 		else if (has_cr_in_index(path))
-			return 0;
+			convert_crlf_into_lf = 0;
 	}
-	check_safe_crlf(path, crlf_action, &stats, checksafe);
-
-	/* Optimization: No CRLF? Nothing to convert, regardless. */
-	if (!stats.crlf)
+	if (checksafe && len) {
+		struct text_stat new_stats;
+		memcpy(&new_stats, &stats, sizeof(new_stats));
+		/* simulate "git add" */
+		if (convert_crlf_into_lf) {
+			new_stats.lonelf += new_stats.crlf;
+			new_stats.crlf = 0;
+		}
+		/* simulate "git checkout" */
+		if (will_convert_lf_to_crlf(len, &new_stats, crlf_action)) {
+			new_stats.crlf += new_stats.lonelf;
+			new_stats.lonelf = 0;
+		}
+		check_safe_crlf(path, crlf_action, &stats, &new_stats, checksafe);
+	}
+	if (!convert_crlf_into_lf)
 		return 0;
 
 	/*
@@ -314,21 +343,9 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
 		return 0;
 
 	gather_stats(src, len, &stats);
-
-	/* No "naked" LF? Nothing to convert, regardless. */
-	if (!stats.lonelf)
+	if (!will_convert_lf_to_crlf(len, &stats, crlf_action))
 		return 0;
 
-	if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
-		/* If we have any CR or CRLF line endings, we do not touch it */
-		/* This is the new safer autocrlf-handling */
-		if (stats.lonecr || stats.crlf )
-			return 0;
-
-		if (convert_is_binary(len, &stats))
-			return 0;
-	}
-
 	/* are we "faking" in place editing ? */
 	if (src == buf->buf)
 		to_free = strbuf_detach(buf, NULL);
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index ab6e962..7de8364 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -393,11 +393,11 @@ test_expect_success 'commit files attr=crlf' '
 
 #                 attr                    LF        CRLF      CRLFmixLF   LF_mix_CR   CRLFNUL
 commit_chk_wrnNNO ""      ""      false   ""        ""        ""          ""          ""
-commit_chk_wrnNNO ""      ""      true    ""        ""        ""          ""          ""
+commit_chk_wrnNNO ""      ""      true    LF_CRLF   ""        ""          ""          ""
 commit_chk_wrnNNO ""      ""      input   ""        ""        ""          ""          ""
 
 commit_chk_wrnNNO "auto"  ""      false   ""        ""        ""          ""          ""
-commit_chk_wrnNNO "auto"  ""      true    ""        ""        ""          ""          ""
+commit_chk_wrnNNO "auto"  ""      true    LF_CRLF   ""        ""          ""          ""
 commit_chk_wrnNNO "auto"  ""      input   ""        ""        ""          ""          ""
 for crlf in true false input
 do
@@ -407,7 +407,7 @@ do
 	commit_chk_wrnNNO ""    lf      $crlf   ""       CRLF_LF    CRLF_LF      ""         CRLF_LF
 	commit_chk_wrnNNO ""    crlf    $crlf   LF_CRLF   ""        LF_CRLF     LF_CRLF     ""
 	commit_chk_wrnNNO auto  lf    	$crlf   ""        ""        ""          ""          ""
-	commit_chk_wrnNNO auto  crlf  	$crlf   ""        ""        ""          ""          ""
+	commit_chk_wrnNNO auto  crlf  	$crlf   LF_CRLF  ""        ""          ""          ""
 	commit_chk_wrnNNO text  lf    	$crlf   ""       CRLF_LF    CRLF_LF     ""          CRLF_LF
 	commit_chk_wrnNNO text  crlf  	$crlf   LF_CRLF   ""        LF_CRLF     LF_CRLF     ""
 done
-- 
2.0.0.rc1.6318.g0c2c796


  parent reply	other threads:[~2016-08-12 16:41 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-08 15:05 t0027 racy? Johannes Schindelin
2016-08-08 15:29 ` Jeff King
2016-08-08 20:32   ` Torsten Bögershausen
2016-08-09  6:51     ` Jeff King
2016-08-09  7:03       ` Jeff King
2016-08-09 11:27         ` Johannes Schindelin
2016-08-09 11:33       ` Torsten Bögershausen
2016-08-09 11:49         ` Jeff King
2016-08-09 12:59           ` Torsten Bögershausen
2016-08-09 13:27             ` Jeff King
2016-08-09 21:28               ` Torsten Bögershausen
2016-08-10 12:28                 ` Johannes Schindelin
2016-08-11 18:58                   ` Torsten Bögershausen
2016-08-11 19:34                     ` Junio C Hamano
2016-08-12  7:24                     ` Jeff King
2016-08-12 16:50           ` [PATCH v1 0/2] Fix conversion warnings tboegi
2016-08-12 16:51           ` [PATCH v1 1/2] t0027: Correct raciness in NNO test tboegi
2016-08-12 17:56             ` Junio C Hamano
2016-08-13 16:50             ` Johannes Sixt
2016-08-13 21:18               ` Torsten Bögershausen
2016-08-14 20:37                 ` Junio C Hamano
2016-08-12 16:51           ` tboegi [this message]
2016-08-12 17:52             ` [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF Junio C Hamano
2016-08-13 21:29           ` [PATCH v2 0/1] convert: Correct NNO tests and missing `LF will be replaced by CRLF` tboegi
2016-08-17 12:46             ` Johannes Schindelin
2016-08-13 21:29           ` [PATCH v2 1/1] " tboegi
2016-08-19  9:41           ` [PATCH v1 0/1] Rename NotNormalized (NNO) into CRLF in index tboegi
2016-08-19 16:39             ` Junio C Hamano
2016-08-19  9:41           ` [PATCH v1 1/1] t0027: " tboegi
2016-08-25 15:52           ` [PATCH v1 0/3] Update eol documentation tboegi
2016-08-25 20:31             ` Junio C Hamano
2016-08-26  1:00               ` Jacob Keller
2016-08-26  7:03               ` Torsten Bögershausen
2016-08-25 15:52           ` [PATCH v1 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10 tboegi
2016-08-25 20:38             ` Junio C Hamano
2016-08-25 15:52           ` [PATCH v1 2/2] gitattributes: Document the unified "auto" handling tboegi
2016-08-25 20:46             ` Junio C Hamano
2016-08-26 20:18               ` [PATCH v2 0/2] Adjust the documentation to " tboegi
2016-08-26 20:18               ` [PATCH v2 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10 tboegi
2016-08-26 20:18               ` [PATCH v2 2/2] gitattributes: Document the unified "auto" handling tboegi
2016-08-26 20:53                 ` Junio C Hamano
2016-08-09 11:33   ` t0027 racy? Johannes Schindelin
2016-08-09 11:38     ` Jeff King
2016-08-08 18:24 ` Torsten Bögershausen
2016-08-09 11:25   ` Johannes Schindelin

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=1471020664-20784-1-git-send-email-tboegi@web.de \
    --to=tboegi@web.de \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).