From: Steffen Prohaska <prohaska@zib.de>
To: torvalds@linux-foundation.org, dpotapov@gmail.com, git@vger.kernel.org
Cc: Steffen Prohaska <prohaska@zib.de>
Subject: [PATCH] [WIP] safecrlf: Add mechanism to warn about irreversible crlf conversions
Date: Sat, 12 Jan 2008 18:54:13 +0100 [thread overview]
Message-ID: <12001604531066-git-send-email-prohaska@zib.de> (raw)
In-Reply-To: <alpine.LFD.1.00.0801111103420.3148@woody.linux-foundation.org>
I promised to think about the CRLF discussion and here is what
I believe we could do:
- Leave the current core.autocrlf mechanism as is.
- Add a mechanism to warn the user if an irreversible conversion happens
- After we have the mechanisms for configuring the conversion and for
configuring the safety level, we can decide which defaults to use on
the different platforms, namely Windows and Unix.
I propose to set the following defaults:
- Unix: core.autocrlf=input, core.safecrlf=warn
- Windows: core.autocrlf=true, core.safecrlf=warn
This patch is declared as WIP because tests and a documentation are missing.
I'm also not sure if calling warning() and die() is the right thing to do at
this place. Interestingly, in some (all?) cases, crlf_to_git() is called two
times for a path during git add, resulting in the warning printed two times. I
didn't yet analyze why this happens. Maybe the the warnings and errors printed
should be more verbose?
[ Linus, Dimitry was right about stats.lf. ]
Steffen
---- snip snap ---
CRLF conversion bears a slight chance of corrupting data.
autocrlf=true will convert CRLF to LF during commit and LF to
CRLF during checkout. A file that containes a mixture of LF and
CRLF before the commit cannot be recreated by git. For text
files this does not really matter because we do not care about
the line endings anyway; but for binary files that are
accidentally classified as text the conversion can result in
corrupted data.
If you recognize such corruption during commit you can easily fix
it by setting the conversion type explicitly in .gitattributes.
Right after committing you still have the original file in your
work tree and this file is not yet corrupted.
However, in mixed Windows/Unix environments text files quite
easily can end up containing a mixture of CRLF and LF line
endings and git should handle such situations gracefully. For
example a user could copy a CRLF file from Windows to Unix and
mix it with an existing LF file there. The result would contain
both types of line endings.
Unfortunately, the desired effect of cleaning up text files
with mixed lineendings and undesired effect of corrupting binary
files can not be distinguished. In both cases CRLF are removed
in an irreversible way. For text files this is the right thing
to do, while for binary file its corrupting data.
In a sane environment committing and checking out the same file
should not modify the origin file in the work tree. For
autocrlf=input the original file must not contain CRLF. For
autocrlf=true the original file must not contain LF without
preceding CR. Otherwise the conversion is irreversible. Note,
git might be able to recreate the original file with different
autocrlf settings, but in the current environment checking out
will yield a file that differs from the file before the commit.
This patch adds a mechanism that can either warn the user about
an irreversible conversion or can even refuse to convert. The
mechanism is controlled by the variable core.safecrlf, with the
following values
- false: disable safecrlf mechanism
- warn: warn about irreversible conversions
- true: refuse irreversible conversions
The default is to warn.
A concept of a safety check was originally proposed in a similar
way by Linus Torvalds.
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
cache.h | 8 ++++++++
config.c | 9 +++++++++
convert.c | 21 +++++++++++++++++++++
environment.c | 1 +
4 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/cache.h b/cache.h
index 39331c2..4e03e3d 100644
--- a/cache.h
+++ b/cache.h
@@ -330,6 +330,14 @@ extern size_t packed_git_limit;
extern size_t delta_base_cache_limit;
extern int auto_crlf;
+enum safe_crlf {
+ SAFE_CRLF_FALSE = 0,
+ SAFE_CRLF_FAIL = 1,
+ SAFE_CRLF_WARN = 2,
+};
+
+extern enum safe_crlf safe_crlf;
+
#define GIT_REPO_VERSION 0
extern int repository_format_version;
extern int check_repository_format(void);
diff --git a/config.c b/config.c
index 857deb6..0a46046 100644
--- a/config.c
+++ b/config.c
@@ -407,6 +407,15 @@ int git_default_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.safecrlf")) {
+ if (value && !strcasecmp(value, "warn")) {
+ safe_crlf = SAFE_CRLF_WARN;
+ return 0;
+ }
+ safe_crlf = git_config_bool(var, value);
+ return 0;
+ }
+
if (!strcmp(var, "user.name")) {
strlcpy(git_default_name, value, sizeof(git_default_name));
return 0;
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);
+ }
+ } 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);
+ }
+ }
+ }
+
strbuf_setlen(buf, dst - buf->buf);
return 1;
}
diff --git a/environment.c b/environment.c
index 18a1c4e..e351e99 100644
--- a/environment.c
+++ b/environment.c
@@ -35,6 +35,7 @@ int pager_use_color = 1;
char *editor_program;
char *excludes_file;
int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */
+enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
/* This is set by setup_git_dir_gently() and/or git_default_config() */
--
1.5.4.rc2.60.g46ee
next prev parent reply other threads:[~2008-01-12 17:55 UTC|newest]
Thread overview: 113+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-07 9:16 CRLF problems with Git on Win32 Peter Karlsson
2008-01-07 9:57 ` Steffen Prohaska
2008-01-07 10:00 ` Junio C Hamano
2008-01-07 12:15 ` Steffen Prohaska
2008-01-07 10:12 ` Jeff King
2008-01-07 18:47 ` Robin Rosenberg
2008-01-07 19:16 ` Johannes Schindelin
2008-01-07 21:03 ` Robin Rosenberg
2008-01-07 21:18 ` Johannes Schindelin
2008-01-07 21:40 ` Steffen Prohaska
[not found] ` <3B08AC4C-A807-4155-8AD7-DC6A6D0FE134-wjoc1KHpMeg@public.gmane.org>
2008-01-07 22:06 ` Junio C Hamano
[not found] ` <7vzlvhxpda.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>
2008-01-07 22:58 ` Linus Torvalds
[not found] ` <alpine.LFD.1.00.0801071457040.3148-5CScLwifNT1QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>
2008-01-07 23:46 ` Gregory Jefferis
2008-01-08 11:09 ` git and unicode Gonzalo Garramuño
2008-01-08 15:09 ` Remi Vanicat
2008-01-08 20:36 ` Robin Rosenberg
2008-01-08 8:55 ` CRLF problems with Git on Win32 Marius Storm-Olsen
2008-01-08 7:02 ` Steffen Prohaska
2008-01-08 7:29 ` Junio C Hamano
2008-01-08 10:08 ` Jeff King
2008-01-08 10:35 ` Junio C Hamano
2008-01-08 12:20 ` Gregory Jefferis
2008-01-08 17:29 ` J. Bruce Fields
[not found] ` <20080108172957.GG22155-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2008-01-08 17:56 ` Steffen Prohaska
2008-01-08 18:07 ` Junio C Hamano
2008-01-08 18:07 ` Junio C Hamano
[not found] ` <7vmyrgry20.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>
2008-01-08 18:58 ` Steffen Prohaska
2008-01-08 19:09 ` J. Bruce Fields
2008-01-08 19:47 ` Junio C Hamano
[not found] ` <7vir24rtfp.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>
2008-01-08 20:02 ` Steffen Prohaska
[not found] ` <B655B6FF-9377-434A-A979-2E758771B0FA-wjoc1KHpMeg@public.gmane.org>
2008-01-08 20:15 ` Junio C Hamano
[not found] ` <7v3at8rs4b.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>
2008-01-08 20:39 ` Steffen Prohaska
2008-01-09 11:03 ` Johannes Schindelin
[not found] ` <alpine.LSU.1.00.0801091100401.31053-OGWIkrnhIhzN0uC3ymp8PA@public.gmane.org>
2008-01-09 12:45 ` Steffen Prohaska
[not found] ` <019B1C82-27BF-4B6B-981D-5498D31B5DD3-wjoc1KHpMeg@public.gmane.org>
2008-01-09 13:32 ` Johannes Schindelin
2008-01-08 20:41 ` Linus Torvalds
2008-01-09 8:03 ` Junio C Hamano
[not found] ` <7vd4sbmnmz.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>
2008-01-09 10:48 ` Johannes Schindelin
2008-01-09 20:25 ` Junio C Hamano
[not found] ` <7vmyrehhkd.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>
2008-01-09 20:50 ` Johannes Schindelin
[not found] ` <alpine.LSU.1.00.0801092047190. 31053@racer.site>
[not found] ` <alpine.LSU.1.00.0801092047190.31053-OGWIkrnhIhzN0uC3ymp8PA@public.gmane.org>
2008-01-09 21:03 ` Steffen Prohaska
[not found] ` <alpine.LSU.1.00.0801091041570.31053-OGWIkrnhIhzN0uC3ymp8PA@public.gmane.org>
2008-01-10 9:25 ` Peter Karlsson
[not found] ` <Pine.LNX.4.64.0801101023380.11922-Hh8n7enkEC8qi7mQTfpNuw@public.gmane.org>
2008-01-10 11:57 ` Johannes Schindelin
2008-01-11 3:03 ` Miles Bader
2008-01-11 3:03 ` Miles Bader
[not found] ` <alpine.LSU.1.00.080110115 5140.31053@racer.site>
[not found] ` <alpine.LSU.1.00.0801101155140.31053-OGWIkrnhIhzN0uC3ymp8PA@public.gmane.org>
2008-01-10 13:28 ` Peter Karlsson
2008-01-10 14:31 ` Peter Harris
[not found] ` <eaa105840801100631p6b95ed86j153d70244d474b03-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-01-11 13:12 ` Peter Karlsson
2008-01-11 15:39 ` Peter Harris
2008-01-08 19:59 ` Steffen Prohaska
2008-01-08 20:11 ` Junio C Hamano
[not found] ` <7vbq7wrsb6.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>
2008-01-08 20:20 ` Steffen Prohaska
2008-01-08 20:50 ` Dmitry Potapov
[not found] ` <20080108205054.GN6951-EQL4cN526mwi5CQI31g/s0B+6BGkLq7r@public.gmane.org>
2008-01-08 21:15 ` Junio C Hamano
2008-01-08 21:57 ` Robin Rosenberg
2008-01-08 21:31 ` Linus Torvalds
[not found] ` <alpine.LFD.1.00.0801081325010.3148-5CScLwifNT1QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>
2008-01-08 22:09 ` Sean
2008-01-08 22:51 ` Dmitry Potapov
[not found] ` <20080108225138.GA23240-EQL4cN526mwi5CQI31g/s0B+6BGkLq7r@public.gmane.org>
2008-01-09 0:01 ` Linus Torvalds
2008-01-09 8:43 ` Abdelrazak Younes
2008-01-10 19:58 ` Gregory Jefferis
2008-01-10 20:20 ` Linus Torvalds
2008-01-10 21:28 ` Gregory Jefferis
2008-01-10 23:23 ` Dmitry Potapov
2008-01-11 0:02 ` Linus Torvalds
2008-01-11 0:32 ` Junio C Hamano
2008-01-11 7:10 ` Steffen Prohaska
2008-01-11 15:58 ` Linus Torvalds
2008-01-11 16:28 ` Steffen Prohaska
2008-01-11 17:25 ` Linus Torvalds
2008-01-11 17:56 ` Steffen Prohaska
2008-01-11 18:10 ` Linus Torvalds
2008-01-11 18:29 ` Steffen Prohaska
2008-01-11 19:16 ` Linus Torvalds
2008-01-11 19:50 ` Sam Ravnborg
2008-01-11 21:18 ` Johannes Schindelin
2008-01-11 22:21 ` Sam Ravnborg
2008-01-12 15:08 ` Dmitry Potapov
2008-01-12 17:54 ` Steffen Prohaska [this message]
2008-01-12 19:14 ` [PATCH] [WIP] safecrlf: Add mechanism to warn about irreversible crlf conversions Dmitry Potapov
2008-01-13 9:05 ` [WIP v2] " Steffen Prohaska
2008-01-11 19:53 ` CRLF problems with Git on Win32 Christer Weinigel
2008-01-14 9:41 ` David Kågedal
2008-01-11 19:00 ` Gregory Jefferis
2008-01-12 15:26 ` Dmitry Potapov
2008-01-10 20:50 ` Rogan Dawes
2008-01-10 21:15 ` Gregory Jefferis
2008-01-11 1:15 ` Junio C Hamano
2008-01-07 21:36 ` Linus Torvalds
2008-01-08 21:26 ` Peter Karlsson
2008-01-09 10:56 ` Johannes Schindelin
2008-01-09 12:41 ` Steffen Prohaska
2008-01-09 13:52 ` Gregory Jefferis
2008-01-09 14:03 ` Johannes Schindelin
2008-01-09 15:22 ` Dmitry Potapov
2008-01-09 15:03 ` Dmitry Potapov
2008-01-09 17:37 ` Gregory Jefferis
2008-01-09 19:05 ` Dmitry Potapov
2008-01-07 21:42 ` Thomas Neumann
2008-01-08 10:56 ` Peter Karlsson
2008-01-08 11:07 ` Jeff King
2008-01-08 11:54 ` Johannes Schindelin
2008-01-08 11:52 ` Johannes Schindelin
2008-01-08 13:07 ` Peter Harris
2008-01-08 15:20 ` Peter Karlsson
2008-01-08 15:58 ` Kelvie Wong
2008-01-08 21:33 ` Dmitry Potapov
2008-01-09 18:46 ` Jan Hudec
2008-01-07 10:13 ` Peter Klavins
2008-01-07 12:58 ` Steffen Prohaska
2008-01-07 13:50 ` Peter Karlsson
2008-01-07 14:14 ` Peter Klavins
2008-01-07 16:05 ` Steffen Prohaska
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=12001604531066-git-send-email-prohaska@zib.de \
--to=prohaska@zib.de \
--cc=dpotapov@gmail.com \
--cc=git@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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.
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 NNTP newsgroup(s).