Re: [PATCH] [WIP] safecrlf: Add mechanism to warn about irreversible crlf conversions
From: Dmitry Potapov <hidden>
Date: 2016-06-15 22:44:05
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Sat, Jan 12, 2008 at 06:54:13PM +0100, Steffen Prohaska wrote:
quoted hunk ↗ jump to hunk
diff --git a/convert.c b/convert.c index 4df7559..598cf0b 100644 --- a/convert.c +++ b/convert.c@@ -132,6 +132,27 @@ static int crlf_to_git(const char *path, const char *src, size_t len, *dst++ = c; } while (--len); } + if (safe_crlf) { + if ((action == CRLF_INPUT) || auto_crlf <= 0) { + /* autocrlf=input: check if we removed CRLFs */ + if (buf->len != dst - buf->buf) { + if (safe_crlf == SAFE_CRLF_WARN) + warning("Stripped CRLF from %s.", path); + else + die("Refusing to strip CRLF from %s.", path); + }
This check is okay, however
+ } else {
+ /* autocrlf=true: check if we had LFs (without CR) */
+ if (stats.lf != stats.crlf) {
+ if (safe_crlf == SAFE_CRLF_WARN)
+ warning(
+ "Checkout will replace LFs with CRLF in %s", path);
+ else
+ die("Checkout would replace LFs with CRLF in %s", path);
+ }
+ }this is not, because if you really want to be sure that file will not be mangled by checkout, you should not allow a text file with naked LF when autocrlf=true. And the following lines after gather_stats() can cause: /* No CR? Nothing to convert, regardless. */ if (!stats.cr) return 0; So, I propose a slightly different patch for convert.c:
diff --git a/convert.c b/convert.c
index 4df7559..9fd88d9 100644
--- a/convert.c
+++ b/convert.c@@ -90,9 +90,6 @@ static int crlf_to_git(const char *path, const char *src, size_t len, return 0; gather_stats(src, len, &stats); - /* No CR? Nothing to convert, regardless. */ - if (!stats.cr) - return 0; if (action == CRLF_GUESS) { /*
@@ -108,8 +105,23 @@ static int crlf_to_git(const char *path, const char *src, size_t len, */ if (is_binary(len, &stats)) return 0; + + if (safe_crlf) { + /* check if we have "naked" LFs */ + if (stats.lf != stats.crlf) { + if (safe_crlf == SAFE_CRLF_WARN) + warning( + "Checkout will replace LFs with CRLF in %s", path); + else + die("Checkout would replace LFs with CRLF in %s", path); + } + } } + /* No CR? Nothing to convert, regardless. */ + if (!stats.cr) + return 0; + /* only grow if not in place */ if (strbuf_avail(buf) + buf->len < len) strbuf_grow(buf, len - buf->len);
@@ -131,6 +143,16 @@ static int crlf_to_git(const char *path, const char *src, size_t len, if (! (c == '\r' && (1 < len && *src == '\n'))) *dst++ = c; } while (--len); + + if (safe_crlf && (action == CRLF_INPUT || auto_crlf <= 0)) { + /* autocrlf=input: check if we removed CRLFs */ + if (buf->len != dst - buf->buf) { + if (safe_crlf == SAFE_CRLF_WARN) + warning("Stripped CRLF from %s.", path); + else + die("Refusing to strip CRLF from %s.", path); + } + } } strbuf_setlen(buf, dst - buf->buf); return 1;
Dmitry