From: "Shawn O. Pearce" <spearce@spearce.org>
To: Alex Riesen <raa.lkml@gmail.com>
Cc: Dmitry Potapov <dpotapov@gmail.com>,
git@vger.kernel.org, Johannes Sixt <johannes.sixt@telecom.at>,
Junio C Hamano <gitster@pobox.com>,
Steffen Prohaska <prohaska@zib.de>
Subject: Re: [PATCH] add GIT_FAST_STAT mode for Cygwin
Date: Wed, 24 Sep 2008 08:02:31 -0700 [thread overview]
Message-ID: <20080924150231.GO3669@spearce.org> (raw)
In-Reply-To: <81b0412b0809240742g2918b300h9114579c4ebf05b4@mail.gmail.com>
Alex Riesen <raa.lkml@gmail.com> wrote:
> 2008/9/24 Dmitry Potapov <dpotapov@gmail.com>:
>
> > Frankly, I don't have strong preference here neither for making this
> > fast version always work nor leave it conditional (perhaps, with the
> > default setting use-fast-version). So, whatever the majority decides
> > is fine with me.
>
> I'm voting for compile-time configuration then.
To be consistent with everything else, compile-time sounds like
what we should do, its how just about every other part of Git
is configured.
However Dmitry pointed out that he has cases where this faster
function doesn't work correctly, and it was path specific. Some
areas of the filesystem work, others don't, on the same system.
A current example of a feature more like this is core.filemode.
A compile-time option makes the feature useful only to those users
who don't ever have a repository which has a mount contained within
the working directory. My understanding of Dmitry's explanation
is he has such cases, which is why I was voting for a runtime
configuration.
A compile-time option means that Git will work fine for years, until
you put a mount in a working directory and *wham* it suddenly stops
working like it should, because of that compile-time optimization
you made long ago and forgot about.
> >> Besides it will remove your setup code, which looks bigger and provoked
> >> more discussion than the real subject itself.
> >
> > I believe Shawn wanted it to be configurable on per-repository basis.
>
> which, I believe, is pointless.
See above. I suggested configurable per-repository because
Dmitry seemed to be saying this feature only works in some of his
repositories and not others. Controlling it by an environment
variable isn't very easy to use as you move between repositories
on the same system.
Maybe I should have leaned more towards compile-time earlier in
the discussion, but Dmitry lead off the patch though with a remark
about users just running the Cygwin package, without building
their own Git. We can't expect the Cygwin maintainers to enable
a feature in a software package that makes it work on 90% of the
Cygwin installs out there; that's just asking for trouble.
But we can compile in a user-configurable switch, where the user can
shoot their own foot off in the name of speed, especially if they
can easily disable it on the oddball repositories where it fails.
Of course it might be even better if the code could auto-sense
when its busted and just switch itself off. E.g. if four or
more consecutive "fast" stat calls fail but the original Cygwin
call succeeds then just always use Cygwin calls for the rest of
the process.
--
Shawn.
next prev parent reply other threads:[~2008-09-24 15:03 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-23 14:06 [PATCH] add GIT_FAST_STAT mode for Cygwin Dmitry Potapov
2008-09-23 14:37 ` Alex Riesen
2008-09-23 16:52 ` Dmitry Potapov
2008-09-23 17:51 ` Jakub Narebski
2008-09-24 11:25 ` Alex Riesen
2008-09-24 14:03 ` Dmitry Potapov
2008-09-24 14:42 ` Alex Riesen
2008-09-24 15:02 ` Shawn O. Pearce [this message]
2008-09-24 15:09 ` Alex Riesen
2008-09-24 15:16 ` Shawn O. Pearce
2008-09-24 15:32 ` Alex Riesen
2008-09-23 15:31 ` Shawn O. Pearce
2008-09-23 17:12 ` Dmitry Potapov
2008-09-23 19:06 ` Shawn O. Pearce
2008-09-23 20:04 ` Dmitry Potapov
2008-09-23 20:17 ` Shawn O. Pearce
2008-09-23 21:28 ` Dmitry Potapov
2008-09-23 21:58 ` Shawn O. Pearce
2008-09-23 19:03 ` Johannes Sixt
2008-09-23 19:48 ` Dmitry Potapov
2008-09-23 20:41 ` Johannes Sixt
2008-09-23 21:11 ` Dmitry Potapov
2008-09-27 7:02 ` [PATCH v2] Add a "fast stat" " Marcus Griep
2008-09-27 8:35 ` Alex Riesen
2008-09-27 10:39 ` Dmitry Potapov
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=20080924150231.GO3669@spearce.org \
--to=spearce@spearce.org \
--cc=dpotapov@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.sixt@telecom.at \
--cc=prohaska@zib.de \
--cc=raa.lkml@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).