On Sat, Nov 14, 2015 at 02:35:01PM +0100, Andreas Krey wrote:
> On Fri, 13 Nov 2015 19:01:18 +0000, Jeff King wrote:
> ...
> > 2. But for a little more work, pushing the is_git_directory() check
> > out to the call-sites gives us probably saner semantics overall.
>
> Oops, now I get it[1]: You mean replacing resolve_gitlink_ref usages
> with is_git_directory, like:
Yes. I mistakenly said is_git_directory, when I really meant
is_git_repository, the new function added in 0179ca7a62. You seem to
have figured out what I meant, but the critical thing is that we check
"$dir/.git", not just "$dir" (and check it both as a git dir and as a
gitfile, as is_git_repository() does).
I'm not sure if we can simply make that function public or not. It's
mostly straightforward, but it does err on the side of "yes, this is a
git repo" if we see a ".git" file we can't read. I think that's probably
reasonable in most sites, but I didn't look closely.
> diff --git a/dir.c b/dir.c
> index d2a8f06..7765dc6 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1375,8 +1375,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
> if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)
> break;
> if (!(dir->flags & DIR_NO_GITLINKS)) {
> - unsigned char sha1[20];
> - if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
> + if (is_git_directory(dirname))
> return path_untracked;
> }
> return path_recurse;
>
> That, I like. If it is correct.
Yes, that's what I had in mind, modulo the directory/repository thing
above (the is_git_repository function also takes a strbuf, so we'd need
to handle that extra allocation somewhere).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html