Merge branch 'sp/safecrlf'
authorJunio C Hamano <gitster@pobox.com>
Sun, 17 Feb 2008 01:59:20 +0000 (17:59 -0800)
committerJunio C Hamano <gitster@pobox.com>
Sun, 17 Feb 2008 01:59:20 +0000 (17:59 -0800)
* sp/safecrlf:
safecrlf: Add mechanism to warn about irreversible crlf conversions

Documentation/config.txt
Documentation/gitattributes.txt
builtin-apply.c
builtin-blame.c
cache.h
config.c
convert.c
diff.c
environment.c
sha1_file.c
t/t0020-crlf.sh
index f9bdb164e054ec2d633baab3da69ddd47e14931f..f2f6a774e044e03e78d83e4e77aedcff122f5688 100644 (file)
@@ -139,6 +139,51 @@ core.autocrlf::
        "text" (i.e. be subjected to the autocrlf mechanism) is
        decided purely based on the contents.
 
+core.safecrlf::
+       If true, makes git check if converting `CRLF` as controlled by
+       `core.autocrlf` is reversible.  Git will verify if a command
+       modifies a file in the work tree either directly or indirectly.
+       For example, committing a file followed by checking out the
+       same file should yield the original file in the work tree.  If
+       this is not the case for the current setting of
+       `core.autocrlf`, git will reject the file.  The variable can
+       be set to "warn", in which case git will only warn about an
+       irreversible conversion but continue the operation.
++
+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 contains a mixture of LF and
+CRLF before the commit cannot be recreated by git.  For text
+files this is the right thing to do: it corrects line endings
+such that we have only LF line endings in the repository.
+But for binary files that are accidentally classified as text the
+conversion can corrupt data.
++
+If you recognize such corruption early 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.  You can explicitly tell
+git that this file is binary and git will handle the file
+appropriately.
++
+Unfortunately, the desired effect of cleaning up text files with
+mixed line endings and the undesired effect of corrupting binary
+files cannot be distinguished.  In both cases CRLFs are removed
+in an irreversible way.  For text files this is the right thing
+to do because CRLFs are line endings, while for binary files
+converting CRLFs corrupts data.
++
+Note, this safety check does not mean that a checkout will generate a
+file identical to the original file for a different setting of
+`core.autocrlf`, but only for the current one.  For example, a text
+file with `LF` would be accepted with `core.autocrlf=input` and could
+later be checked out with `core.autocrlf=true`, in which case the
+resulting file would contain `CRLF`, although the original file
+contained `LF`.  However, in both work trees the line endings would be
+consistent, that is either all `LF` or all `CRLF`, but never mixed.  A
+file with mixed line endings would be reported by the `core.safecrlf`
+mechanism.
+
 core.symlinks::
        If false, symbolic links are checked out as small plain files that
        contain the link text. linkgit:git-update-index[1] and
index 35a29fd60c4aa70fbbd7c05ed40e4b545225c74d..84ec9623a26e2c36f7346a430a46a4a4915e0382 100644 (file)
@@ -133,6 +133,26 @@ When `core.autocrlf` is set to "input", line endings are
 converted to LF upon checkin, but there is no conversion done
 upon checkout.
 
+If `core.safecrlf` is set to "true" or "warn", git verifies if
+the conversion is reversible for the current setting of
+`core.autocrlf`.  For "true", git rejects irreversible
+conversions; for "warn", git only prints a warning but accepts
+an irreversible conversion.  The safety triggers to prevent such
+a conversion done to the files in the work tree, but there are a
+few exceptions.  Even though...
+
+- "git add" itself does not touch the files in the work tree, the
+  next checkout would, so the safety triggers;
+
+- "git apply" to update a text file with a patch does touch the files
+  in the work tree, but the operation is about text files and CRLF
+  conversion is about fixing the line ending inconsistencies, so the
+  safety does not trigger;
+
+- "git diff" itself does not touch the files in the work tree, it is
+  often run to inspect the changes you intend to next "git add".  To
+  catch potential problems early, safety triggers.
+
 
 `ident`
 ^^^^^^^
index 46dad5b2a1ce6d70d540df8a40da94702dde68c7..6a88ff018df8890af8ef2e17340d5969dc1e396b 100644 (file)
@@ -1430,7 +1430,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
        case S_IFREG:
                if (strbuf_read_file(buf, path, st->st_size) != st->st_size)
                        return error("unable to open or read %s", path);
-               convert_to_git(path, buf->buf, buf->len, buf);
+               convert_to_git(path, buf->buf, buf->len, buf, 0);
                return 0;
        default:
                return -1;
index c7e68874e7f02ac150343ab6d7a0aa95601f2ba3..1cf254dcca7f814694741ce5ea1ec2b88a26706f 100644 (file)
@@ -2073,7 +2073,7 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
                if (strbuf_read(&buf, 0, 0) < 0)
                        die("read error %s from stdin", strerror(errno));
        }
-       convert_to_git(path, buf.buf, buf.len, &buf);
+       convert_to_git(path, buf.buf, buf.len, &buf, 0);
        origin->file.ptr = buf.buf;
        origin->file.size = buf.len;
        pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
diff --git a/cache.h b/cache.h
index 18fe8447f3bdff2a772716c9293e44e357178a4c..e1000bccb2470ad66038aabbf57b0af96e506f11 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -383,6 +383,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);
@@ -691,7 +699,8 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);
 
 /* convert.c */
 /* returns 1 if *dst was used */
-extern int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst);
+extern int convert_to_git(const char *path, const char *src, size_t len,
+                          struct strbuf *dst, enum safe_crlf checksafe);
 extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);
 
 /* add */
index 7881bbc7782b7937e450ad99c4bb6bda67c1f2e3..8064cae18290d66ee69f1a56e501391ecb317b3d 100644 (file)
--- a/config.c
+++ b/config.c
@@ -415,6 +415,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")) {
                if (!value)
                        return config_error_nonbool(var);
index 552707e8e65997ebfc2120887783c4fc5698e19f..d8c94cb3edeceff7a79b2f5e6ba7cad250e6b301 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -85,8 +85,39 @@ static int is_binary(unsigned long size, struct text_stat *stats)
        return 0;
 }
 
+static void check_safe_crlf(const char *path, int action,
+                            struct text_stat *stats, enum safe_crlf checksafe)
+{
+       if (!checksafe)
+               return;
+
+       if (action == CRLF_INPUT || auto_crlf <= 0) {
+               /*
+                * CRLFs would not be restored by checkout:
+                * check if we'd remove CRLFs
+                */
+               if (stats->crlf) {
+                       if (checksafe == SAFE_CRLF_WARN)
+                               warning("CRLF will be replaced by LF in %s.", path);
+                       else /* i.e. SAFE_CRLF_FAIL */
+                               die("CRLF would be replaced by LF in %s.", path);
+               }
+       } else if (auto_crlf > 0) {
+               /*
+                * CRLFs would be added by checkout:
+                * check if we have "naked" LFs
+                */
+               if (stats->lf != stats->crlf) {
+                       if (checksafe == SAFE_CRLF_WARN)
+                               warning("LF will be replaced by CRLF in %s", path);
+                       else /* i.e. SAFE_CRLF_FAIL */
+                               die("LF would be replaced by CRLF in %s", path);
+               }
+       }
+}
+
 static int crlf_to_git(const char *path, const char *src, size_t len,
-                       struct strbuf *buf, int action)
+                       struct strbuf *buf, int action, enum safe_crlf checksafe)
 {
        struct text_stat stats;
        char *dst;
@@ -95,9 +126,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) {
                /*
@@ -115,6 +143,12 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
                        return 0;
        }
 
+       check_safe_crlf(path, action, &stats, checksafe);
+
+       /* Optimization: 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);
@@ -536,7 +570,8 @@ static int git_path_check_ident(const char *path, struct git_attr_check *check)
        return !!ATTR_TRUE(value);
 }
 
-int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst)
+int convert_to_git(const char *path, const char *src, size_t len,
+                   struct strbuf *dst, enum safe_crlf checksafe)
 {
        struct git_attr_check check[3];
        int crlf = CRLF_GUESS;
@@ -558,7 +593,7 @@ int convert_to_git(const char *path, const char *src, size_t len, struct strbuf
                src = dst->buf;
                len = dst->len;
        }
-       ret |= crlf_to_git(path, src, len, dst, crlf);
+       ret |= crlf_to_git(path, src, len, dst, crlf, checksafe);
        if (ret) {
                src = dst->buf;
                len = dst->len;
diff --git a/diff.c b/diff.c
index 41ec2ced78d7f0d8fe86eea0285b76f70843a3a8..58fe7750f9ca7d63192164a3ac8da27553489d11 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1631,7 +1631,7 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
                 * Convert from working tree format to canonical git format
                 */
                strbuf_init(&buf, 0);
-               if (convert_to_git(s->path, s->data, s->size, &buf)) {
+               if (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf)) {
                        size_t size = 0;
                        munmap(s->data, s->size);
                        s->should_munmap = 0;
index fa3633372b8249899ff1e57a2f1431de029b211d..3527f1663f3f948085ab56a81609ed9764b4e6e9 100644 (file)
@@ -35,6 +35,7 @@ int pager_use_color = 1;
 const char *editor_program;
 const 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() */
index 66a4e00fa83fd9fc853a1ba8a308b05cdc030967..41799492f909e19d907fe5d954463f9fba177a4e 100644 (file)
@@ -2358,7 +2358,8 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,
        if ((type == OBJ_BLOB) && S_ISREG(st->st_mode)) {
                struct strbuf nbuf;
                strbuf_init(&nbuf, 0);
-               if (convert_to_git(path, buf, size, &nbuf)) {
+               if (convert_to_git(path, buf, size, &nbuf,
+                                  write_object ? safe_crlf : 0)) {
                        munmap(buf, size);
                        buf = strbuf_detach(&nbuf, &size);
                        re_allocated = 1;
index 8b27aa892b2b56056b21874cb81c1ddd7956a60a..90ea081db666d707246e37affc5676e6c3502741 100755 (executable)
@@ -8,6 +8,10 @@ q_to_nul () {
        tr Q '\000'
 }
 
+q_to_cr () {
+       tr Q '\015'
+}
+
 append_cr () {
        sed -e 's/$/Q/' | tr Q '\015'
 }
@@ -42,6 +46,60 @@ test_expect_success setup '
        echo happy.
 '
 
+test_expect_success 'safecrlf: autocrlf=input, all CRLF' '
+
+       git config core.autocrlf input &&
+       git config core.safecrlf true &&
+
+       for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
+       ! git add allcrlf
+'
+
+test_expect_success 'safecrlf: autocrlf=input, mixed LF/CRLF' '
+
+       git config core.autocrlf input &&
+       git config core.safecrlf true &&
+
+       for w in Oh here is CRLFQ in text; do echo $w; done | q_to_cr >mixed &&
+       ! git add mixed
+'
+
+test_expect_success 'safecrlf: autocrlf=true, all LF' '
+
+       git config core.autocrlf true &&
+       git config core.safecrlf true &&
+
+       for w in I am all LF; do echo $w; done >alllf &&
+       ! git add alllf
+'
+
+test_expect_success 'safecrlf: autocrlf=true mixed LF/CRLF' '
+
+       git config core.autocrlf true &&
+       git config core.safecrlf true &&
+
+       for w in Oh here is CRLFQ in text; do echo $w; done | q_to_cr >mixed &&
+       ! git add mixed
+'
+
+test_expect_success 'safecrlf: print warning only once' '
+
+       git config core.autocrlf input &&
+       git config core.safecrlf warn &&
+
+       for w in I am all LF; do echo $w; done >doublewarn &&
+       git add doublewarn &&
+       git commit -m "nowarn" &&
+       for w in Oh here is CRLFQ in text; do echo $w; done | q_to_cr >doublewarn &&
+       test $(git add doublewarn 2>&1 | grep "CRLF will be replaced by LF" | wc -l) = 1
+'
+
+test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '
+       git config core.autocrlf false &&
+       git config core.safecrlf false &&
+       git reset --hard HEAD^
+'
+
 test_expect_success 'update with autocrlf=input' '
 
        rm -f tmp one dir/two three &&