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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 11, 2015, 21:10 UTC
Message-ID: <xmqqvbclpm3h.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1439268979-10456-1-git-send-email-dturner@twopensource.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.

Show 15 quoted lines

> 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.

Show 11 quoted lines

> 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?

Show 16 quoted lines

> 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.

Show 5 quoted lines

> +	) &&
> +	! 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.