All of lore.kernel.org
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: Junio C Hamano <gitster@pobox.com>
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: Thu, 28 Jan 2016 08:10:00 +0100	[thread overview]
Message-ID: <20160128070959.GA6815@ecki.hitronhub.home> (raw)
In-Reply-To: <xmqqmvrq7nok.fsf@gitster.mtv.corp.google.com>

On Wed, Jan 27, 2016 at 12:49:31PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I wonder what would break if we ask this question instead:
> >
> >     We do not know if the working tree file and the indexed data
> >     match.  Let's see if "git checkout" of that path would leave the
> >     same data as what currently is in the working tree file.

If we do this, then git diff should show the diff between
convert_to_worktree(index state) and the worktree state. That would be
nice, because the diff would actually show what we have in the worktree.
It keeps confusing me that with eol conversion enabled, git diff does
not actually show me the worktree state.

However, even if the file is clean in that direction, there could be a
mismatch between convert_to_git(worktree state) and the index state.
This will happen for example in t0025.4, where we have a CRLF file in
the index and the worktree, but convert_to_git converts it to a file
with LF line endings. Still, I do not see a problem if we also provide a
command like git add --fix-index, which will force normalization of all
files.

> > If we did this, "reset --hard HEAD" followed by "diff HEAD" will by
> > definition always report "is clean" as long as nobody changes files
> > in the working tree, even with the inconsistent data in the index.

Yes, this is a more elegant and a more complete solution to the problem
which prompted me to submit the GIT_ATTRIBUTES_DISABLED patch.

> > This still requires that convert_to_working_tree(), i.e. your smudge
> > filter, is deterministic, though, but I think that is a sensible
> > assumption for sane people, even for those with inconsistent data in
> > the index.

Deterministic, yes. But not unchanging. When a smudge filter is added,
or modified, or if the filter program changes, we still have to remove
the index before we can trust git diff again. The only way to avoid this
would be to somehow detect if the conversion itself changes. One could
hash the attributes, but changes to the filter configuration or the
filter itself are hard to detect. So I think we have to live with this.

> [...] Doing the other check will have to
> inflate the blob data and apply the convert_to_working_tree()
> processing, and also read the whole thing from the filesystem and
> compare, which is more work at runtime.

If we assume that the smudge filter is deterministic, then we could also
hash the output of convert_to_working_tree, and store the hash in the
index. With this optimization, the comparision would be less work,
because we do not have to apply a filter again, whereas currently we
have to apply convert_to_git.

> IOW, I am saying that the "add --fix-index" lunchbreak patch I sent
> earlier in the thread that has to hold the data in-core while
> processing is not a production quality patch ;-)

Ok. The existing implementation in renormalize_buffer (convert.c) works
for me, though.

  reply	other threads:[~2016-01-28  7:12 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 [this message]
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
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=20160128070959.GA6815@ecki.hitronhub.home \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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.