From: Michael Haggerty <mhagger@alum.mit.edu>
To: David Aguilar <davvid@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
Jiang Xin <worldhello.net@gmail.com>,
Lea Wiemann <lewiemann@gmail.com>,
David Reiss <dreiss@facebook.com>, Johannes Sixt <j6t@kdbg.org>,
git@vger.kernel.org, "Lars R. Damerow" <lars@pixar.com>,
Jeff King <peff@peff.net>,
Marc Jordan <marc.jordan@disneyanimation.com>
Subject: Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
Date: Thu, 15 Nov 2012 09:18:09 +0100 [thread overview]
Message-ID: <50A4A541.6090201@alum.mit.edu> (raw)
In-Reply-To: <CAJDDKr5F5EcXaTuPWgE5MZJQ=Of6MwW+RmRhhXOLyfQzanjEwQ@mail.gmail.com>
On 11/13/2012 09:50 PM, David Aguilar wrote:
> On Mon, Nov 12, 2012 at 9:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> The log message of the original commit (0454dd93bf) described the
>>> following scenario: a /home partition under which user home directories
>>> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid
>>> hitting /home/.git, /home/.git/objects, and /home/objects (which would
>>> attempt to automount those directories). I believe that this scenario
>>> would not be slowed down by my patches.
>>>
>>> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
>>> slowdown?
>>
>> Yeah, I was also wondering about that.
>>
>> David?
>
> I double-checked our configuration and all the parent directories
> of those listed in GIT_CEILING_DIRECTORIES are local,
> so our particular configuration would not have a performance hit.
>
> We do have multiple directories listed there. Some of them share
> a parent directory. I'm assuming the implementation is simple and
> does not try and avoid repeating the check when the parent dir is
> the same across multiple entries.
>
> In any case, it won't be a problem in practice based on my
> reading of the current code.
OK, so we're back to the following status: some people (including me)
are nervous that this change could cause a performance regression,
though it seems that the most sensible ways of using the
GIT_CEILING_DIRECTORIES feature would not be affected.
In favor: Currently, if a directory containing a symlink is added to
GIT_CEILING_DIRECTORIES, then GIT_CEILING_DIRECTORIES will not work, git
has no way of recognizing that there is a problem, and the only symptom
observable by the user is that the hoped-for performance improvement
from using GIT_CEILING_DIRECTORIES will not materialize (or will
disappear after a filesystem reorg) [1].
Against: The change will cause a performance regression if a
slow-to-stat directory is listed in GIT_CEILING_DIRECTORIES. The
slowdown will occur whenever git is run outside of a true git-managed
project, most nastily in the case of using __git_ps1 in a shell prompt.
I don't have a preference either way about whether these patches should
be merged.
Michael
[1] It is also conceivable that GIT_CEILING_DIRECTORIES is being used to
*hide* an enclosing git project rather than to inform git that there are
no enclosing projects, in which case the enclosing project would *not*
be hidden. This is in fact the mechanism by which the problem causes
failures in our test suite. But I don't expect that this is a common
real-world scenario, and anyway such a failure would be obvious to the
user and quickly fixed.
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
next prev parent reply other threads:[~2012-11-15 8:18 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-21 5:57 [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks Michael Haggerty
2012-10-21 5:57 ` [PATCH v3 1/8] Introduce new static function real_path_internal() Michael Haggerty
2012-10-21 5:57 ` [PATCH v3 2/8] real_path_internal(): add comment explaining use of cwd Michael Haggerty
2012-10-21 5:57 ` [PATCH v3 3/8] Introduce new function real_path_if_valid() Michael Haggerty
2012-10-21 5:57 ` [PATCH v3 4/8] longest_ancestor_length(): use string_list_split() Michael Haggerty
2012-10-21 5:57 ` [PATCH v3 5/8] longest_ancestor_length(): take a string_list argument for prefixes Michael Haggerty
2012-10-21 5:57 ` [PATCH v3 6/8] longest_ancestor_length(): require prefix list entries to be normalized Michael Haggerty
2012-10-22 20:04 ` Johannes Sixt
2012-10-21 5:57 ` [PATCH v3 7/8] normalize_ceiling_entry(): resolve symlinks Michael Haggerty
2012-10-21 5:57 ` [PATCH v3 8/8] string_list_longest_prefix(): remove function Michael Haggerty
2012-10-21 6:51 ` [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks Junio C Hamano
2012-10-22 8:26 ` Michael Haggerty
2012-10-29 0:15 ` David Aguilar
2012-10-29 1:42 ` Junio C Hamano
2012-10-29 5:10 ` Michael Haggerty
2012-11-12 17:47 ` Junio C Hamano
2012-11-13 20:50 ` David Aguilar
2012-11-15 8:18 ` Michael Haggerty [this message]
2013-02-20 6:20 ` Anders Kaseorg
2013-02-20 6:55 ` Junio C Hamano
2013-02-20 9:09 ` [RFC] Provide a mechanism to turn off symlink resolution in ceiling paths Michael Haggerty
2013-02-20 17:41 ` Junio C Hamano
2013-02-21 22:53 ` Junio C Hamano
2013-02-22 7:23 ` Michael Haggerty
2013-02-20 9:39 ` [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks Anders Kaseorg
2012-10-29 5:34 ` Lars Damerow
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=50A4A541.6090201@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=davvid@gmail.com \
--cc=dreiss@facebook.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=lars@pixar.com \
--cc=lewiemann@gmail.com \
--cc=marc.jordan@disneyanimation.com \
--cc=peff@peff.net \
--cc=worldhello.net@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 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.