On Thu, Sep 01, 2016 at 03:31:28PM -0700, Junio C Hamano wrote:
> -- >8 --
> Subject: symbolic-ref -d: do not allow removal of HEAD
>
> If you delete the symbolic-ref HEAD from a repository, Git no longer
> considers it valid, and even "git symbolic-ref HEAD refs/heads/master"
> would not be able to recover from that state.
>
> In the spirit similar to afe5d3d5 ("symbolic ref: refuse non-ref
> targets in HEAD", 2009-01-29), forbid removal of HEAD.
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
Makes sense. You might want to change "it" in "no longer considers it
valid" to "the repository". At first I thought "it" referred to the
symref. Which obviously shouldn't be valid after being deleted. :)
> I decided against it for now for no good reason, other than I am a
> bit superstitious, but it may be a good idea to move these safety
> checks to delete_ref() and create_symref() in the longer term.
Yeah, that somehow feels weird and too low-level to me. After all, we
_do_ want to drop HEAD as a symref when we turn it into a detached HEAD.
The point of this (and afe5d3d5) is to prevent people from shooting
themselves in the foot. Internal Git code should know to avoid this
foot-shooting itself.
OTOH, I think "git update-ref --no-deref -d HEAD" is another user-facing
hole-in-foot opportunity, and it would be blocked by putting this into
delete_ref().
> -test_expect_success 'symbolic-ref deletes HEAD' '
> - git symbolic-ref -d HEAD &&
> +test_expect_success 'HEAD cannot be removed' '
> + test_must_fail git symbolic-ref -d HEAD
> +'
> +
> +test_expect_success 'symbolic-ref can be deleted' '
> + git symbolic-ref NOTHEAD refs/heads/foo &&
> + git symbolic-ref -d NOTHEAD &&
> test_path_is_file .git/refs/heads/foo &&
> - test_path_is_missing .git/HEAD
> + test_path_is_missing .git/NOTHEAD
> '
> reset_to_sane
Do you want another "reset_to_sane" call after your new test? Otherwise
if it fails the "symbolic-ref can be deleted" test will start operating
on the parent repository.
-Peff