Junio C Hamano <gitster@xxxxxxxxx> writes:
> The change to t0023 is merely an example that shows that existing
> tests assume the convert_to_git() way of defining the dirtyness of
> the working tree. It used to be OK to have core.autocrlf set to true,
> have LF terminated file on the working tree and add it to the index,
> and the resulting state was "We just added it to the index, and
> nobody touched the index nor the working tree file--by definition
> the working tree IS CLEAN". With your updated semantics, that no
> longer is true. "We just added it, but if we check it out, we would
> normalize the line ending to be CRLF on the working tree, so the
> working tree is dirty" is what happens.
>
> There are tons of tests that would break the same way all of which
> needs to be looked at and fixed if we were to go in this direction.
That made me think further aloud. I haven't thought things through,
but I wonder what happens if we do both. That is, we define the
working tree file is clean if either:
* the result of running convert_to_git() on the working tree
contents matches what is in the index (because that would mean
doing another "git add" on the path is a no-op); OR
* the result of running convert_to_working_tree() on the content
in the index matches what is in the working tree (because that
would mean doing another "git checkout -f" on the path is a
no-op).
A possible downside (but again, I haven't thought things through, so
this may be a non-issue) of doing this is that it may make it even
harder to "fix" an index entry or a working tree file that is
inconsistent with the user's conversion settings. Even when "git
add" would allow the user to fix an index entry by applying (an
updated) convert_to_git() filter to the working tree file, because
of the new rule that works in the opposite direction, we would end
up saying "the working tree file is clean, and there is no point
doing 'git add'". And vice versa for fixing a working tree file by
running "git checkout".
Also this will make "update-index --refresh" potentially take twice
as long for paths that are not known to be clean and indeed dirty,
as they would need to be processed twice.
An updated patch to do so would look like this. At least we don't
have to update the expectation t0023 makes with this approach.
read-cache.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 84616c8..42d9452 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,17 +156,78 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)
ce_mark_uptodate(ce);
}
+/*
+ * Compare the data in buf with the data in the file pointed by fd and
+ * return 0 if they are identical, and non-zero if they differ.
+ */
+static int compare_with_fd(const char *input, ssize_t len, int fd)
+{
+ for (;;) {
+ char buf[1024 * 16];
+ ssize_t chunk_len, read_len;
+
+ chunk_len = sizeof(buf) < len ? sizeof(buf) : len;
+ read_len = xread(fd, buf, chunk_len ? chunk_len : 1);
+
+ if (!read_len)
+ /* EOF on the working tree file */
+ return !len ? 0 : -1;
+
+ if (!len)
+ /* we expected there is nothing left */
+ return -1;
+
+ if (memcmp(buf, input, read_len))
+ return -1;
+ input += read_len;
+ len -= read_len;
+ }
+}
+
+/*
+ * Does the file in the working tree match what is in the index?
+ */
static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
{
int match = -1;
int fd = open(ce->name, O_RDONLY);
+ /*
+ * Would another "git add" on the path change what is in the
+ * index for the path?
+ */
if (fd >= 0) {
unsigned char sha1[20];
if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
match = hashcmp(sha1, ce->sha1);
/* index_fd() closed the file descriptor already */
}
+ if (!match)
+ return match;
+
+ /*
+ * Would another "git checkout -f" out of the index change
+ * what is in the working tree file?
+ */
+ fd = open(ce->name, O_RDONLY);
+ if (fd >= 0) {
+ enum object_type type;
+ unsigned long size;
+ void *data = read_sha1_file(ce->sha1, &type, &size);
+
+ if (type == OBJ_BLOB) {
+ struct strbuf worktree = STRBUF_INIT;
+ if (convert_to_working_tree(ce->name, data, size,
+ &worktree)) {
+ free(data);
+ data = strbuf_detach(&worktree, &size);
+ }
+ if (!compare_with_fd(data, size, fd))
+ match = 0;
+ }
+ free(data);
+ close(fd);
+ }
return match;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html