On Tue, Mar 01, 2016 at 06:47:52PM -0500, David Turner wrote:
> > My fix for this was to teach read_mailmap to avoid looking for
> > HEAD:.mailmap if we are not in a repository, but to continue with the
> > others (.mailmap in the cwd, and the mailmap.file config variable).
> > ...
> > But I do think your patch is a potential regression there, if anybody
> > does do that.
>
> Your version sounds better. But I don't see it in the patch set you
> sent earlier?
It's not. Sorry to be unclear. There were _two_ cleanups I was talking
about (cases where we don't check whether we're in a repo, and fact that
the repo startup code is unreliable), and I got sucked into the second
one. I'll try to work up and share my startup_info one today.
> When writing my patch, I had assumed that the issue was the resolve_ref
> on the HEAD that's an argument -- but it's not. The actual traceback
> is:
> [...]
> #2 resolve_ref_unsafe (refname=refname@entry=0x550b3b "HEAD",
> resolve_flags=resolve_flags@entry=0,
> sha1=sha1@entry=0x7fffffffd100 "\b\326\377\377\377\177",
> flags=flags@entry=0x7fffffffd0fc) at refs/files-backend.c:1600
> #3 0x00000000004ffe69 in read_config () at remote.c:471
Oh, right. I did see problems here but missed them when comparing my
patch to yours. I ended up in remote.c:read_config, having it check
whether startup_info->have_repository is set; if it isn't, there is no
point in looking at HEAD.
That covers this case, and several others I happened across. Thanks for
clarifying.
> I'm not sure what the right way to fix this is -- in read_config, we're
> about to access some stuff in a repo (config, HEAD). It's OK to skip
> that stuff if we're not in a repo, but we don't want to run
> setup_git_directory twice (that breaks some stuff), and some of the
> other callers have already called it. On top of your earlier
> repo_initialized patch, we could add the following to read_config:
>
> + if (!repo_initialized) {
> + int nongit = 0;
> + setup_git_directory_gently(&nongit);
> + if (nongit)
> + return;
> + }
>
> But that patch I think was not intended to be permanent. Still, it
> does seem odd that there's no straightforward way to know if the repo
> is initialized. Am I missing something?
No, there isn't a straightforward way; I think we'll have to add one.
I'll polish up my series which does this.
-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