All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Clemens Buchacher <drizzd@aon.at>
Cc: "Thomas Gummerer" <t.gummerer@gmail.com>,
	"Jeff King" <peff@peff.net>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Lars Schneider" <larsxschneider@gmail.com>,
	"Mike Hommey" <mh@glandium.org>,
	git@vger.kernel.org
Subject: Re: [PATCH] travis-ci: run previously failed tests first, then slowest to fastest
Date: Tue, 02 Feb 2016 15:14:53 -0800	[thread overview]
Message-ID: <xmqq37tar9g2.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160201193340.GA892@ecki> (Clemens Buchacher's message of "Mon, 1 Feb 2016 20:33:42 +0100")

Clemens Buchacher <drizzd@aon.at> writes:

> On Mon, Feb 01, 2016 at 10:17:24AM -0800, Junio C Hamano wrote:
>> 
>> Your proposal is to redefine "is the working tree dirty?"; it would
>> check if "git checkout -f" would change what is in the working tree.
>
> I like this definition. Sounds obviously right.

So this is an illustration.  The change to ce_compare_data() has a
room for improvement in that it assumes that it can always slurp the
whole blob in-core; it should try to use the streaming interface
when it makes sense.  Otherwise we would not be able to handle a
blob that we used to be able to (as index_fd() streams), which would
be a regression.

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.


 read-cache.c       | 53 +++++++++++++++++++++++++++++++++++++++++++++++++----
 t/t0023-crlf-am.sh |  2 +-
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 84616c8..c284f78 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,16 +156,61 @@ 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?
+ * That is, do we lose any data from the working tree copy if we
+ * did a new "git checkout" of that path out of 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);
 
 	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 */
+		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;
 }
diff --git a/t/t0023-crlf-am.sh b/t/t0023-crlf-am.sh
index f9bbb91..5c086b4 100755
--- a/t/t0023-crlf-am.sh
+++ b/t/t0023-crlf-am.sh
@@ -27,7 +27,7 @@ EOF
 test_expect_success 'setup' '
 
 	git config core.autocrlf true &&
-	echo foo >bar &&
+	printf "%s\r\n" foo >bar &&
 	git add bar &&
 	test_tick &&
 	git commit -m initial

  reply	other threads:[~2016-02-02 23:15 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-19  9:24 [PATCH] travis-ci: run previously failed tests first, then slowest to fastest larsxschneider
2016-01-19 19:12 ` Jeff King
2016-01-19 23:00   ` Junio C Hamano
2016-01-20  0:26     ` Mike Hommey
2016-01-20  1:46       ` Junio C Hamano
2016-01-20  1:56         ` Jeff King
2016-01-20  9:22         ` Lars Schneider
2016-01-22  2:33           ` brian m. carlson
2016-01-22  5:52             ` Jeff King
2016-01-22  6:07               ` Jeff King
2016-01-24 14:34                 ` Thomas Gummerer
2016-01-24 20:03                   ` Junio C Hamano
2016-01-24 22:05                     ` Junio C Hamano
2016-01-25 14:42                       ` Thomas Gummerer
2016-01-25 17:26                         ` Junio C Hamano
2016-01-25 21:52                           ` Junio C Hamano
2016-01-27 15:16                             ` Clemens Buchacher
2016-01-27 19:05                               ` Junio C Hamano
2016-01-27 20:49                                 ` Junio C Hamano
2016-01-28  7:10                                   ` Clemens Buchacher
2016-01-28 21:32                                     ` Junio C Hamano
2016-01-30  8:13                                       ` Clemens Buchacher
2016-02-01 18:17                                         ` Junio C Hamano
2016-02-01 19:33                                           ` Clemens Buchacher
2016-02-02 23:14                                             ` Junio C Hamano [this message]
2016-02-03  8:31                                               ` Junio C Hamano
2016-02-01 20:26                                           ` Torsten Bögershausen
2016-01-28  6:20                                 ` eol round trip Was: [PATCH] travis-ci: run previously failed Torsten Bögershausen
2016-01-25 22:41                           ` [PATCH] travis-ci: run previously failed tests first, then slowest to fastest Thomas Gummerer
2016-01-20  1:53       ` Jeff King
2016-01-20  9:10       ` Lars Schneider
2016-01-19 20:00 ` Junio C Hamano
2016-01-19 22:53   ` Junio C Hamano
2016-01-19 23:06     ` Jeff King
2016-01-19 23:26       ` Junio C Hamano
2016-01-19 23:29         ` Jeff King
2016-01-19 23:30         ` Junio C Hamano
2016-01-19 23:27       ` Jeff King
2016-01-20  7:55   ` Johannes Schindelin
2016-01-20  9:04   ` Lars Schneider
2016-01-20 20:35     ` Junio C Hamano

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=xmqq37tar9g2.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    --cc=mh@glandium.org \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    --cc=t.gummerer@gmail.com \
    --cc=tboegi@web.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.