From: David Turner <David.Turner@twosigma.com>
To: 'Christian Couder' <christian.couder@gmail.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git <git@vger.kernel.org>, "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
"Ben Peart" <benpeart@microsoft.com>
Subject: RE: [PATCH 0/1] Preserve the untracked cache across checkout, reset --hard, etc
Date: Mon, 8 May 2017 15:58:12 +0000 [thread overview]
Message-ID: <2c0be7c0a8ce4f5ea49dfaf3f8511bc0@exmbdft7.ad.twosigma.com> (raw)
In-Reply-To: <CAP8UFD3qoeQE5ZbFHGLE-_B4rOr3X2m-WPB7rb8QAbZA6zMRxg@mail.gmail.com>
Can you actually keep the email address as my Twopensource one? I want to make sure that Twitter, my employer at the time, gets credit for this work (just as I want to make sure that my current employer, Two Sigma, gets credit for my current work).
Please feel free to add Signed-off-by: David Turner <dturner@twosigma.com> in case that makes tracking easier.
Thanks.
WRT the actual patch, I want to note that past me did not do a great job here. The tests do not correctly check that the post-checkout untracked cache is still valid after a checkout. For example, let's say that previously, the directory foo was entirely untracked (but it contained a file bar), but after the checkout, there is a file foo/baz. Does the untracked cache need to get updated?
Unfortunately, the untracked cache is very unlikely to make it to the top of my priority list any time soon, so I won't be able to correct this test (and, if necessary, correct the code). But I would strongly suggest that the test be improved before this code is merged.
Thanks for CCing me.
> -----Original Message-----
> From: Christian Couder [mailto:christian.couder@gmail.com]
> Sent: Monday, May 8, 2017 6:12 AM
> To: Johannes Schindelin <johannes.schindelin@gmx.de>
> Cc: git <git@vger.kernel.org>; Junio C Hamano <gitster@pobox.com>; Nguyễn
> Thái Ngọc Duy <pclouds@gmail.com>; Ben Peart <benpeart@microsoft.com>;
> David Turner <David.Turner@twosigma.com>
> Subject: Re: [PATCH 0/1] Preserve the untracked cache across checkout, reset --
> hard, etc
>
> (Adding Dave in Cc as it looks like he is involved.)
>
> On Mon, May 8, 2017 at 11:41 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > I recently sent out a request for assistance, after noticing that the
> > untracked cache is simply thrown away after operations such as `git
> > checkout` or `git reset --hard`:
> >
> > http://public-inbox.org/git/alpine.DEB.2.20.1705031202470.3480@virtual
> > box/
> >
> > Duy responded with some high-level reasoning that it should be
> > possible to simply reuse the untracked cache data structure in the new
> > index, as he had a gut feeling that "we do invalidation right".
> >
> > I did not have time to back that up by a thorough analysis of the
> > code, but it turns out that it is unnecessary: Ben Peart pointed me to
> > a patch of Dave Turner's that was submitted as part of the watchman
> > series, addressing the very issue about which I was concerned.
> >
> > And I trust Dave to have validated the idea that the untracked cache
> > invalidation "is done right" even when we simply move the pointer to a
> > different index_state struct than originally.
> >
> > Seeing as the untracked cache being dropped unceremoniously when it
> > should not be dropped, in a surprising number of operations, I think
> > it is a sensible change, and important, too, and independent enough
> > from the watchman patches to merit being separated out and applied
> > pretty soon.
> >
> > So what I did was simply to drop the two lines from this patch that
> > referred to index_state fields added by Dave's watchman patch series.
> >
> > Please do not mistake this for a sign that I am disinterested in
> > watchman support, far from it... stay tuned ;-)
> >
> > Oh, and I adjusted Dave's email address. Dave, is that okay?
> >
> > As we are in a feature freeze phase, I was debating whether to send
> > out this patch now or later.
> >
> > Having thought about it for quite a bit, I am now convinced that this
> > patch fixes a bug in the untracked cache feature that is so critical
> > as to render it useless: if you
> >
> > - have to switch between branches frequently, or
> > - rebase frequently (which calls `git reset --hard`), or
> > - stash frequently (which calls `git reset --hard`),
> >
> > it is as if you had not enabled the untracked cache at all. Even
> > worse, Git will do a ton of work to recreate the untracked cache and
> > to store it as an index extension, *just* to throw the untracked away in the
> end.
> >
> >
> > David Turner (1):
> > unpack-trees: preserve index extensions
> >
> > cache.h | 1 +
> > read-cache.c | 6 ++++++
> > t/t7063-status-untracked-cache.sh | 22 ++++++++++++++++++++++
> > unpack-trees.c | 1 +
> > 4 files changed, 30 insertions(+)
> >
> >
> > base-commit: 4fa66c85f11bc5a541462ca5ae3246aa0ce02e74
> > Published-As:
> > https://github.com/dscho/git/releases/tag/preserve-untracked-cache-v1
> > Fetch-It-Via: git fetch https://github.com/dscho/git
> > preserve-untracked-cache-v1
> >
> > --
> > 2.12.2.windows.2.800.gede8f145e06
> >
next prev parent reply other threads:[~2017-05-08 15:58 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-08 9:41 [PATCH 0/1] Preserve the untracked cache across checkout, reset --hard, etc Johannes Schindelin
2017-05-08 9:41 ` [PATCH 1/1] unpack-trees: preserve index extensions Johannes Schindelin
2017-05-08 10:12 ` [PATCH 0/1] Preserve the untracked cache across checkout, reset --hard, etc Christian Couder
2017-05-08 15:58 ` David Turner [this message]
2017-05-09 5:02 ` Junio C Hamano
2017-05-09 12:51 ` Ben Peart
2017-05-18 16:58 ` Ben Peart
2017-05-18 17:30 ` David Turner
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=2c0be7c0a8ce4f5ea49dfaf3f8511bc0@exmbdft7.ad.twosigma.com \
--to=david.turner@twosigma.com \
--cc=benpeart@microsoft.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--cc=pclouds@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).