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

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:06:09

David Turner [off-list ref] 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.
quoted hunk
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.
quoted hunk
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?
quoted hunk
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.

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

From: David Turner <hidden>
Date: 2016-06-15 23:06:09

On Tue, 2015-08-11 at 14:10 -0700, Junio C Hamano wrote:
David Turner [off-list ref] writes:
quoted
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.
What does gitk do that's relevant here?
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.
There are a few reasons for refs/worktree hierarchy:

1. To make it easy to see the behavior of a ref at a glance.
2. To avoid too many different special-cases.

It's true that special-casing refs/bisect would make for fewer code
changes for now, but I am worried that next week we'll discover a new
situation where we'll want per-worktree refs and then we'll have to add
more special-cases.
quoted
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.
I will do this.  Should for-each-ref include or exclude these refs?  One
simple way to do "include" is to always create the refs/worktree
directory (when creating refs/).  

I'll also fix the rest of the things you suggested.

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

From: David Turner <hidden>
Date: 2016-06-15 23:06:09

On Tue, 2015-08-11 at 14:10 -0700, Junio C Hamano wrote:
David Turner [off-list ref] writes:
P.S. I noticed an issue with patch 2/2; I had reverted a now-unnecessary
hack, but accidentally reverted the whole file.  So I'll need to re-roll
anyway.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help