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

The following didn't work as expected:
- In a middle of a merge
- merge.renormalize is true,
- .gitattributes = "* text=auto"
- core.eol = crlf

Merge a blob with CRLF "first line\r\nsame line\r\n" and a blob
with LF "first line\nsame line\n".

The expected result of the merge is "first line\nsame line\n".

The content in the working tree is "first line\r\nsame line\r\n",
and ce_compare_data() should find that the content is clean and return 0.

Deep down crlf_to_git() is invoked, to check if CRLF are converted or not.

The "new safer autocrlf handling" calls blob_has_cr().

Instead of using the sha1 of the blob, (CRLF in this example),
the function get_sha1_from_index() is invoked.
get_sha1_from_index() decides to return "ours" when in the middle of
the merge, which is LF.

As a result, the CRLF in the worktree are converted into LF before
the comparison.
The contents of LF and CRLF don't match any more.

The problem is that ce_compare_data() has ce->sha1, but the sha1 is lost
on it's way into blob_has_cr().

Forwarding ce->sha1 from ce_compare_data() into crlf_to_git() makes sure
that blob_has_cr() looks at the appropriate blob.

Add a new parameter index_blob_sha1 to convert_to_git(), and forward the
sha1 from ce_compare_data() into convert_to_git(). Other callers use NULL
for index_blob_sha1, and the sha1 is determined from path
using get_sha1_from_cache(path). This is the same handling as before.

In the same spirit, forward the sha1 into would_convert_to_git().

While at it, rename has_cr_in_index() into blob_has_cr()
and replace 0 with SAFE_CRLF_FALSE.

Add a TC in t6038 to have a test coverage under Linux.

Signed-off-by: Torsten Bögershausen <tbo...@web.de>
---
 builtin/apply.c            |  3 ++-
 builtin/blame.c            |  2 +-
 cache.h                    |  1 +
 combine-diff.c             |  3 ++-
 convert.c                  | 43 ++++++++++++++++++++++++++------------
 convert.h                  | 15 ++++++++++----
 diff.c                     |  3 ++-
 dir.c                      |  2 +-
 read-cache.c               |  4 +++-
 sha1_file.c                | 12 ++++++++---
 t/t6038-merge-text-auto.sh | 51 +++++++++++++++++++++++++---------------------
 11 files changed, 90 insertions(+), 49 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 8e4da2e..0cf9a0a 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2140,7 +2140,8 @@ 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, 0);
+               convert_to_git(path, buf->buf, buf->len, buf,
+                              SAFE_CRLF_FALSE, NULL);
                return 0;
        default:
                return -1;
diff --git a/builtin/blame.c b/builtin/blame.c
index 21f42b0..1c523b6 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2377,7 +2377,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
                if (strbuf_read(&buf, 0, 0) < 0)
                        die_errno("failed to read from stdin");
        }
-       convert_to_git(path, buf.buf, buf.len, &buf, 0);
+       convert_to_git(path, buf.buf, buf.len, &buf, SAFE_CRLF_FALSE, NULL);
        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 322ee40..86f48f9 100644
--- a/cache.h
+++ b/cache.h
@@ -605,6 +605,7 @@ extern int ie_modified(const struct index_state *, const 
struct cache_entry *, s
 
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
+#define HASH_USE_SHA_NOT_PATH 4
 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum 
object_type type, const char *path, unsigned flags);
 extern int index_path(unsigned char *sha1, const char *path, struct stat *st, 
unsigned flags);
 
diff --git a/combine-diff.c b/combine-diff.c
index 8f2313d..8b535a7 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1052,7 +1052,8 @@ static void show_patch_diff(struct combine_diff_path 
*elem, int num_parent,
                        if (is_file) {
                                struct strbuf buf = STRBUF_INIT;
 
-                               if (convert_to_git(elem->path, result, len, 
&buf, safe_crlf)) {
+                               if (convert_to_git(elem->path, result, len,
+                                                  &buf, safe_crlf, NULL)) {
                                        free(result);
                                        result = strbuf_detach(&buf, &len);
                                        result_size = len;
diff --git a/convert.c b/convert.c
index 67d69b5..802ee7c 100644
--- a/convert.c
+++ b/convert.c
@@ -219,23 +219,28 @@ static void check_safe_crlf(const char *path, enum 
crlf_action crlf_action,
        }
 }
 
-static int has_cr_in_index(const char *path)
+static int blob_has_cr(const unsigned char *index_blob_sha1)
 {
        unsigned long sz;
        void *data;
-       int has_cr;
-
-       data = read_blob_data_from_cache(path, &sz);
+       int has_cr = 0;
+       enum object_type type;
+       if (!index_blob_sha1)
+               return 0;
+       data = read_sha1_file(index_blob_sha1, &type, &sz);
        if (!data)
                return 0;
-       has_cr = memchr(data, '\r', sz) != NULL;
+       if (type == OBJ_BLOB)
+               has_cr = memchr(data, '\r', sz) != NULL;
+
        free(data);
        return has_cr;
 }
 
 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)
+                      enum crlf_action crlf_action, enum safe_crlf checksafe,
+                      const unsigned char *index_blob_sha1)
 {
        struct text_stat stats;
        char *dst;
@@ -256,14 +261,23 @@ static int crlf_to_git(const char *path, const char *src, 
size_t len,
        if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || 
crlf_action == CRLF_AUTO_CRLF) {
                if (convert_is_binary(len, &stats))
                        return 0;
+
                /*
                 * If the file in the index has any CR in it, do not convert.
                 * This is the new safer autocrlf handling.
                 */
                if (checksafe == SAFE_CRLF_RENORMALIZE)
                        checksafe = SAFE_CRLF_FALSE;
-               else if (has_cr_in_index(path))
-                       return 0;
+               else {
+                       /*
+                        * If the file in the index has any CR in it, do not 
convert.
+                        * This is the new safer autocrlf handling.
+                        */
+                       if (!index_blob_sha1)
+                               index_blob_sha1 = get_sha1_from_cache(path);
+                       if (blob_has_cr(index_blob_sha1))
+                               return 0;
+               }
        }
        check_safe_crlf(path, crlf_action, &stats, checksafe);
 
@@ -855,7 +869,8 @@ const char *get_convert_attr_ascii(const char *path)
 }
 
 int convert_to_git(const char *path, const char *src, size_t len,
-                   struct strbuf *dst, enum safe_crlf checksafe)
+                  struct strbuf *dst, enum safe_crlf checksafe,
+                  const unsigned char *index_blob_sha1)
 {
        int ret = 0;
        const char *filter = NULL;
@@ -876,7 +891,7 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
                src = dst->buf;
                len = dst->len;
        }
-       ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
+       ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe, 
index_blob_sha1);
        if (ret && dst) {
                src = dst->buf;
                len = dst->len;
@@ -885,7 +900,8 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
 }
 
 void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
-                             enum safe_crlf checksafe)
+                             enum safe_crlf checksafe,
+                             const unsigned char *index_blob_sha1)
 {
        struct conv_attrs ca;
        convert_attrs(&ca, path);
@@ -896,7 +912,8 @@ void convert_to_git_filter_fd(const char *path, int fd, 
struct strbuf *dst,
        if (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))
                die("%s: clean filter '%s' failed", path, ca.drv->name);
 
-       crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
+       crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action,
+                   checksafe, index_blob_sha1);
        ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
 }
 
@@ -951,7 +968,7 @@ int renormalize_buffer(const char *path, const char *src, 
size_t len, struct str
                src = dst->buf;
                len = dst->len;
        }
-       return ret | convert_to_git(path, src, len, dst, SAFE_CRLF_RENORMALIZE);
+       return ret | convert_to_git(path, src, len, dst, SAFE_CRLF_RENORMALIZE, 
NULL);
 }
 
 /*****************************************************************
diff --git a/convert.h b/convert.h
index 81b6cdf..8c34dd5 100644
--- a/convert.h
+++ b/convert.h
@@ -39,19 +39,26 @@ extern const char *get_convert_attr_ascii(const char *path);
 
 /* returns 1 if *dst was used */
 extern int convert_to_git(const char *path, const char *src, size_t len,
-                         struct strbuf *dst, enum safe_crlf checksafe);
+                         struct strbuf *dst, enum safe_crlf checksafe,
+                         const unsigned char *index_blob_sha1);
+
 extern int convert_to_working_tree(const char *path, const char *src,
                                   size_t len, struct strbuf *dst);
 extern int renormalize_buffer(const char *path, const char *src, size_t len,
                              struct strbuf *dst);
-static inline int would_convert_to_git(const char *path)
+static inline int would_convert_to_git(const char *path,
+                                      const unsigned char *index_blob_sha1)
 {
-       return convert_to_git(path, NULL, 0, NULL, 0);
+       return convert_to_git(path, NULL, 0, NULL, SAFE_CRLF_FALSE,
+                             index_blob_sha1);
 }
+
 /* Precondition: would_convert_to_git_filter_fd(path) == true */
 extern void convert_to_git_filter_fd(const char *path, int fd,
                                     struct strbuf *dst,
-                                    enum safe_crlf checksafe);
+                                    enum safe_crlf checksafe,
+                                    const unsigned char *index_blob_sha1);
+
 extern int would_convert_to_git_filter_fd(const char *path);
 
 /*****************************************************************
diff --git a/diff.c b/diff.c
index d3734d3..a8308e0 100644
--- a/diff.c
+++ b/diff.c
@@ -2810,7 +2810,8 @@ int diff_populate_filespec(struct diff_filespec *s, 
unsigned int flags)
                /*
                 * Convert from working tree format to canonical git format
                 */
-               if (convert_to_git(s->path, s->data, s->size, &buf, crlf_warn)) 
{
+               if (convert_to_git(s->path, s->data, s->size, &buf,
+                                  crlf_warn, NULL)) {
                        size_t size = 0;
                        munmap(s->data, s->size);
                        s->should_munmap = 0;
diff --git a/dir.c b/dir.c
index 6172b34..66f1ffb 100644
--- a/dir.c
+++ b/dir.c
@@ -712,7 +712,7 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
                                 (pos = cache_name_pos(fname, strlen(fname))) 
>= 0 &&
                                 !ce_stage(active_cache[pos]) &&
                                 ce_uptodate(active_cache[pos]) &&
-                                !would_convert_to_git(fname))
+                                !would_convert_to_git(fname, NULL))
                                hashcpy(sha1_stat->sha1, 
active_cache[pos]->sha1);
                        else
                                hash_sha1_file(buf, size, "blob", 
sha1_stat->sha1);
diff --git a/read-cache.c b/read-cache.c
index a3ef967..c109b6d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -163,7 +163,9 @@ static int ce_compare_data(const struct cache_entry *ce, 
struct stat *st)
 
        if (fd >= 0) {
                unsigned char sha1[20];
-               if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
+               unsigned flags = HASH_USE_SHA_NOT_PATH;
+               memcpy(sha1, ce->sha1, sizeof(sha1));
+               if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, flags))
                        match = hashcmp(sha1, ce->sha1);
                /* index_fd() closed the file descriptor already */
        }
diff --git a/sha1_file.c b/sha1_file.c
index d5e1121..10630f0 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3273,6 +3273,7 @@ static int index_mem(unsigned char *sha1, void *buf, 
size_t size,
 {
        int ret, re_allocated = 0;
        int write_object = flags & HASH_WRITE_OBJECT;
+       const int valid_sha1 = flags & HASH_USE_SHA_NOT_PATH;
 
        if (!type)
                type = OBJ_BLOB;
@@ -3283,7 +3284,8 @@ static int index_mem(unsigned char *sha1, void *buf, 
size_t size,
        if ((type == OBJ_BLOB) && path) {
                struct strbuf nbuf = STRBUF_INIT;
                if (convert_to_git(path, buf, size, &nbuf,
-                                  write_object ? safe_crlf : SAFE_CRLF_FALSE)) 
{
+                                  write_object ? safe_crlf : SAFE_CRLF_FALSE,
+                                  valid_sha1 ? sha1 : NULL)) {
                        buf = strbuf_detach(&nbuf, &size);
                        re_allocated = 1;
                }
@@ -3311,13 +3313,15 @@ static int index_stream_convert_blob(unsigned char 
*sha1, int fd,
 {
        int ret;
        const int write_object = flags & HASH_WRITE_OBJECT;
+       const int valid_sha1 = flags & HASH_USE_SHA_NOT_PATH;
        struct strbuf sbuf = STRBUF_INIT;
 
        assert(path);
        assert(would_convert_to_git_filter_fd(path));
 
        convert_to_git_filter_fd(path, fd, &sbuf,
-                                write_object ? safe_crlf : SAFE_CRLF_FALSE);
+                                write_object ? safe_crlf : SAFE_CRLF_FALSE,
+                                valid_sha1 ? sha1 : NULL);
 
        if (write_object)
                ret = write_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),
@@ -3394,6 +3398,8 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
             enum object_type type, const char *path, unsigned flags)
 {
        int ret;
+       const unsigned char *sha1_ce;
+       sha1_ce = flags & HASH_USE_SHA_NOT_PATH ? sha1 : NULL;
 
        /*
         * Call xsize_t() only when needed to avoid potentially unnecessary
@@ -3404,7 +3410,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
        else if (!S_ISREG(st->st_mode))
                ret = index_pipe(sha1, fd, type, path, flags);
        else if (st->st_size <= big_file_threshold || type != OBJ_BLOB ||
-                (path && would_convert_to_git(path)))
+                (path && would_convert_to_git(path,sha1_ce)))
                ret = index_core(sha1, fd, xsize_t(st->st_size), type, path,
                                 flags);
        else
diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index 33b77ee..5e8d5fa 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -91,16 +91,13 @@ test_expect_success 'Merge after setting text=auto' '
        compare_files expected file
 '
 
-test_expect_success 'Merge addition of text=auto' '
+test_expect_success 'Merge addition of text=auto eol=LF' '
+       git config core.eol lf &&
        cat <<-\EOF >expected &&
        first line
        same line
        EOF
 
-       if test_have_prereq NATIVE_CRLF; then
-               append_cr <expected >expected.temp &&
-               mv expected.temp expected
-       fi &&
        git config merge.renormalize true &&
        git rm -fr . &&
        rm -f .gitattributes &&
@@ -109,17 +106,31 @@ test_expect_success 'Merge addition of text=auto' '
        compare_files  expected file
 '
 
+test_expect_success 'Merge addition of text=auto eol=CRLF' '
+       git config core.eol crlf &&
+       cat <<-\EOF >expected &&
+       first line
+       same line
+       EOF
+
+       append_cr <expected >expected.temp &&
+       mv expected.temp expected &&
+       git config merge.renormalize true &&
+       git rm -fr . &&
+       rm -f .gitattributes &&
+       git reset --hard b &&
+       echo >&2 "After git reset --hard b" &&
+       git ls-files -s --eol >&2 &&
+       git merge a &&
+       compare_files  expected file
+'
+
 test_expect_success 'Detect CRLF/LF conflict after setting text=auto' '
+       git config core.eol native &&
        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 >>expected &&
+       echo same line >>expected &&
+       echo ======= >>expected &&
        echo first line | append_cr >>expected &&
        echo same line | append_cr >>expected &&
        echo ">>>>>>>" >>expected &&
@@ -135,15 +146,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 ======= >>expected &&
+       echo first line >>expected &&
+       echo same line >>expected &&
        echo ">>>>>>>" >>expected &&
        git config merge.renormalize false &&
        rm -f .gitattributes &&
-- 
2.0.0.rc1.6318.g0c2c796

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to