v3, 6 partsrefs: Introduce pseudoref and per-worktree ref concepts

> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
> per-worktree.  We don't want multiple notes merges happening at once
> (in different worktrees), so we want to make these refs true refs.
>
> So, we lowercase NOTES_MERGE_REF and friends.  That way, backends
> that distinguish between pseudorefs and real refs can correctly
> handle notes merges.
>
> This will also enable us to prevent pseudorefs from being updated by
> the ref update machinery e.g. git update-ref.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---

>  	# Fail to finalize merge
>  	test_must_fail git notes merge --commit >output 2>&1 &&
> -	# .git/NOTES_MERGE_* must remain
> -	test -f .git/NOTES_MERGE_PARTIAL &&
> -	test -f .git/NOTES_MERGE_REF &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
> +	# .git/notes_merge_* must remain
> +	git rev-parse --verify notes_merge_partial &&
> +	git rev-parse --verify notes_merge_ref &&
> +	test -f .git/notes_merge_worktree/$commit_sha1 &&
> +	test -f .git/notes_merge_worktree/$commit_sha2 &&
> +	test -f .git/notes_merge_worktree/$commit_sha3 &&
> +	test -f .git/notes_merge_worktree/$commit_sha4 &&

> David Turner <dturner@twopensource.com> writes:
> 
> > All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
> > per-worktree.  We don't want multiple notes merges happening at once
> > (in different worktrees), so we want to make these refs true refs.
> >
> > So, we lowercase NOTES_MERGE_REF and friends.  That way, backends
> > that distinguish between pseudorefs and real refs can correctly
> > handle notes merges.
> >
> > This will also enable us to prevent pseudorefs from being updated by
> > the ref update machinery e.g. git update-ref.
> >
> > Signed-off-by: David Turner <dturner@twopensource.com>
> > ---
> 
> This seems to do a bit more than what it claims to do.  As this kind
> of changes are very error-prone to review, I did a bulk replace of
> the three all-caps NOTES_*THING* and compared the result with what
> this patch gives to spot this:
> 
> >  	# Fail to finalize merge
> >  	test_must_fail git notes merge --commit >output 2>&1 &&
> > -	# .git/NOTES_MERGE_* must remain
> > -	test -f .git/NOTES_MERGE_PARTIAL &&
> > -	test -f .git/NOTES_MERGE_REF &&
> > -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&
> > -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&
> > -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
> > -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
> > +	# .git/notes_merge_* must remain
> > +	git rev-parse --verify notes_merge_partial &&
> > +	git rev-parse --verify notes_merge_ref &&
> > +	test -f .git/notes_merge_worktree/$commit_sha1 &&
> > +	test -f .git/notes_merge_worktree/$commit_sha2 &&
> > +	test -f .git/notes_merge_worktree/$commit_sha3 &&
> > +	test -f .git/notes_merge_worktree/$commit_sha4 &&
> 
> The two "rev-parse --verify" looks semi-sensible [*1*];
> notes_merge_partial is all lowercase and it refers to
> $GIT_DIR/notes_merge_partial, because they are shared across working
> tree. 
> 
> But then why are $GIT_DIR/notes_merge_worktree/* still checked with
> "test -f"?  If they are not refs or ref-like things, why should they
> be downcased?  If they are, why not "rev-parse --verify"?

> [Footnote]
> 
> *1* I say "semi-" sensible, because it looks ugly.  All ref-like
>     things immediately below $GIT_DIR/ are all-caps by convention.
>     Perhaps it is a better idea to move it under refs/; "everything
>     under refs/ is shared across working trees" is probably a much
>     better rule than "all caps but HEAD is special".

> David Turner <dturner@twopensource.com> writes:
>
>> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
>> per-worktree.  We don't want multiple notes merges happening at once
>> (in different worktrees), so we want to make these refs true refs.
>> ...
> The two "rev-parse --verify" looks semi-sensible [*1*];
> notes_merge_partial is all lowercase and it refers to
> $GIT_DIR/notes_merge_partial, because they are shared across working
> tree. 
>
> But then why are $GIT_DIR/notes_merge_worktree/* still checked with
> "test -f"?  If they are not refs or ref-like things, why should they
> be downcased?  If they are, why not "rev-parse --verify"?
>
> [Footnote]
>
> *1* I say "semi-" sensible, because it looks ugly.  All ref-like
>     things immediately below $GIT_DIR/ are all-caps by convention.
>     Perhaps it is a better idea to move it under refs/; "everything
>     under refs/ is shared across working trees" is probably a much
>     better rule than "all caps but HEAD is special".

> Prevent merges to the same notes branch from different worktrees.
> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same
> code we use to check that two HEADs in different worktrees don't point
> to the same branch.  Modify that code, die_if_checked_out, to take a
> "head" ref to examine; previously, it just looked at HEAD.
> 
> Reported-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  branch.c                         | 15 +++++----
>  branch.h                         |  2 +-
>  builtin/checkout.c               |  2 +-
>  builtin/notes.c                  |  2 ++
>  builtin/worktree.c               |  2 +-
>  t/t3320-notes-merge-worktrees.sh | 71 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 84 insertions(+), 10 deletions(-)
>  create mode 100755 t/t3320-notes-merge-worktrees.sh
> 
> diff --git a/branch.c b/branch.c
> index c85be07..60eadc6 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -311,21 +311,22 @@ void remove_branch_state(void)
>  	unlink(git_path("SQUASH_MSG"));
>  }
>  
> -static void check_linked_checkout(const char *branch, const char *id)
> +static void check_linked_checkout(const char *head, const char *branch,
> +				  const char *id)
>  {
>  	struct strbuf sb = STRBUF_INIT;
>  	struct strbuf path = STRBUF_INIT;
>  	struct strbuf gitdir = STRBUF_INIT;
>  
>  	/*
> -	 * $GIT_COMMON_DIR/HEAD is practically outside
> +	 * $GIT_COMMON_DIR/$head is practically outside
>  	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
>  	 * uses git_path). Parse the ref ourselves.
>  	 */
>  	if (id)
> -		strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
> +		strbuf_addf(&path, "%s/worktrees/%s/%s", get_git_common_dir(), id, head);
>  	else
> -		strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
> +		strbuf_addf(&path, "%s/%s", get_git_common_dir(), head);
>  
>  	if (!strbuf_readlink(&sb, path.buf, 0)) {
>  		if (!starts_with(sb.buf, "refs/") ||
> @@ -356,13 +357,13 @@ done:
>  	strbuf_release(&gitdir);
>  }
>  
> -void die_if_checked_out(const char *branch)
> +void die_if_checked_out(const char *head, const char *branch)
>  {
>  	struct strbuf path = STRBUF_INIT;
>  	DIR *dir;
>  	struct dirent *d;
>  
> -	check_linked_checkout(branch, NULL);
> +	check_linked_checkout(head, branch, NULL);
>  
>  	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
>  	dir = opendir(path.buf);
> @@ -373,7 +374,7 @@ void die_if_checked_out(const char *branch)
>  	while ((d = readdir(dir)) != NULL) {
>  		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
>  			continue;
> -		check_linked_checkout(branch, d->d_name);
> +		check_linked_checkout(head, branch, d->d_name);
>  	}
>  	closedir(dir);
>  }
> diff --git a/branch.h b/branch.h
> index 58aa45f..3577fe5 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -57,6 +57,6 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);
>   * worktree and die (with a message describing its checkout location) if
>   * it is.
>   */
> -extern void die_if_checked_out(const char *branch);
> +extern void die_if_checked_out(const char *head, const char *branch);
>  
>  #endif
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e1403be..a5e9f2d 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1107,7 +1107,7 @@ static int checkout_branch(struct checkout_opts *opts,
>  		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
>  		if (head_ref &&
>  		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
> -			die_if_checked_out(new->path);
> +			die_if_checked_out("HEAD", new->path);
>  		free(head_ref);
>  	}
>  
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 63f95fc..8794ba1 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -19,6 +19,7 @@
>  #include "string-list.h"
>  #include "notes-merge.h"
>  #include "notes-utils.h"
> +#include "branch.h"
>  
>  static const char * const git_notes_usage[] = {
>  	N_("git notes [--ref <notes-ref>] [list [<object>]]"),
> @@ -829,6 +830,7 @@ static int merge(int argc, const char **argv, const char *prefix)
>  		update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, NULL,
>  			   0, UPDATE_REFS_DIE_ON_ERR);
>  		/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
> +		die_if_checked_out("NOTES_MERGE_REF", default_notes_ref());
>  		if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
>  			die("Failed to store link to current notes ref (%s)",
>  			    default_notes_ref());
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 430b51e..bfdb90f 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -203,7 +203,7 @@ static int add_worktree(const char *path, const char *refname,
>  	else if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
>  		 ref_exists(symref.buf)) { /* it's a branch */
>  		if (!opts->force)
> -			die_if_checked_out(symref.buf);
> +			die_if_checked_out("HEAD", symref.buf);
>  	} else { /* must be a commit */
>  		commit = lookup_commit_reference_by_name(refname);
>  		if (!commit)
> diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
> new file mode 100755
> index 0000000..b102c23
> --- /dev/null
> +++ b/t/t3320-notes-merge-worktrees.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2015 Twitter, Inc
> +#
> +
> +test_description='Test merging of notes trees in multiple worktrees'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup commit' '
> +	test_commit tantrum
> +'
> +
> +commit_tantrum=$(git rev-parse tantrum^{commit})
> +
> +test_expect_success 'setup notes ref (x)' '
> +	git config core.notesRef refs/notes/x &&
> +	git notes add -m "x notes on tantrum" tantrum
> +'
> +
> +test_expect_success 'setup local branch (y)' '
> +	git update-ref refs/notes/y refs/notes/x &&
> +	git config core.notesRef refs/notes/y &&
> +	git notes remove tantrum
> +'
> +
> +test_expect_success 'setup remote branch (z)' '
> +	git update-ref refs/notes/z refs/notes/x &&
> +	git config core.notesRef refs/notes/z &&
> +	git notes add -f -m "conflicting notes on tantrum" tantrum
> +'
> +
> +test_expect_success 'modify notes ref ourselves (x)' '
> +	git config core.notesRef refs/notes/x &&
> +	git notes add -f -m "more conflicting notes on tantrum" tantrum
> +'
> +
> +test_expect_success 'create some new worktrees' '
> +	git worktree add -b newbranch worktree master &&
> +	git worktree add -b newbranch2 worktree2 master
> +'
> +
> +test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
> +	git config core.notesRef refs/notes/y &&
> +	test_must_fail git notes merge z &&
> +	echo "ref: refs/notes/y" > expect &&
> +	test_cmp .git/NOTES_MERGE_REF expect
> +'
> +
> +test_expect_success 'merge z into y while mid-merge in another workdir fails' '
> +	(
> +		cd worktree &&
> +		git config core.notesRef refs/notes/y &&
> +		test_must_fail git notes merge z 2>err &&
> +		grep "is already checked out" err
> +	) &&
> +	test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
> +'
> +
> +test_expect_success 'merge z into x while mid-merge on y succeeds' '
> +	(
> +		cd worktree2 &&
> +		git config core.notesRef refs/notes/x &&
> +		test_must_fail git notes merge z 2>&1 >out &&
> +		grep -v "is already checked out" out
> +	) &&
> +	echo "ref: refs/notes/x" > expect &&
> +	test_cmp .git/worktrees/worktree2/NOTES_MERGE_REF expect
> +'
> +
> +test_done

> Prevent merges to the same notes branch from different worktrees.
> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same
> code we use to check that two HEADs in different worktrees don't point
> to the same branch.  Modify that code, die_if_checked_out, to take a
> "head" ref to examine; previously, it just looked at HEAD.
>
> Reported-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---

>  branch.c                         | 15 +++++----
>  branch.h                         |  2 +-
>  builtin/checkout.c               |  2 +-
>  builtin/notes.c                  |  2 ++
>  builtin/worktree.c               |  2 +-
>  t/t3320-notes-merge-worktrees.sh | 71 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 84 insertions(+), 10 deletions(-)
>  create mode 100755 t/t3320-notes-merge-worktrees.sh

> diff --git a/branch.c b/branch.c
> index c85be07..60eadc6 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -311,21 +311,22 @@ void remove_branch_state(void)
>  	unlink(git_path("SQUASH_MSG"));
>  }
>  
> -static void check_linked_checkout(const char *branch, const char *id)
> +static void check_linked_checkout(const char *head, const char *branch,
> +				  const char *id)
>  {
>  	struct strbuf sb = STRBUF_INIT;
>  	struct strbuf path = STRBUF_INIT;
>  	struct strbuf gitdir = STRBUF_INIT;
>  
>  	/*
> -	 * $GIT_COMMON_DIR/HEAD is practically outside
> +	 * $GIT_COMMON_DIR/$head is practically outside
>  	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
>  	 * uses git_path). Parse the ref ourselves.
>  	 */
>  	if (id)
> -		strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
> +		strbuf_addf(&path, "%s/worktrees/%s/%s", get_git_common_dir(), id, head);
>  	else
> -		strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
> +		strbuf_addf(&path, "%s/%s", get_git_common_dir(), head);
>  
>  	if (!strbuf_readlink(&sb, path.buf, 0)) {
>  		if (!starts_with(sb.buf, "refs/") ||
> @@ -356,13 +357,13 @@ done:
>  	strbuf_release(&gitdir);
>  }
>  
> -void die_if_checked_out(const char *branch)
> +void die_if_checked_out(const char *head, const char *branch)
>  {
>  	struct strbuf path = STRBUF_INIT;
>  	DIR *dir;
>  	struct dirent *d;
>  
> -	check_linked_checkout(branch, NULL);
> +	check_linked_checkout(head, branch, NULL);
>  
>  	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
>  	dir = opendir(path.buf);
> @@ -373,7 +374,7 @@ void die_if_checked_out(const char *branch)
>  	while ((d = readdir(dir)) != NULL) {
>  		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
>  			continue;
> -		check_linked_checkout(branch, d->d_name);
> +		check_linked_checkout(head, branch, d->d_name);
>  	}
>  	closedir(dir);
>  }
> diff --git a/branch.h b/branch.h
> index 58aa45f..3577fe5 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -57,6 +57,6 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);
>   * worktree and die (with a message describing its checkout location) if
>   * it is.
>   */
> -extern void die_if_checked_out(const char *branch);
> +extern void die_if_checked_out(const char *head, const char *branch);
>  
>  #endif
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e1403be..a5e9f2d 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1107,7 +1107,7 @@ static int checkout_branch(struct checkout_opts *opts,
>  		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
>  		if (head_ref &&
>  		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
> -			die_if_checked_out(new->path);
> +			die_if_checked_out("HEAD", new->path);
>  		free(head_ref);
>  	}
>  
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 63f95fc..8794ba1 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -19,6 +19,7 @@
>  #include "string-list.h"
>  #include "notes-merge.h"
>  #include "notes-utils.h"
> +#include "branch.h"
>  
>  static const char * const git_notes_usage[] = {
>  	N_("git notes [--ref <notes-ref>] [list [<object>]]"),
> @@ -829,6 +830,7 @@ static int merge(int argc, const char **argv, const char *prefix)
>  		update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, NULL,
>  			   0, UPDATE_REFS_DIE_ON_ERR);
>  		/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
> +		die_if_checked_out("NOTES_MERGE_REF", default_notes_ref());
>  		if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
>  			die("Failed to store link to current notes ref (%s)",
>  			    default_notes_ref());
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 430b51e..bfdb90f 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -203,7 +203,7 @@ static int add_worktree(const char *path, const char *refname,
>  	else if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
>  		 ref_exists(symref.buf)) { /* it's a branch */
>  		if (!opts->force)
> -			die_if_checked_out(symref.buf);
> +			die_if_checked_out("HEAD", symref.buf);
>  	} else { /* must be a commit */
>  		commit = lookup_commit_reference_by_name(refname);
>  		if (!commit)
> diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
> new file mode 100755
> index 0000000..b102c23
> --- /dev/null
> +++ b/t/t3320-notes-merge-worktrees.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2015 Twitter, Inc
> +#
> +
> +test_description='Test merging of notes trees in multiple worktrees'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup commit' '
> +	test_commit tantrum
> +'
> +
> +commit_tantrum=$(git rev-parse tantrum^{commit})
> +
> +test_expect_success 'setup notes ref (x)' '
> +	git config core.notesRef refs/notes/x &&
> +	git notes add -m "x notes on tantrum" tantrum
> +'
> +
> +test_expect_success 'setup local branch (y)' '
> +	git update-ref refs/notes/y refs/notes/x &&
> +	git config core.notesRef refs/notes/y &&
> +	git notes remove tantrum
> +'
> +
> +test_expect_success 'setup remote branch (z)' '
> +	git update-ref refs/notes/z refs/notes/x &&
> +	git config core.notesRef refs/notes/z &&
> +	git notes add -f -m "conflicting notes on tantrum" tantrum
> +'
> +
> +test_expect_success 'modify notes ref ourselves (x)' '
> +	git config core.notesRef refs/notes/x &&
> +	git notes add -f -m "more conflicting notes on tantrum" tantrum
> +'
> +
> +test_expect_success 'create some new worktrees' '
> +	git worktree add -b newbranch worktree master &&
> +	git worktree add -b newbranch2 worktree2 master
> +'
> +
> +test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
> +	git config core.notesRef refs/notes/y &&
> +	test_must_fail git notes merge z &&
> +	echo "ref: refs/notes/y" > expect &&
> +	test_cmp .git/NOTES_MERGE_REF expect
> +'
> +
> +test_expect_success 'merge z into y while mid-merge in another workdir fails' '
> +	(
> +		cd worktree &&
> +		git config core.notesRef refs/notes/y &&
> +		test_must_fail git notes merge z 2>err &&
> +		grep "is already checked out" err
> +	) &&
> +	test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
> +'
> +
> +test_expect_success 'merge z into x while mid-merge on y succeeds' '
> +	(
> +		cd worktree2 &&
> +		git config core.notesRef refs/notes/x &&
> +		test_must_fail git notes merge z 2>&1 >out &&
> +		grep -v "is already checked out" out
> +	) &&
> +	echo "ref: refs/notes/x" > expect &&
> +	test_cmp .git/worktrees/worktree2/NOTES_MERGE_REF expect
> +'
> +
> +test_done

> David Turner <dturner@twopensource.com> writes:
>> Prevent merges to the same notes branch from different worktrees.
>> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same
>> code we use to check that two HEADs in different worktrees don't point
>> to the same branch.  Modify that code, die_if_checked_out, to take a
>> "head" ref to examine; previously, it just looked at HEAD.
>>
>> Reported-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: David Turner <dturner@twopensource.com>
>> ---
>
> Thanks for following through.  As I didn't report anything, I do not
> deserve that label, but it's OK ;-)
>
> I know that it is a requirement to protect NOTES_MERGE_REF from
> being used by multiple places for "notes merge" to play well with
> the multi-worktree world order, but because I do not know if that is
> sufficient, I'm asking a few people for further review.

> On Wed, Jul 29, 2015 at 12:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> David Turner <dturner@twopensource.com> writes:
>>> Prevent merges to the same notes branch from different worktrees.
>>> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same
>>> code we use to check that two HEADs in different worktrees don't point
>>> to the same branch.  Modify that code, die_if_checked_out, to take a
>>> "head" ref to examine; previously, it just looked at HEAD.
>>>
>>> Reported-by: Junio C Hamano <gitster@pobox.com>
>>> Signed-off-by: David Turner <dturner@twopensource.com>
>>> ---
>>
>> Thanks for following through.  As I didn't report anything, I do not
>> deserve that label, but it's OK ;-)
>>
>> I know that it is a requirement to protect NOTES_MERGE_REF from
>> being used by multiple places for "notes merge" to play well with
>> the multi-worktree world order, but because I do not know if that is
>> sufficient, I'm asking a few people for further review.
>
> As just stated in a related thread, I don't think it makes sense to
> have NOTES_MERGE_REF per worktree, as the notes merge is always
> completely unrelated to the current worktree (or the current branch
> for that matter). AFAICS this patch is all about handling per-worktree
> NOTES_MERGE_REFs, and as such I'm NAK on this patch. Instead, there
> should only be one NOTES_MERGE_REF per _repo_, and although we might
> want to expand to allow multiple concurrent notes merges in the
> future, that is still a per-repo, and not a per-worktree thing, hence
> completely unrelated to David's current effort.

> Prevent merges to the same notes branch from different worktrees.
> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same
> code we use to check that two HEADs in different worktrees don't point
> to the same branch.  Modify that code, die_if_checked_out, to take a
> "head" ref to examine; previously, it just looked at HEAD.

> Reported-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
> diff --git a/branch.c b/branch.c
> index c85be07..60eadc6 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -311,21 +311,22 @@ void remove_branch_state(void)
>         unlink(git_path("SQUASH_MSG"));
>  }
>
> -static void check_linked_checkout(const char *branch, const char *id)
> +static void check_linked_checkout(const char *head, const char *branch,
> +                                 const char *id)
>  {
>         struct strbuf sb = STRBUF_INIT;
>         struct strbuf path = STRBUF_INIT;
>         struct strbuf gitdir = STRBUF_INIT;
>
>         /*
> -        * $GIT_COMMON_DIR/HEAD is practically outside
> +        * $GIT_COMMON_DIR/$head is practically outside
>          * $GIT_DIR so resolve_ref_unsafe() won't work (it
>          * uses git_path). Parse the ref ourselves.
>          */
>         if (id)
> -               strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
> +               strbuf_addf(&path, "%s/worktrees/%s/%s", get_git_common_dir(), id, head);
>         else
> -               strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
> +               strbuf_addf(&path, "%s/%s", get_git_common_dir(), head);
>
>         if (!strbuf_readlink(&sb, path.buf, 0)) {
>                 if (!starts_with(sb.buf, "refs/") ||
> @@ -356,13 +357,13 @@ done:
>         strbuf_release(&gitdir);
>  }
>
> -void die_if_checked_out(const char *branch)
> +void die_if_checked_out(const char *head, const char *branch)
>  {
>         struct strbuf path = STRBUF_INIT;
>         DIR *dir;
>         struct dirent *d;
>
> -       check_linked_checkout(branch, NULL);
> +       check_linked_checkout(head, branch, NULL);
>
>         strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
>         dir = opendir(path.buf);
> @@ -373,7 +374,7 @@ void die_if_checked_out(const char *branch)
>         while ((d = readdir(dir)) != NULL) {
>                 if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
>                         continue;
> -               check_linked_checkout(branch, d->d_name);
> +               check_linked_checkout(head, branch, d->d_name);
>         }
>         closedir(dir);
>  }
> diff --git a/branch.h b/branch.h
> index 58aa45f..3577fe5 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -57,6 +57,6 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);
>   * worktree and die (with a message describing its checkout location) if
>   * it is.
>   */
> -extern void die_if_checked_out(const char *branch);
> +extern void die_if_checked_out(const char *head, const char *branch);
>
>  #endif

> Junio C Hamano <gitster@pobox.com> writes:
>> David Turner <dturner@twopensource.com> writes:
>>> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
>>> per-worktree.  We don't want multiple notes merges happening at once
>>> (in different worktrees), so we want to make these refs true refs.
>>> ...
>> The two "rev-parse --verify" looks semi-sensible [*1*];
>> notes_merge_partial is all lowercase and it refers to
>> $GIT_DIR/notes_merge_partial, because they are shared across working
>> tree.
>>
>> But then why are $GIT_DIR/notes_merge_worktree/* still checked with
>> "test -f"?  If they are not refs or ref-like things, why should they
>> be downcased?  If they are, why not "rev-parse --verify"?
>>
>> [Footnote]
>>
>> *1* I say "semi-" sensible, because it looks ugly.  All ref-like
>>     things immediately below $GIT_DIR/ are all-caps by convention.
>>     Perhaps it is a better idea to move it under refs/; "everything
>>     under refs/ is shared across working trees" is probably a much
>>     better rule than "all caps but HEAD is special".
>
> I am not sure if "we don't want multiple notes merges at once" is a
> valid cause for this inconsistency in the first place.  When you try
> to start merging a notes tree in one place (leaving NOTES_MERGE_REF
> and friends in the ref storage shared across worktrees), should you
> be prevented from merging a different notes tree in another?  Isn't
> the whole point of having refs/notes/ hierarchy to allow different
> notes to hang underneath there, and isn't NOTES_MERGE_REF symref
> about keeping track of which one of them is being worked on in this
> working tree?

> We don't want multiple usual merges at once in different worktrees,
> either; rather, we don't want conflicting updates to the same branch,
> be it done by a merge or a single-parent commit, from different
> worktrees.  And the way we protect ourselves is by forbidding two
> checkout of the same branch by controlling what HEAD can point at.
>
> Making NOTES_MERGE_REF shared across working trees feels like
> solving that "no multiple checkouts" problem by making HEAD shared
> across working trees, ensuring that only one of them can have usable
> HEAD.  Instead, shouldn't we be solving the issue by allowing
> NOTES_MERGE_REF in multiple working trees point at different refs
> but ensuring that there is at most one working tree that updates one
> particular ref in refs/notes/ hierarchy?  Can't we learn from (and
> even better, reuse) the logic that controls HEAD for this purpose?
>
> I think it is OK for us to admit that the "notes" subsystem is not
> quite ready to work well with multiple working tree world yet [*1*],
> and move this series forward without worrying about them.
>
>
> [Footnote]
>
> *1* I am not saying that the notes subsystem can forever stay to be
>     second class citizen that does not know about multiple worktrees
>     or pluggable ref backends.  But my brief scan of builtin/notes.c
>     seems to indicate that there are quite a lot of work to be done
>     to prepare it for these two big changes.  My gut feeling is
>     that:
>
>     - NOTES_MERGE_REF symref is like HEAD, that is per working tree
>       but is controlled across working trees---no two working trees
>       can "checkout" the same refs/notes/* tree for conflict
>       resolution at the same time.

>
>     - NOTES_MERGE_PARTIAL is like MERGE_HEAD or the index in that it
>       is merely a state of in-progress merge that is private to a
>       single working tree.  $GIT_DIR/NOTES_MERGE_PARTIAL is just
>       fine, and we do not have to change it.

>
>     - NOTES_MERGE_WORKTREE is a temporary area in the filesystem
>       that houses bunch of blob files (i.e. notes contents).  It
>       should be per-working tree but it is not even refs.
>       $GIT_DIR/NOTES_MERGE_WORKTREE is just fine, and we do not have
>       to change it.

>
>     And as to adjusting to the "ref backend with pseudo ref" world,
>     I think the code is more-or-less ready, especially because your
>     recent round of this series teaches the "pseudorefs private to
>     working tree" to update_ref() and delete_ref(), relieving the
>     ref API users like notes subsystem from having to worry about
>     them.
>
>     So the only potential issue in the notes subsystem is to ensure
>     NOTES_MERGE_REF from multiple working trees do not point at the
>     same thing at the same time, with a similar protection we have
>     for HEAD, I think.

> Johan Herland <johan@herland.net> writes:
>> However, in any case, notes merges are always per _repo_ and never per
>> _worktree_, so this is all unrelated to the current patch/discussion
>> AFAICS.
>
> Thanks for chiming in, but I actually think you are confused.
>
> "git merge" is always per _repo_ in the sense that the result of a
> merge of a topic to the 'master' is recorded in the 'master' which
> is per-repo.  In the multi-worktree world order, that does not
> change.  What changes is that you could have different worktrees
> that check out different branches.  Worktree A may have 'master'
> checked out and do the merge there to update the tip of 'master'.
> But while worktree A is doing that, worktree B may have 'next'
> checked out and do an unrelated merge there.  Once worktree A leaves
> 'master' by checking out another branch, worktree B is free to check
> out 'master' and do further merges there.  Merging into 'master' is
> per _repo_, but the act of merging is per worktree.

>  It may
> conflict and you may need to "lock" refs/notes/commits from being
> touched by other worktrees while resolving that, but that does not
> mean other worktrees cannot do a merge of refs/notes/someotherthing
> at all.

> So while you are working on merging and resolving, that intermediate
> state is *NOT* per _repo_ at all.  It is at most per worktree (Yes
> you could extend and have one notes_merge_ref per each refs/notes/*
> ref to make it even finer grained to allow more than one notes merge
> going on inside a single worktree, but I do not think it is worth
> it).

> Here is where we start to differ. I would say that starting a notes
> merge is completely unrelated to your worktree. Consider this:
> ...
> This is not the case for notes merges. If I start a notes merge from
> worktree A, there is no "occupation" of that worktree. Before the
> notes merge is resolved, I can do whatever I want in worktree A
> (including checking out a different branch, performing a rebase,
> whatever...). Instead, the notes merge creates its own worktree (that
> is "occupied" until the notes merge is completed), which is completely
> unrelated to worktree A.

> Let's say I have two worktrees, A and B, and from worktree A, I have
> started a notes-merge into refs/notes/commits. Now:
>
>  - From worktree B I should NOT be able to start another notes-merge
> into refs/notes/commits.

> I can
> just as easily say:
>
>  - From worktree A I should NOT be able to start another notes-merge
> into refs/notes/commits.

> For now, it would make more
> sense to only allow a single notes-merge across all worktrees. I.e.
> when starting a notes-merge from ANY worktree, it should simply fail
> if there is an existing unresolved notes merge (no matter which
> worktree started that unresolved notes merge).

> Johan Herland <johan@herland.net> writes:
>> Here is where we start to differ. I would say that starting a notes
>> merge is completely unrelated to your worktree. Consider this:
>
> It sounds like what a notes merge really wants is a new linked worktree
> that has branch refs/notes/foo checked out:
>
> * This would allow multiple notes merges to take place at the same time
> provided they target different merge references.
>
> * This would prevent multiple notes merges to the same notes reference
> at the same time by the same mechanism that prevents the same branch
> from being checked out in two linked worktrees at the same time.
>
> It's just a thought; I have no idea whether it is practical...
>
> Michael
>

> Johan Herland <johan@herland.net> writes:
>> Here is where we start to differ. I would say that starting a notes
>> merge is completely unrelated to your worktree. Consider this:
>
> It sounds like what a notes merge really wants is a new linked worktree
> that has branch refs/notes/foo checked out:

> * This would allow multiple notes merges to take place at the same time
> provided they target different merge references.
>
> * This would prevent multiple notes merges to the same notes reference
> at the same time by the same mechanism that prevents the same branch
> from being checked out in two linked worktrees at the same time.
>
> It's just a thought; I have no idea whether it is practical...

> Michael
>
> --
> Michael Haggerty
> mhagger@alum.mit.edu
>

> Yes, almost. There are some complications with the concept of
> "checking out" a notes tree:
>
>  - The notes tree fanout must be flattened (so that when merging two
> note trees with different fanout, conflicting notes (e.g. deadbeef...
> and de/adbeef....) are turned into a file-level conflict in the notes
> merge worktree (i.e. contents with conflict markers in
> .git/NOTES_MERGE_WORKTREE/deadbeef...).

>> * This would allow multiple notes merges to take place at the same time
>> provided they target different merge references.
>>
>> * This would prevent multiple notes merges to the same notes reference
>> at the same time by the same mechanism that prevents the same branch
>> from being checked out in two linked worktrees at the same time.
>>
>> It's just a thought; I have no idea whether it is practical...
>
> I'm not sure whether it's worth trying to reuse the same linked
> worktree mechanism for notes trees, when (a) the concept of "checking
> out" a notes tree is so different, as explained above, and (b)
> currently the only use case for a notes worktree is during a notes
> merge.

> It sounds like what a notes merge really wants is a new linked worktree
> that has branch refs/notes/foo checked out:
>
> * This would allow multiple notes merges to take place at the same time
> provided they target different merge references.
>
> * This would prevent multiple notes merges to the same notes reference
> at the same time by the same mechanism that prevents the same branch
> from being checked out in two linked worktrees at the same time.
>
> It's just a thought; I have no idea whether it is practical...

> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> It sounds like what a notes merge really wants is a new linked worktree
>> that has branch refs/notes/foo checked out:
>>
>> * This would allow multiple notes merges to take place at the same time
>> provided they target different merge references.
>>
>> * This would prevent multiple notes merges to the same notes reference
>> at the same time by the same mechanism that prevents the same branch
>> from being checked out in two linked worktrees at the same time.
>>
>> It's just a thought; I have no idea whether it is practical...
>
> That was certainly one of the possibilities that crossed my mind.
>
> In any case, the primary thing I am interested in at this point is
> to unblock David's "prepare things so that we can put primary refs
> in a different ref backends more easily" topic, and I've already
> made my point a few messages ago upstream:
>
>     I think it is OK for us to admit that the "notes" subsystem is
>     not quite ready to work well with multiple working tree world
>     yet [*1*], and move this series forward without worrying about
>     them.
>
> So doing the absolute minimum, leaving the "now what can we do to
> improve notes-merge process?" outside the scope of the topic.

> On Wed, Jul 29, 2015 at 4:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> So doing the absolute minimum, leaving the "now what can we do to
>> improve notes-merge process?" outside the scope of the topic.
>
> That's exactly what I was also trying to do: David's topic should not
> have to deal with NOTES_MERGE_* at all. Simply leave it all as
> something that belongs in $GIT_COMMON_DIR, and let's solve concurrent
> unrelated notes merges as a wholly independent, separate topic.

> Johan Herland <johan@herland.net> writes:
>
>> Here is where we start to differ. I would say that starting a notes
>> merge is completely unrelated to your worktree. Consider this:
>> ...
>> This is not the case for notes merges. If I start a notes merge from
>> worktree A, there is no "occupation" of that worktree. Before the
>> notes merge is resolved, I can do whatever I want in worktree A
>> (including checking out a different branch, performing a rebase,
>> whatever...). Instead, the notes merge creates its own worktree (that
>> is "occupied" until the notes merge is completed), which is completely
>> unrelated to worktree A.
>
> That does not mean the notes merge that you started when you were
> sitting in worktree A has to be shared with worktree B, by say doing
> "vi .git/NOTES_MERGE_WORKTREE/$commit && git notes merge --commit".
>
> Also the above does not explain why it is sensible for you to forbid
> worktree B from doing an unrelated notes merge of a different ref
> under refs/notes/* while your worktree A is doing a notes merge.

>> In principle, I agree that an ongoing notes-merge into
>> refs/notes/someotherthing should be able to coexist with an ongoing
>> notes-merge into refs/notes/commits. However, it does not make sense
>> to bind those notes-merges to a specific worktree.
>
> The thing is, the choice is between per worktree or per repository.
> Taking the latter would mean you can only be doing one notes merge
> at a time, even though you prepared multiple worktrees so that you
> can work on different things at a time.  It is true that there is
> nothing inherent that ties a note merge to a worktree (a worktree is
> tied to a branch that is checed out, and there is no tie between a
> branch and a notes tree), but "not inherantly tied to" does not
> automatically mean "has to be one per repository".  You'd ideally
> want to allow N workspaces for N refs/notes/* refs.
>
> But people work in worktrees, and that is their unit of working
> space.  From that point of view, unless you are proposing a
> completely different design where the primary worktree can be used
> only for manipulating notes (hence, you can have worktrees for
> resolving refs/notes/A and refs/notes/B, in addition to the other
> worktrees you use to advance branches), treating NOTES_MERGE_REF as
> a per-worktree entity just like HEAD and the index is, would be the
> most sensible comporise.

> I believe it is a bad compromise. It complicates the code, and it
> provides a concurrent notes merges that is unnecessarily tied to (and
> dependent on) worktrees. For example, if I wanted to perform N
> concurrent notes merges in a project that happens to have a huge
> worktree, I would now have to create N copies of the huge worktree...

> Johan Herland <johan@herland.net> writes:
>
>> I believe it is a bad compromise. It complicates the code, and it
>> provides a concurrent notes merges that is unnecessarily tied to (and
>> dependent on) worktrees. For example, if I wanted to perform N
>> concurrent notes merges in a project that happens to have a huge
>> worktree, I would now have to create N copies of the huge worktree...
>
> Who said worktree has to be populated?  You should be able to have
> an absolutely empty checkout just like "clone --no-checkout".

> On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johan Herland <johan@herland.net> writes:
>>
>>> I believe it is a bad compromise. It complicates the code, and it
>>> provides a concurrent notes merges that is unnecessarily tied to (and
>>> dependent on) worktrees. For example, if I wanted to perform N
>>> concurrent notes merges in a project that happens to have a huge
>>> worktree, I would now have to create N copies of the huge worktree...
>>
>> Who said worktree has to be populated?  You should be able to have
>> an absolutely empty checkout just like "clone --no-checkout".
>
> IMHO that's an insane workaround that only serves to highlight the
> conceptual problems of binding notes merges (as they are implemented
> today) to worktrees.

> Johan Herland <johan@herland.net> writes:
>> On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Johan Herland <johan@herland.net> writes:
>>>
>>>> I believe it is a bad compromise. It complicates the code, and it
>>>> provides a concurrent notes merges that is unnecessarily tied to (and
>>>> dependent on) worktrees. For example, if I wanted to perform N
>>>> concurrent notes merges in a project that happens to have a huge
>>>> worktree, I would now have to create N copies of the huge worktree...
>>>
>>> Who said worktree has to be populated?  You should be able to have
>>> an absolutely empty checkout just like "clone --no-checkout".
>>
>> IMHO that's an insane workaround that only serves to highlight the
>> conceptual problems of binding notes merges (as they are implemented
>> today) to worktrees.
>
> Actually, the name "linked worktree" is probably a misnomer.
>
> There is nothing fundamental in the mechanism or in the concept that
> says that these multiple $GIT_DIR's must not be a bare one.  The
> main thing the separation between $GIT_DIR and $GIT_COMMON_DIR
> affords you is that you can have some things shared across them
> (e.g. refs/*, objects) while making others per $GIT_DIR (e.g. HEAD,
> index, etc.).
>
> With that in mind, it is not an insane workaround but a very natural
> mechanism suited exactly for what you want to do: using a feature
> (e.g. "notes merge") that currently can have at most one instance
> running at a time because it stores its state inside $GIT_DIR, and
> you want to have N concurrent one going.  You keep that "state per
> running instance" inside $GIT_DIR (i.e. not shared) and use the
> "linked worktree" mechanism to have multiple $GIT_DIR, connected
> to the same $GIT_COMMON_DIR.

>> But, whatever. This is unrelated to David's current effort, and I
>> don't want to hold that up, so please move along, nothing to see here.
>
> I need this part from an earlier message answered to unblock David's
> topic:
>
>     Now we are getting somewhere.  So is there more that is needed
>     than separating NOTES_MERGE_REF per worktree to make this work
>     (remember, multiple notes-merge in a single worktree is a
>     non-goal, just like multiple merge in a single worktree is not
>     supported today and will not be)?  Is there some other state
>     that is not captured by NOTES_MERGE_REF and friends that you
>     would end up recording a wrong merge result, if two worktrees
>     that have NOTES_MERGE_REF pointing at a different ref in
>     refs/notes/* try to do the notes-merge at the same time?

> If we do not change anything (not even applying the "[PATCH] notes: handle multiple worktrees" patch
> we are discussing), all these things prefixed with NOTES_ will
> become per $GIT_DIR with linked worktrees.
>
>     NOTES_EDITMSG, NOTES_MERGE_REF, NOTES_MERGE_PARTIAL,
>     NOTES_MERGE_WORKTREE
>
> The user could attempt to start different notes merges in her
> multiple $GIT_DIRs.  The question is to what degree we want to
> support her.
>
> Is it sufficient to have these per $GIT_DIR, when the user has two
> $GIT_DIRs connected to the same repository and wants to do two
> "notes merge" acting on different ref in refs/notes/*?  Or are there
> some other states in the shared part kept, which would be stomped on
> by simultaneously running "notes merge" instances in different
> $GIT_DIRs, that make this not to work?  Any other problems in the
> remainder of the current implementation of "notes merge"?

> If there are reasons/limitations that make simultaneous "notes
> merge" of different notes in different $GIT_DIRs impossible, then I
> agree we shouldn't bother with "[PATCH] notes: handle multiple worktrees" patch.  We should just
> declare "do not do it, it does not (yet) work".
>
> But if there isn't, "[PATCH] notes: handle multiple worktrees" is the absolute minimum thing we could
> do to make "notes merge" usable by making sure that the user does
> not attempt merging the same refs/notes/commits in two different
> places.

>> Actually, the name "linked worktree" is probably a misnomer.
>> ...
> Makes sense, although currently, IINM, those multiple $GIT_DIRs must
> be associated with strictly different branches, which is completely
> unrelated to the desired notes-merge restriction (which applies to
> notes refs - not branches). But this has been discussed to death,
> already.

>> The user could attempt to start different notes merges in her
>> multiple $GIT_DIRs.  The question is to what degree we want to
>> support her.
>>
>> Is it sufficient to have these per $GIT_DIR, when the user has two
>> $GIT_DIRs connected to the same repository and wants to do two
>> "notes merge" acting on different ref in refs/notes/*?  Or are there
>> some other states in the shared part kept, which would be stomped on
>> by simultaneously running "notes merge" instances in different
>> $GIT_DIRs, that make this not to work?  Any other problems in the
>> remainder of the current implementation of "notes merge"?
>
> Still, I believe the answer is "No".

>> But if there isn't, "[PATCH] notes: handle multiple worktrees" is
>> the absolute minimum thing we could
>> do to make "notes merge" usable by making sure that the user does
>> not attempt merging the same refs/notes/commits in two different
>> places.
>
> Sure. There's no point in delaying a patch that works well in practice
> just because I have a delusion of a theoretically cleaner solution
> that won't make any difference in practice.

> Add a function ref_type, which categorizes refs as per-worktree,
> pseudoref, or normal ref.
>
> Later, we will use this in refs.c to treat pseudorefs specially.
> Alternate ref backends may use it to treat both pseudorefs and
> per-worktree refs differently.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
> diff --git a/refs.c b/refs.c
> index 0b96ece..553ae8b 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2848,6 +2848,35 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)
>         return 0;
>  }
>
> +static int is_per_worktree_ref(const char *refname)
> +{
> +       return !strcmp(refname, "HEAD");
> +}
> +
> +static int is_pseudoref_syntax(const char *refname)
> +{
> +       const char *c;
> +
> +       if (strchr(refname, '/'))
> +               return 0;

> +               if (!isupper(*c) && *c != '-' && *c != '_')
> +                       return 0;
> +       }
> +
> +       return 1;
> +}
> +
> +enum ref_type ref_type(const char *refname)
> +{
> +       if (is_per_worktree_ref(refname))
> +               return REF_TYPE_PER_WORKTREE;
> +       if (is_pseudoref_syntax(refname))
> +               return REF_TYPE_PSEUDOREF;
> +       return REF_TYPE_NORMAL;
> +}

> Pseudorefs should not be updated through the ref transaction
> API, because alternate ref backends still need to store pseudorefs
> in GIT_DIR (instead of wherever they store refs).  Instead,
> change update_ref and delete_ref to call pseudoref-specific
> functions.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
> diff --git a/refs.c b/refs.c
> index 553ae8b..2bd6aa6 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2877,12 +2877,87 @@ enum ref_type ref_type(const char *refname)
> +static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1)
> +{
> +       static struct lock_file lock;
> +       const char *filename;
> +
> +       filename = git_path("%s", pseudoref);
> +
> +       if (old_sha1 && !is_null_sha1(old_sha1)) {
> +               int fd;
> +               unsigned char actual_old_sha1[20];
> +
> +               fd = hold_lock_file_for_update(&lock, filename,
> +                                              LOCK_DIE_ON_ERROR);
> +               if (fd < 0)
> +                       die_errno(_("Could not open '%s' for writing"), filename);
> +               read_ref(pseudoref, actual_old_sha1);
> +               if (hashcmp(actual_old_sha1, old_sha1)) {
> +                       warning("Unexpected sha1 when deleting %s", pseudoref);
> +                       return -1;

> +               }
> +
> +               unlink(filename);
> +               rollback_lock_file(&lock);
> +       } else {
> +               unlink(filename);
> +       }
> +
> +       return 0;
> +}