From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Andreas Krey <a.krey@gmx.de>,
Michael Haggerty <mhagger@alum.mit.edu>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] refs.c: get_ref_cache: use a bucket hash
Date: Tue, 17 Mar 2015 01:48:00 -0400 [thread overview]
Message-ID: <20150317054759.GA16860@peff.net> (raw)
In-Reply-To: <xmqqd248p4o9.fsf@gitster.dls.corp.google.com>
On Mon, Mar 16, 2015 at 10:35:18PM -0700, Junio C Hamano wrote:
> > It looks like we don't even really care about the value of HEAD. We just
> > want to know "is it a git directory?". I think in other places (like
> > "git add"), we just do an existence check for "$dir/.git". That would
> > not catch a bare repository, but I do not think the current check does
> > either (it is looking for submodules, which always have a .git).
>
> If we wanted to be consistent, perhaps we should be reusing the "is
> this a git repository?" check used by the auto-discovery codepath
> (setup.c:is_git_directory(), perhaps?), but the idea looks simple
> enough and sounds sensible.
Yeah, I almost suggested that, but I'm concerned that would make us
inconsistent with how we report untracked files. I thought that dir.c
used ".git" as a magic token there.
But it seems I'm wrong. We do ignore ".git" directly in treat_path(),
but treat_directory actually checks resolve_gitlink_ref. I think this
will suffer the same problem as Andreas's original issue (e.g., if you
run "git ls-files -o").
Likewise, I think dir.c:remove_dir_recurse is in a similar boat.
Grepping for resolve_gitlink_ref, it looks like there may be others,
too.
All of these should be using the same test, I think. Doing that with
is_git_directory() is probably OK. It is a little more expensive than we
might want for mass-use (it actually opens and parses the HEAD file in
each directory), but it quits early when we _don't_ see a git directory,
which would be the common case here.
-Peff
next prev parent reply other threads:[~2015-03-17 5:48 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-16 14:20 [PATCH] refs.c: get_ref_cache: use a bucket hash Andreas Krey
2015-03-16 17:19 ` Thomas Gummerer
2015-03-16 17:23 ` Junio C Hamano
2015-03-16 18:40 ` Andreas Krey
2015-03-17 2:40 ` Jeff King
2015-03-17 5:35 ` Junio C Hamano
2015-03-17 5:48 ` Jeff King [this message]
2015-11-13 15:29 ` Andreas Krey
2015-11-14 0:01 ` Jeff King
2015-11-14 13:22 ` Andreas Krey
2015-11-14 13:35 ` Andreas Krey
2015-11-16 16:31 ` Jeff King
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=20150317054759.GA16860@peff.net \
--to=peff@peff.net \
--cc=a.krey@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
/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.