Re: [PATCH] symbolic ref: refuse non-ref targets in HEAD

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 29, 2009, 07:53 UTC
Message-ID: <7vljsuh7kf.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20090129045205.GA31183@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> I know that using symbolic-ref manually is rare, but both I and the
> original poster have been bitten by this (and figuring out what is going
> on and fixing it is quite painful). But most importantly, I don't think
> this can possibly hurt anyone trying to use this legitimately, since the
> exact thing it is protecting against corrupts your repo. :)

I generally do not like adding artificial limitation to plumbing like this patch does, because the end user making silly mistake using plumbing is a sign that there was something lacking in the Porcelain.

But for this particular case, I do not think any future usage of symbolic-ref plubming will get inconvenienced with the change. I would even suggest making the check tighter to insist on refs/heads/ (not just refs/) and tighten validate_headref() in path.c to match.

> Please beware that running the test script on the current "master" will
> actually hose your git repo (test 3 kills the trash directory's
> .git/HEAD, which means test 4 thinks your parent .git/ is its current
> repo). Maybe it makes sense to do a precautionary reset in between.

In addition, perhaps it may make sense to use test_create_repo to go one level deeper before starting to play around, so that trash directory's repository will prevent you from going any further up.