Re* [PATCH v2 1/2] refs: refs/worktree/* become per-worktree

[Junio C Hamano <gitster@pobox.com>]

David Turner <dturner@twopensource.com> writes:

> We need a place to stick refs for bisects in progress that is not
> shared between worktrees.  So we use the refs/worktree/ hierarchy.

This is by itself OK, but to help existing Porcelains, most notably
"gitk", I think refs/bisect/ hierarchy should be treated the same
way.  Moving them out of refs/bisect/ to refs/worktree/bisect/ would
not be a good idea.

Because refs/worktree/ hierarchy is not needed right now, I think we
can even live with just special casing refs/bisect/ the way this
patch does, without adding refs/worktree/ support at this point.

> Note that git for-each-ref may have inconsistent behavior (I think; I
> haven't confirmed this), sometimes showing refs/worktree/* and sometimes
> not.  In the long run, we should fix this, but right now, I don't know
> that it matters, since the only refs affected are these bisect refs.

We should fix that before this hits 'master', preferrably before
this hits 'next', especially if we add support for the more generic
refs/worktree/.  If it is only for refs/bisect/, we might be OK, but
I didn't think things through.

> diff --git a/path.c b/path.c
> index 10f4cbf..da0f767 100644
> --- a/path.c
> +++ b/path.c
> @@ -92,8 +92,9 @@ static void replace_dir(struct strbuf *buf, int len, const char *newdir)
>  }
>  
>  static const char *common_list[] = {
> +	"/refs", /* special case, since refs/worktree/ is per-worktree */
>  	"/branches", "/hooks", "/info", "!/logs", "/lost-found",
> -	"/objects", "/refs", "/remotes", "/worktrees", "/rr-cache", "/svn",
> +	"/objects", "/remotes", "/worktrees", "/rr-cache", "/svn",
>  	"config", "!gc.pid", "packed-refs", "shallow",
>  	NULL
>  };

This is not a new problem, but we probably should use a data
structure that is more performant than a linear list for this.

Also "!gc.pid" hack should be cleaned up.  It does not make much
sense to force all the calls to git_path() pay the price of checking
if each element in common-list begins with '!' and skip, even though
that '!' hack is only used in count-objects and nowhere else.

> diff --git a/refs.c b/refs.c
> index e6fc3fe..d43bfe1 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2656,6 +2656,10 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)
>  	struct ref_entry *packed_entry;
>  	int is_tag_ref = starts_with(entry->name, "refs/tags/");
>  
> +	/* Do not pack per-worktree refs: */
> +	if (starts_with(entry->name, "refs/worktree/"))
> +		return 0;

This should use is_per_worktree_ref(), no?

> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index 9d21c19..c9fd1ca 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -1131,4 +1131,20 @@ test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches
>  )
>  '
>  
> +test_expect_success 'handle per-worktree refs in refs/worktree' '
> +	git commit --allow-empty -m "initial commit" &&
> +	git worktree add -b branch worktree &&
> +	(
> +		cd worktree &&
> +		git commit --allow-empty -m "test commit"  &&
> +		git update-ref refs/worktree/something HEAD &&
> +		git rev-parse refs/worktree/something > ../worktree-head

Redirection '>', '>>', '<' etc. sticks to the pathname that comes
after it.

> +	) &&
> +	! test -e .git/refs/worktree &&
> +	test_must_fail git rev-parse refs/worktree/something &&
> +	git update-ref refs/worktree/something HEAD &&
> +	git rev-parse refs/worktree/something > main-head &&

Ditto.

Thanks.