From: David Turner <dturner@twopensource.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, Christian Couder <christian.couder@gmail.com>
Subject: Re: [PATCH 2/5] Add watchman support to reduce index refresh cost
Date: Mon, 02 Nov 2015 16:19:34 -0500 [thread overview]
Message-ID: <1446499174.4131.20.camel@twopensource.com> (raw)
In-Reply-To: <1446386146-10438-3-git-send-email-pclouds@gmail.com>
On Sun, 2015-11-01 at 14:55 +0100, Nguyễn Thái Ngọc Duy wrote:
> The previous patch has the logic to clear bits in 'WAMA' bitmap. This
> patch has logic to set bits as told by watchman. The missing bit,
> _using_ these bits, are not here yet.
>
> A lot of this code is written by David Turner originally, mostly from
> [1]. I'm just copying and polishing it a bit.
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/248006
Our code has evolved somewhat from there[1]. It looks like you've
incorporated some of these updates. But I wanted to call out one thing
in particular that it doesn't look like this patch handles: there's some
real ugliness on at least OSX around case-changing renames.
You probably won't be able to use our code directly as-is, but we'll
want to do something about this situation. This is thicket of TOCTOU
issues. For instance, watchman learns that a file called FOO has
changed, but by the time it goes to the filesystem to look up the
canonical case, FOO has been renamed to foo already (or renamed and then
deleted).
I won't say for sure that your code is insufficient as I haven't yet
tried it out, but we should ensure this case is handled before this is
merged.
[1] https://github.com/dturner-tw/git/tree/dturner/watchman
> +
> + pos = index_name_pos(istate, wm->name, strlen(wm->name));
This is the bit where case matters.
next prev parent reply other threads:[~2015-11-02 21:19 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-01 13:55 [PATCH 0/5] Use watchman to reduce index refresh time Nguyễn Thái Ngọc Duy
2015-11-01 13:55 ` [PATCH 1/5] read-cache: add watchman 'WAMA' extension Nguyễn Thái Ngọc Duy
2015-11-02 22:03 ` David Turner
2015-11-03 19:17 ` Duy Nguyen
2015-11-03 19:49 ` David Turner
2015-11-01 13:55 ` [PATCH 2/5] Add watchman support to reduce index refresh cost Nguyễn Thái Ngọc Duy
2015-11-02 21:19 ` David Turner [this message]
2015-11-01 13:55 ` [PATCH 3/5] read-cache: allow index-helper to prepare shm before git reads it Nguyễn Thái Ngọc Duy
2015-11-01 13:55 ` [PATCH 4/5] index-helper: use watchman to avoid refreshing index with lstat() Nguyễn Thái Ngọc Duy
2015-11-01 13:55 ` [PATCH 5/5] update-index: enable/disable watchman support Nguyễn Thái Ngọc Duy
2015-11-02 14:54 ` [PATCH 0/5] Use watchman to reduce index refresh time Paolo Ciarrocchi
2015-11-02 19:23 ` Duy Nguyen
2015-11-03 9:21 ` Duy Nguyen
2015-11-03 10:26 ` Paolo Ciarrocchi
2015-11-09 20:06 ` Christian Couder
2015-11-10 21:04 ` David Turner
2015-11-20 9:45 ` Christian Couder
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=1446499174.4131.20.camel@twopensource.com \
--to=dturner@twopensource.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--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).