Re: [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
- From
Jeff King <peff@peff.net>
- Date
- Oct 11, 2011, 16:16 UTC
- Message-ID
- <20111011161652.GA15629@sigill.intra.peff.net>
- In-Reply-To
- <1316121043-29367-20-git-send-email-mhagger@alum.mit.edu>
On Thu, Sep 15, 2011 at 11:10:40PM +0200, Michael Haggerty wrote:
> While resolving references, if a reference is found that is in an > unrecognized format, emit a warning (and then fail, as before). > Wouldn't *you* want to know?
Not necessarily. :)
I noticed this today, and bisected it to this patch:
$ git.v1.7.7 show config fatal: ambiguous argument 'config': unknown revision or path not in the working tree. Use '--' to separate paths from revisions
$ git.mh.check-ref-format-3 show config warning: reference in .git/config is formatted incorrectly warning: reference in .git/config is formatted incorrectly fatal: ambiguous argument 'config': unknown revision or path not in the working tree. Use '--' to separate paths from revisions
which is arguably not a big deal, as we eventually report failure anyway. But it's even worse with:
$ git branch config v1.7.7 $ git show --oneline config warning: reference in .git/config is formatted incorrectly 703f05a Git 1.7.7
As you probably guessed, this is due to the ref resolution rules first looking directly in .git for refs, which catches things like "HEAD" and fully-qualified refs like "refs/heads/foo".
Which means we have been considering ".git/config" as a possible ref for a long time, which is arguably wrong. Your patch only makes it more obvious that this is the case.
I wonder if the right solution is for us to be more picky about what can be found in $GIT_DIR. Maybe matching all-uppercase, or starting with "refs/", which I think would match existing convention?
-Peff