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
List information: http://vger.kernel.org/majordomo-info.html
* 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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).