git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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/

  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).