Fix git-gc losing objects in multi worktree

PATCH Superseded Nguyễn Thái Ngọc Duy 3 participants 33 messages Feb 17, 2017 – Apr 20, 2017 Documentation

Share & tools

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:19 UTC #0 lore raw

cover Fix git-gc losing objects in multi worktree

So here is my latest attempt on fixing this issue. For people who are not aware of it, git-gc does not take per-worktree refs, reflogs and indexes into account. An odb prune may leave HEAD and references in other worktrees pointing to nowhere.

This series is based on my "kill parse_ref()" series [1], which is based on yet another one, which is on top of mh/submodule-hash. But you can get everything from my github [2].

The series introduces a new set of refs_* API and replaces the old *_submodule() one, opening up the opportunity to access refs from another worktree. rev-list learns a new option, --single-worktree, to control the new behavior.

reflog iterator from files-backend.c does not support per-worktree items, so it's updated here to do that. It still looks ugly, but I think this is a good "middle ground" until compound ref store comes. At that point we can separate "single worktree" ref store vs "linked worktree" one.

I'm adding Stefan here as well since I added a new FIXME in submodule.c in 11/15. I think it's ok (again, for now). But another look from submodule people would be much better.

[1] http://public-inbox.org/git/%[email protected]%3E/
[2] https://github.com/pclouds/git/commits/prune-in-worktrees-2

Nguyễn Thái Ngọc Duy (15):
revision.h: new flag in struct rev_info wrt. worktree-related refs revision.c: refactor add_index_objects_to_pending() revision.c: --indexed-objects add objects from all worktrees
refs: move submodule slash stripping code to get_submodule_ref_store
refs: add refs_read_ref[_full]()
refs: add refs_head_ref()
refs: add refs_for_each_ref()
refs: add a refs_for_each_in() and friends
revision.c: use refs_for_each*() instead of for_each_*_submodule()
refs: remove dead for_each_*_submodule()
revision.c: --all adds HEAD from all worktrees
refs: add refs_for_each_reflog[_ent]()
files-backend: make reflog iterator go through per-worktree reflog revision.c: --reflog add HEAD reflog from all worktrees
rev-list: expose and document --single-worktree

 Documentation/rev-list-options.txt |   8 ++
 reachable.c                        |   1 +
 refs.c                             | 171 ++++++++++++++++++++++++-------------
 refs.h                             |  25 ++++--
 refs/files-backend.c               |  24 +++++-
 revision.c                         | 130 +++++++++++++++++++++++-----
 revision.h                         |   1 +
 submodule.c                        |   2 +
 t/t5304-prune.sh                   |  37 ++++++++
 9 files changed, 305 insertions(+), 94 deletions(-)

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:19 UTC #1 lore raw

PATCH 1/15 revision.h: new flag in struct rev_info wrt. worktree-related refs

The revision walker can walk through per-worktree refs like HEAD or SHA-1 references in the index. These currently are from the current worktree only. This new flag is added to change rev-list behavior in this regard:

When single_worktree is set, only current worktree is considered. When it is not set (which is the default), all worktrees are considered.

The default is chosen so because the two big components that rev-list works with are object database (entirely shared between worktrees) and refs (mostly shared). It makes sense that default behavior goes per-repo too instead of per-worktree.

The flag will eventually be exposed as a rev-list argument with documents. For now it stays internal until the new behavior is fully implemented.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 revision.h | 1 +
 1 file changed, 1 insertion(+)

revision.h +1 -0

index 9fac1a607..c851b94ad 100644
@@ -88,6 +88,7 @@ struct rev_info {
 			topo_order:1,
 			simplify_merges:1,
 			simplify_by_decoration:1,
+			single_worktree:1,
 			tag_objects:1,
 			tree_objects:1,
 			blob_objects:1,

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:19 UTC #2 lore raw

PATCH 2/15 revision.c: refactor add_index_objects_to_pending()

In reply to Nguyễn Thái Ngọc Duy

The core code is factored out and take 'struct index_state *' instead so that we can reuse it to add objects from index files other than .git/index in the next patch.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 revision.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

revision.c +12 -6

index b37dbec37..ece868a25 100644
@@ -1263,13 +1263,13 @@ static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,
 
 }
 
-void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)
+static void do_add_index_objects_to_pending(struct rev_info *revs,
+					    struct index_state *istate)
 {
 	int i;
 
-	read_cache();
-	for (i = 0; i < active_nr; i++) {
-		struct cache_entry *ce = active_cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 		struct blob *blob;
 
 		if (S_ISGITLINK(ce->ce_mode))

@@ -1282,13 +1282,19 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)
 					     ce->ce_mode, ce->name);
 	}
 
-	if (active_cache_tree) {
+	if (istate->cache_tree) {
 		struct strbuf path = STRBUF_INIT;
-		add_cache_tree(active_cache_tree, revs, &path);
+		add_cache_tree(istate->cache_tree, revs, &path);
 		strbuf_release(&path);
 	}
 }
 
+void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
+{
+	read_cache();
+	do_add_index_objects_to_pending(revs, &the_index);
+}
+
 static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
 			    int exclude_parent)
 {

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:19 UTC #3 lore raw

PATCH 3/15 revision.c: --indexed-objects add objects from all worktrees

In reply to Nguyễn Thái Ngọc Duy

This is the result of single_worktree flag never being set (no way to up until now). To get objects from current index only, set single_worktree.

The other add_index_objects_to_pending's caller is mark_reachable_objects() (e.g. "git prune") which also mark objects from all indexes.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 revision.c       | 21 +++++++++++++++++++++
 t/t5304-prune.sh |  9 +++++++++
 2 files changed, 30 insertions(+)

revision.c +21 -0

index ece868a25..d82f72ff3 100644
@@ -19,6 +19,7 @@
 #include "dir.h"
 #include "cache-tree.h"
 #include "bisect.h"
+#include "worktree.h"
 
 volatile show_early_output_fn_t show_early_output;

@@ -1291,8 +1292,28 @@ static void do_add_index_objects_to_pending(struct rev_info *revs,
 
 void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
 {
+	struct worktree **worktrees, **p;
+
 	read_cache();
 	do_add_index_objects_to_pending(revs, &the_index);
+
+	if (revs->single_worktree)
+		return;
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+		struct index_state istate = {0};
+
+		if (wt->is_current)
+			continue; /* current index already taken care of */
+
+		if (read_index_from(&istate,
+				    worktree_git_path(wt, "index")) > 0)
+			do_add_index_objects_to_pending(revs, &istate);
+		discard_index(&istate);
+	}
+	free_worktrees(worktrees);
 }
 
 static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,

t/t5304-prune.sh +9 -0

index 133b5842b..cba45c7be 100755
@@ -283,4 +283,13 @@ test_expect_success 'prune: handle alternate object database' '
 	git -C B prune
 '
 
+test_expect_success 'prune: handle index in multiple worktrees' '
+	git worktree add second-worktree &&
+	echo "new blob for second-worktree" >second-worktree/blob &&
+	git -C second-worktree add blob &&
+	git prune --expire=now &&
+	git -C second-worktree show :blob >actual &&
+	test_cmp second-worktree/blob actual
+'
+
 test_done

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #4 lore raw

PATCH 4/15 refs: move submodule slash stripping code to get_submodule_ref_store

In reply to Nguyễn Thái Ngọc Duy

This is a better place that will benefit all submodule callers instead of just resolve_gitlink_ref()

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

refs.c +20 ~~-18~~

index 23e0a8eda..9c86c44b8 100644
@@ -1321,25 +1321,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 int resolve_gitlink_ref(const char *submodule, const char *refname,
 			unsigned char *sha1)
 {
-	size_t len = strlen(submodule);
 	struct ref_store *refs;
 	int flags;
 
-	while (len && submodule[len - 1] == '/')
-		len--;
-
-	if (!len)
-		return -1;
-
-	if (submodule[len]) {
-		/* We need to strip off one or more trailing slashes */
-		char *stripped = xmemdupz(submodule, len);
-
-		refs = get_submodule_ref_store(stripped);
-		free(stripped);
-	} else {
-		refs = get_submodule_ref_store(submodule);
-	}
+	refs = get_submodule_ref_store(submodule);
 
 	if (!refs)
 		return -1;

@@ -1458,7 +1443,17 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 {
 	struct strbuf submodule_sb = STRBUF_INIT;
 	struct ref_store *refs;
+	char *to_free = NULL;
 	int ret;
+	size_t len;
+
+	if (submodule) {
+		len = strlen(submodule);
+		while (len && submodule[len - 1] == '/')
+			len--;
+		if (!len)
+			submodule = NULL;
+	}
 
 	if (!submodule || !*submodule) {
 		/*

@@ -1468,15 +1463,19 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 		return get_main_ref_store();
 	}
 
+	if (submodule[len])
+		/* We need to strip off one or more trailing slashes */
+		submodule = to_free = xmemdupz(submodule, len);
+
 	refs = lookup_submodule_ref_store(submodule);
 	if (refs)
-		return refs;
+		goto done;
 
 	strbuf_addstr(&submodule_sb, submodule);
 	ret = is_nonbare_repository_dir(&submodule_sb);
 	strbuf_release(&submodule_sb);
 	if (!ret)
-		return refs;
+		goto done;
 
 	ret = submodule_to_gitdir(&submodule_sb, submodule);
 	if (!ret)

@@ -1485,6 +1484,9 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 
 	if (refs)
 		register_submodule_ref_store(refs, submodule);
+
+done:
+	free(to_free);
 	return refs;
 }

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #5 lore raw

PATCH 5/15 refs: add refs_read_ref[_full]()

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs.c | 19 ++++++++++++++++---
 refs.h |  5 +++++
 2 files changed, 21 insertions(+), 3 deletions(-)

refs.c +16 -3

index 9c86c44b8..06890db5d 100644
@@ -186,16 +186,29 @@ struct ref_filter {
 	void *cb_data;
 };
 
-int read_ref_full(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)
+int refs_read_ref_full(struct ref_store *refs,
+		       const char *refname, int resolve_flags,
+		       unsigned char *sha1, int *flags)
 {
-	if (resolve_ref_unsafe(refname, resolve_flags, sha1, flags))
+	if (refs_resolve_ref_unsafe(refs, refname, resolve_flags, sha1, flags))
 		return 0;
 	return -1;
 }
 
+int read_ref_full(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)
+{
+	return refs_read_ref_full(get_main_ref_store(), refname,
+				  resolve_flags, sha1, flags);
+}
+
+int refs_read_ref(struct ref_store *refs, const char *refname, unsigned char *sha1)
+{
+	return refs_read_ref_full(refs, refname, RESOLVE_REF_READING, sha1, NULL);
+}
+
 int read_ref(const char *refname, unsigned char *sha1)
 {
-	return read_ref_full(refname, RESOLVE_REF_READING, sha1, NULL);
+	return refs_read_ref(get_main_ref_store(), refname, sha1);
 }
 
 int ref_exists(const char *refname)

refs.h +5 -0

index bce77891a..229a97f59 100644
@@ -568,5 +568,10 @@ int refs_create_symref(struct ref_store *refs,
 		       const char *refname,
 		       const char *target,
 		       const char *logmsg);
+int refs_read_ref_full(struct ref_store *refs,
+		       const char *refname, int resolve_flags,
+		       unsigned char *sha1, int *flags);
+int refs_read_ref(struct ref_store *refs,
+		  const char *refname, unsigned char *sha1);
 
 #endif /* REFS_H */

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #6 lore raw

PATCH 6/15 refs: add refs_head_ref()

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs.c | 19 +++++++++----------
 refs.h |  1 +
 2 files changed, 10 insertions(+), 10 deletions(-)

refs.c +9 ~~-10~~

index 06890db5d..26758b8cf 100644
@@ -1139,27 +1139,26 @@ int rename_ref_available(const char *old_refname, const char *new_refname)
 	return ok;
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
 	struct object_id oid;
 	int flag;
 
-	if (submodule) {
-		if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
-			return fn("HEAD", &oid, 0, cb_data);
-
-		return 0;
-	}
-
-	if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
+	if (!refs_read_ref_full(refs, "HEAD", RESOLVE_REF_READING,
+				oid.hash, &flag))
 		return fn("HEAD", &oid, flag, cb_data);
 
 	return 0;
 }
 
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+{
+	return refs_head_ref(get_submodule_ref_store(submodule), fn, cb_data);
+}
+
 int head_ref(each_ref_fn fn, void *cb_data)
 {
-	return head_ref_submodule(NULL, fn, cb_data);
+	return refs_head_ref(get_main_ref_store(), fn, cb_data);
 }
 
 /*

refs.h +1 -0

index 229a97f59..54c038e3c 100644
@@ -573,5 +573,6 @@ int refs_read_ref_full(struct ref_store *refs,
 		       unsigned char *sha1, int *flags);
 int refs_read_ref(struct ref_store *refs,
 		  const char *refname, unsigned char *sha1);
+int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
 
 #endif /* REFS_H */

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #7 lore raw

PATCH 7/15 refs: add refs_for_each_ref()

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs.c | 33 ++++++++++++++++++++++-----------
 refs.h |  1 +
 2 files changed, 23 insertions(+), 11 deletions(-)

refs.c +22 ~~-11~~

index 26758b8cf..fc6cca3db 100644
@@ -1170,10 +1170,9 @@ int head_ref(each_ref_fn fn, void *cb_data)
  * non-zero value, stop the iteration and return that value;
  * otherwise, return 0.
  */
-static int do_for_each_ref(const char *submodule, const char *prefix,
+static int do_for_each_ref(struct ref_store *refs, const char *prefix,
 			   each_ref_fn fn, int trim, int flags, void *cb_data)
 {
-	struct ref_store *refs = get_submodule_ref_store(submodule);
 	struct ref_iterator *iter;
 
 	if (!refs)

@@ -1185,19 +1184,26 @@ static int do_for_each_ref(const char *submodule, const char *prefix,
 	return do_for_each_ref_iterator(iter, fn, cb_data);
 }
 
+int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+{
+	return do_for_each_ref(refs, "", fn, 0, 0, cb_data);
+}
+
 int for_each_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(NULL, "", fn, 0, 0, cb_data);
+	return do_for_each_ref(get_main_ref_store(), "", fn, 0, 0, cb_data);
 }
 
 int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(submodule, "", fn, 0, 0, cb_data);
+	return do_for_each_ref(get_submodule_ref_store(submodule),
+			       "", fn, 0, 0, cb_data);
 }
 
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(NULL, prefix, fn, strlen(prefix), 0, cb_data);
+	return do_for_each_ref(get_main_ref_store(),
+			       prefix, fn, strlen(prefix), 0, cb_data);
 }
 
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)

@@ -1206,19 +1212,23 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig
 
 	if (broken)
 		flag = DO_FOR_EACH_INCLUDE_BROKEN;
-	return do_for_each_ref(NULL, prefix, fn, 0, flag, cb_data);
+	return do_for_each_ref(get_main_ref_store(),
+			       prefix, fn, 0, flag, cb_data);
 }
 
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 		each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(submodule, prefix, fn, strlen(prefix), 0, cb_data);
+	return do_for_each_ref(get_submodule_ref_store(submodule),
+			       prefix, fn, strlen(prefix), 0, cb_data);
 }
 
 int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(NULL, git_replace_ref_base, fn,
-			       strlen(git_replace_ref_base), 0, cb_data);
+	return do_for_each_ref(get_main_ref_store(),
+			       git_replace_ref_base, fn,
+			       strlen(git_replace_ref_base),
+			       0, cb_data);
 }
 
 int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)

@@ -1226,14 +1236,15 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
 	strbuf_addf(&buf, "%srefs/", get_git_namespace());
-	ret = do_for_each_ref(NULL, buf.buf, fn, 0, 0, cb_data);
+	ret = do_for_each_ref(get_main_ref_store(),
+			      buf.buf, fn, 0, 0, cb_data);
 	strbuf_release(&buf);
 	return ret;
 }
 
 int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(NULL, "", fn, 0,
+	return do_for_each_ref(get_main_ref_store(), "", fn, 0,
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
 }

refs.h +1 -0

index 54c038e3c..8e3b4e839 100644
@@ -574,5 +574,6 @@ int refs_read_ref_full(struct ref_store *refs,
 int refs_read_ref(struct ref_store *refs,
 		  const char *refname, unsigned char *sha1);
 int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
 
 #endif /* REFS_H */

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #8 lore raw

PATCH 8/15 refs: add a refs_for_each_in() and friends

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs.c | 39 +++++++++++++++++++++++++++++++--------
 refs.h |  5 +++++
 2 files changed, 36 insertions(+), 8 deletions(-)

refs.c +31 -8

index fc6cca3db..37b03d4ff 100644
@@ -300,34 +300,52 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_li
 	for_each_rawref(warn_if_dangling_symref, &data);
 }
 
+int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+{
+	return refs_for_each_ref_in(refs, "refs/tags/", fn, cb_data);
+}
+
 int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
-	return for_each_ref_in("refs/tags/", fn, cb_data);
+	return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
 }
 
 int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
-	return for_each_ref_in_submodule(submodule, "refs/tags/", fn, cb_data);
+	return refs_for_each_tag_ref(get_submodule_ref_store(submodule),
+				     fn, cb_data);
+}
+
+int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+{
+	return refs_for_each_ref_in(refs, "refs/heads/", fn, cb_data);
 }
 
 int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
-	return for_each_ref_in("refs/heads/", fn, cb_data);
+	return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
 }
 
 int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
-	return for_each_ref_in_submodule(submodule, "refs/heads/", fn, cb_data);
+	return refs_for_each_branch_ref(get_submodule_ref_store(submodule),
+					fn, cb_data);
+}
+
+int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+{
+	return refs_for_each_ref_in(refs, "refs/remotes/", fn, cb_data);
 }
 
 int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
-	return for_each_ref_in("refs/remotes/", fn, cb_data);
+	return refs_for_each_remote_ref(get_main_ref_store(), fn, cb_data);
 }
 
 int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
-	return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, cb_data);
+	return refs_for_each_remote_ref(get_submodule_ref_store(submodule),
+					fn, cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn fn, void *cb_data)

@@ -1200,10 +1218,15 @@ int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 			       "", fn, 0, 0, cb_data);
 }
 
+int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
+			 each_ref_fn fn, void *cb_data)
+{
+	return do_for_each_ref(refs, prefix, fn, strlen(prefix), 0, cb_data);
+}
+
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(get_main_ref_store(),
-			       prefix, fn, strlen(prefix), 0, cb_data);
+	return refs_for_each_ref_in(get_main_ref_store(), prefix, fn, cb_data);
 }
 
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)

refs.h +5 -0

index 8e3b4e839..8fc82deda 100644
@@ -575,5 +575,10 @@ int refs_read_ref(struct ref_store *refs,
 		  const char *refname, unsigned char *sha1);
 int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
 int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
+			 each_ref_fn fn, void *cb_data);
+int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
 
 #endif /* REFS_H */

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #9 lore raw

PATCH 9/15 revision.c: use refs_for_each*() instead of for_each_*_submodule()

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 revision.c | 48 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 16 deletions(-)

revision.c +32 ~~-16~~

index d82f72ff3..d82c37b44 100644
@@ -1189,12 +1189,19 @@ void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)
 	string_list_append(*ref_excludes_p, exclude);
 }
 
-static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags,
-		int (*for_each)(const char *, each_ref_fn, void *))
+static void handle_refs(struct ref_store *refs,
+			struct rev_info *revs, unsigned flags,
+			int (*for_each)(struct ref_store *, each_ref_fn, void *))
 {
 	struct all_refs_cb cb;
+
+	if (!refs) {
+		/* this could happen with uninitialized submodules */
+		return;
+	}
+
 	init_all_refs_cb(&cb, revs, flags);
-	for_each(submodule, handle_one_ref, &cb);
+	for_each(refs, handle_one_ref, &cb);
 }
 
 static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)

@@ -2067,23 +2074,25 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 	ctx->argc -= n;
 }
 
-static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data, const char *term) {
+static int for_each_bisect_ref(struct ref_store *refs, each_ref_fn fn,
+			       void *cb_data, const char *term)
+{
 	struct strbuf bisect_refs = STRBUF_INIT;
 	int status;
 	strbuf_addf(&bisect_refs, "refs/bisect/%s", term);
-	status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+	status = refs_for_each_ref_in(refs, bisect_refs.buf, fn, cb_data);
 	strbuf_release(&bisect_refs);
 	return status;
 }
 
-static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int for_each_bad_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
-	return for_each_bisect_ref(submodule, fn, cb_data, term_bad);
+	return for_each_bisect_ref(refs, fn, cb_data, term_bad);
 }
 
-static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
-	return for_each_bisect_ref(submodule, fn, cb_data, term_good);
+	return for_each_bisect_ref(refs, fn, cb_data, term_good);
 }
 
 static int handle_revision_pseudo_opt(const char *submodule,

@@ -2092,8 +2101,14 @@ static int handle_revision_pseudo_opt(const char *submodule,
 {
 	const char *arg = argv[0];
 	const char *optarg;
+	struct ref_store *refs;
 	int argcount;
 
+	if (submodule) {
+		refs = get_submodule_ref_store(submodule);
+	} else
+		refs = get_main_ref_store();
+
 	/*
 	 * NOTE!
 	 *

@@ -2105,22 +2120,23 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	 * register it in the list at the top of handle_revision_opt.
 	 */
 	if (!strcmp(arg, "--all")) {
-		handle_refs(submodule, revs, *flags, for_each_ref_submodule);
-		handle_refs(submodule, revs, *flags, head_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_ref);
+		handle_refs(refs, revs, *flags, refs_head_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
-		handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_branch_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--bisect")) {
 		read_bisect_terms(&term_bad, &term_good);
-		handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
-		handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
+		handle_refs(refs, revs, *flags, for_each_bad_bisect_ref);
+		handle_refs(refs, revs, *flags ^ (UNINTERESTING | BOTTOM),
+			    for_each_good_bisect_ref);
 		revs->bisect = 1;
 	} else if (!strcmp(arg, "--tags")) {
-		handle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_tag_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--remotes")) {
-		handle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_remote_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #10 lore raw

PATCH 10/15 refs: remove dead for_each_*_submodule()

In reply to Nguyễn Thái Ngọc Duy

These are used in revision.c. After the last patch they are replaced with the refs_ version. Delete them (except for_each_remote_ref_submodule which is still used by submodule.c)

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs.c | 30 ------------------------------
 refs.h |  9 ---------
 2 files changed, 39 deletions(-)

refs.c +0 ~~-30~~

index 37b03d4ff..fa2df7a1d 100644
@@ -310,12 +310,6 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 	return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
 }
 
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
-	return refs_for_each_tag_ref(get_submodule_ref_store(submodule),
-				     fn, cb_data);
-}
-
 int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
 	return refs_for_each_ref_in(refs, "refs/heads/", fn, cb_data);

@@ -326,12 +320,6 @@ int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 	return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
 }
 
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
-	return refs_for_each_branch_ref(get_submodule_ref_store(submodule),
-					fn, cb_data);
-}
-
 int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
 	return refs_for_each_ref_in(refs, "refs/remotes/", fn, cb_data);

@@ -1169,11 +1157,6 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 	return 0;
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
-	return refs_head_ref(get_submodule_ref_store(submodule), fn, cb_data);
-}
-
 int head_ref(each_ref_fn fn, void *cb_data)
 {
 	return refs_head_ref(get_main_ref_store(), fn, cb_data);

@@ -1212,12 +1195,6 @@ int for_each_ref(each_ref_fn fn, void *cb_data)
 	return do_for_each_ref(get_main_ref_store(), "", fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
-	return do_for_each_ref(get_submodule_ref_store(submodule),
-			       "", fn, 0, 0, cb_data);
-}
-
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
 			 each_ref_fn fn, void *cb_data)
 {

@@ -1239,13 +1216,6 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig
 			       prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-		each_ref_fn fn, void *cb_data)
-{
-	return do_for_each_ref(get_submodule_ref_store(submodule),
-			       prefix, fn, strlen(prefix), 0, cb_data);
-}
-
 int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(get_main_ref_store(),

refs.h +0 -9

index 8fc82deda..986d408bd 100644
@@ -202,15 +202,6 @@ int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
-int for_each_ref_submodule(const char *submodule,
-			   each_ref_fn fn, void *cb_data);
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-		each_ref_fn fn, void *cb_data);
-int for_each_tag_ref_submodule(const char *submodule,
-			       each_ref_fn fn, void *cb_data);
-int for_each_branch_ref_submodule(const char *submodule,
-				  each_ref_fn fn, void *cb_data);
 int for_each_remote_ref_submodule(const char *submodule,
 				  each_ref_fn fn, void *cb_data);

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #11 lore raw

PATCH 11/15 revision.c: --all adds HEAD from all worktrees

In reply to Nguyễn Thái Ngọc Duy

Unless single_worktree is set, --all now adds HEAD from all worktrees.

Since reachable.c code does not use setup_revisions(), we need to call other_head_refs_submodule() explicitly there to have the same effect on "git prune", so that we won't accidentally delete objects needed by some other HEADs.

A new FIXME is added because we would need something like

    int refs_other_head_refs(struct ref_store *, each_ref_fn, cb_data);

in addition to other_head_refs() to handle it, which might require

    int get_submodule_worktrees(const char *submodule, int flags);

It could be a separate topic to reduce the scope of this one.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 reachable.c      |  1 +
 refs.c           | 22 ++++++++++++++++++++++
 refs.h           |  1 +
 revision.c       | 13 +++++++++++++
 submodule.c      |  2 ++
 t/t5304-prune.sh | 12 ++++++++++++
 6 files changed, 51 insertions(+)

reachable.c +1 -0

index d0199cace..61a6ec05c 100644
@@ -177,6 +177,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
 
 	/* detached HEAD is not included in the list above */
 	head_ref(add_one_ref, revs);
+	other_head_refs(add_one_ref, revs);
 
 	/* Add all reflog info */
 	if (mark_reflog)

refs.c +22 -0

index fa2df7a1d..ce165c0ea 100644
@@ -1676,3 +1676,25 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 
 	return refs->be->rename_ref(refs, oldref, newref, logmsg);
 }
+
+int other_head_refs(each_ref_fn fn, void *cb_data)
+{
+	struct worktree **worktrees, **p;
+	int ret = 0;
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+		struct ref_store *refs;
+
+		if (wt->is_current)
+			continue;
+
+		refs = get_worktree_ref_store(wt);
+		ret = refs_head_ref(refs, fn, cb_data);
+		if (ret)
+			break;
+	}
+	free_worktrees(worktrees);
+	return ret;
+}

refs.h +1 -0

index 986d408bd..6665e5c57 100644
@@ -190,6 +190,7 @@ typedef int each_ref_fn(const char *refname,
  * stop the iteration.
  */
 int head_ref(each_ref_fn fn, void *cb_data);
+int other_head_refs(each_ref_fn fn, void *cb_data);
 int for_each_ref(each_ref_fn fn, void *cb_data);
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,

revision.c +13 -0

index d82c37b44..8ee929cef 100644
@@ -2105,6 +2105,13 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	int argcount;
 
 	if (submodule) {
+		/*
+		 * We need some something like get_submodule_worktrees()
+		 * before we can go through all worktrees of a submodule,
+		 * .e.g with adding all HEADs from --all, which is not
+		 * supported right now, so stick to single worktree.
+		 */
+		assert(revs->single_worktree != 0);
 		refs = get_submodule_ref_store(submodule);
 	} else
 		refs = get_main_ref_store();

@@ -2122,6 +2129,12 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (!strcmp(arg, "--all")) {
 		handle_refs(refs, revs, *flags, refs_for_each_ref);
 		handle_refs(refs, revs, *flags, refs_head_ref);
+		if (!revs->single_worktree) {
+			struct all_refs_cb cb;
+
+			init_all_refs_cb(&cb, revs, *flags);
+			other_head_refs(handle_one_ref, &cb);
+		}
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
 		handle_refs(refs, revs, *flags, refs_for_each_branch_ref);

submodule.c +2 -0

index 3ce589d55..aed47baaf 100644
@@ -1155,6 +1155,8 @@ static int find_first_merges(struct object_array *result, const char *path,
 			oid_to_hex(&a->object.oid));
 	init_revisions(&revs, NULL);
 	rev_opts.submodule = path;
+	/* FIXME: can't handle linked worktrees in submodules yet */
+	revs.single_worktree = path != NULL;
 	setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);
 
 	/* save all revisions from the above list that contain b */

t/t5304-prune.sh +12 -0

index cba45c7be..683bdb031 100755
@@ -292,4 +292,16 @@ test_expect_success 'prune: handle index in multiple worktrees' '
 	test_cmp second-worktree/blob actual
 '
 
+test_expect_success 'prune: handle HEAD in multiple worktrees' '
+	git worktree add --detach third-worktree &&
+	echo "new blob for third-worktree" >third-worktree/blob &&
+	git -C third-worktree add blob &&
+	git -C third-worktree commit -m "third" &&
+	rm .git/worktrees/third-worktree/index &&
+	test_must_fail git -C third-worktree show :blob &&
+	git prune --expire=now &&
+	git -C third-worktree show HEAD:blob >actual &&
+	test_cmp third-worktree/blob actual
+'
+
 test_done

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:20 UTC #12 lore raw

PATCH 12/15 refs: add refs_for_each_reflog[_ent]()

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs.c | 19 ++++++++++++++-----
 refs.h |  3 +++
 2 files changed, 17 insertions(+), 5 deletions(-)

refs.c +14 -5

index ce165c0ea..622c6b669 100644
@@ -1592,9 +1592,8 @@ int verify_refname_available(const char *refname,
 	return refs->be->verify_refname_available(refs, refname, extra, skip, err);
 }
 
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
-	struct ref_store *refs = get_main_ref_store();
 	struct ref_iterator *iter;
 
 	iter = refs->be->reflog_iterator_begin(refs);

@@ -1602,6 +1601,11 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
 	return do_for_each_ref_iterator(iter, fn, cb_data);
 }
 
+int for_each_reflog(each_ref_fn fn, void *cb_data)
+{
+	return refs_for_each_reflog(get_main_ref_store(), fn, cb_data);
+}
+
 int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
 				void *cb_data)
 {

@@ -1611,12 +1615,17 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
 						     fn, cb_data);
 }
 
+int refs_for_each_reflog_ent(struct ref_store *refs, const char *refname,
+			     each_reflog_ent_fn fn, void *cb_data)
+{
+	return refs->be->for_each_reflog_ent(refs, refname, fn, cb_data);
+}
+
 int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
 			void *cb_data)
 {
-	struct ref_store *refs = get_main_ref_store();
-
-	return refs->be->for_each_reflog_ent(refs, refname, fn, cb_data);
+	return refs_for_each_reflog_ent(get_main_ref_store(),
+					refname, fn, cb_data);
 }
 
 int reflog_exists(const char *refname)

refs.h +3 -0

index 6665e5c57..5c1b99596 100644
@@ -572,5 +572,8 @@ int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
 int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
 int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
 int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_reflog_ent(struct ref_store *refs, const char *refname,
+			     each_reflog_ent_fn fn, void *cb_data);
 
 #endif /* REFS_H */

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:21 UTC #13 lore raw

PATCH 13/15 files-backend: make reflog iterator go through per-worktree reflog

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 refs/files-backend.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

refs/files-backend.c +23 -1

index 011a7e256..d429f8713 100644
@@ -3314,6 +3314,7 @@ struct files_reflog_iterator {
 	struct ref_iterator base;
 
 	struct dir_iterator *dir_iterator;
+	struct dir_iterator *dir_iterator2;
 	struct object_id oid;
 };

@@ -3334,6 +3335,10 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
 		if (ends_with(diter->basename, ".lock"))
 			continue;
 
+		if (iter->dir_iterator2 &&
+		    starts_with(diter->relative_path, "refs/bisect/"))
+			continue;
+
 		if (read_ref_full(diter->relative_path, 0,
 				  iter->oid.hash, &flags)) {
 			error("bad ref for %s", diter->path.buf);

@@ -3346,7 +3351,11 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
 		return ITER_OK;
 	}
 
-	iter->dir_iterator = NULL;
+	iter->dir_iterator = iter->dir_iterator2;
+	if (iter->dir_iterator2) {
+		iter->dir_iterator2 = NULL;
+		return files_reflog_iterator_advance(ref_iterator);
+	}
 	if (ref_iterator_abort(ref_iterator) == ITER_ERROR)
 		ok = ITER_ERROR;
 	return ok;

@@ -3367,6 +3376,12 @@ static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator)
 	if (iter->dir_iterator)
 		ok = dir_iterator_abort(iter->dir_iterator);
 
+	if (iter->dir_iterator2) {
+		int ok2 = dir_iterator_abort(iter->dir_iterator2);
+		if (ok2 == ITER_ERROR)
+			ok = ok2;
+	}
+
 	base_ref_iterator_free(ref_iterator);
 	return ok;
 }

@@ -3389,6 +3404,13 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
 	files_path(refs, &sb, "logs");
 	iter->dir_iterator = dir_iterator_begin(sb.buf);
 	strbuf_release(&sb);
+
+	if (strcmp(refs->gitdir.buf, refs->gitcommondir.buf)) {
+		strbuf_addf(&sb, "%s/logs", refs->gitdir.buf);
+		iter->dir_iterator2 = dir_iterator_begin(sb.buf);
+		strbuf_release(&sb);
+	}
+
 	return ref_iterator;
 }

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:21 UTC #14 lore raw

PATCH 14/15 revision.c: --reflog add HEAD reflog from all worktrees

In reply to Nguyễn Thái Ngọc Duy

Note that add_other_reflogs_to_pending() is a bit inefficient, since it scans reflog for all refs of each worktree, including shared refs, so the shared ref's reflog is scanned over and over again.

We could update refs API to pass "per-worktree only" flag to avoid that. But long term we should be able to obtain a "per-worktree only" ref store and would need to revert the changes in reflog iteration API. So let's just wait until then.

add_reflogs_to_pending() is called by reachable.c so by default "git prune" will examine reflog from all worktrees.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 revision.c       | 28 +++++++++++++++++++++++++++-
 t/t5304-prune.sh | 16 ++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

revision.c +27 -1

index 8ee929cef..ecfd6fea6 100644
@@ -1134,6 +1134,7 @@ struct all_refs_cb {
 	int warned_bad_reflog;
 	struct rev_info *all_revs;
 	const char *name_for_errormsg;
+	struct ref_store *refs;
 };
 
 int ref_excluded(struct string_list *ref_excludes, const char *path)

@@ -1169,6 +1170,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 {
 	cb->all_revs = revs;
 	cb->all_flags = flags;
+	cb->refs = NULL;
 }
 
 void clear_ref_exclusion(struct string_list **ref_excludes_p)

@@ -1237,17 +1239,41 @@ static int handle_one_reflog(const char *path, const struct object_id *oid,
 	struct all_refs_cb *cb = cb_data;
 	cb->warned_bad_reflog = 0;
 	cb->name_for_errormsg = path;
-	for_each_reflog_ent(path, handle_one_reflog_ent, cb_data);
+	refs_for_each_reflog_ent(cb->refs, path,
+				 handle_one_reflog_ent, cb_data);
 	return 0;
 }
 
+static void add_other_reflogs_to_pending(struct all_refs_cb *cb)
+{
+	struct worktree **worktrees, **p;
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+
+		if (wt->is_current)
+			continue;
+
+		cb->refs = get_worktree_ref_store(wt);
+		refs_for_each_reflog(cb->refs,
+				     handle_one_reflog,
+				     cb);
+	}
+	free_worktrees(worktrees);
+}
+
 void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 {
 	struct all_refs_cb cb;
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
+	cb.refs = get_main_ref_store();
 	for_each_reflog(handle_one_reflog, &cb);
+
+	if (!revs->single_worktree)
+		add_other_reflogs_to_pending(&cb);
 }
 
 static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,

t/t5304-prune.sh +16 -0

index 683bdb031..6694c19a1 100755
@@ -304,4 +304,20 @@ test_expect_success 'prune: handle HEAD in multiple worktrees' '
 	test_cmp third-worktree/blob actual
 '
 
+test_expect_success 'prune: handle HEAD reflog in multiple worktrees' '
+	git config core.logAllRefUpdates true &&
+	echo "lost blob for third-worktree" >expected &&
+	(
+		cd third-worktree &&
+		cat ../expected >blob &&
+		git add blob &&
+		git commit -m "second commit in third" &&
+		git reset --hard HEAD^
+	) &&
+	git prune --expire=now &&
+	SHA1=`git hash-object expected` &&
+	git -C third-worktree show "$SHA1" >actual &&
+	test_cmp expected actual
+'
+
 test_done

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-02-17 14:21 UTC #15 lore raw

PATCH 15/15 rev-list: expose and document --single-worktree

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 Documentation/rev-list-options.txt | 8 ++++++++
 revision.c                         | 2 ++
 2 files changed, 10 insertions(+)

Documentation/rev-list-options.txt +8 -0

index 5da7cf5a8..dd773f97c 100644
@@ -179,6 +179,14 @@ explicitly.
 	Pretend as if all objects mentioned by reflogs are listed on the
 	command line as `<commit>`.
 
+--single-worktree::
+	By default, all working trees will be examined by the
+	following options when there are more than one (see
+	linkgit:git-worktree[1]): `--all`, `--reflog` and
+	`--indexed-objects`.
+	This option forces them to examine the current working tree
+	only.
+
 --ignore-missing::
 	Upon seeing an invalid object name in the input, pretend as if
 	the bad input was not given.

revision.c +2 -0

index ecfd6fea6..2a27e55fe 100644
@@ -2222,6 +2222,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
 			return error("invalid argument to --no-walk");
 	} else if (!strcmp(arg, "--do-walk")) {
 		revs->no_walk = 0;
+	} else if (!strcmp(arg, "--single-worktree")) {
+		revs->single_worktree = 1;
 	} else {
 		return 0;
 	}

Signature · 1 line

2.11.0.157.gd943d85

Johannes Schindelin

2017-02-17 17:10 UTC #16 lore raw

In reply to Nguyễn Thái Ngọc Duy

Hi Duy,

Quoted 4 lines · Nguyễn Thái Ngọc Duy

So here is my latest attempt on fixing this issue. For people who are not aware of it, git-gc does not take per-worktree refs, reflogs and indexes into account. An odb prune may leave HEAD and references in other worktrees pointing to nowhere.

Thank you so much for working on this. The bug really affects my daily work very, very negatively.

Will try to review as soon as possible.

Ciao,
Dscho

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:12 UTC #17 lore raw

v2 cover Fix git-gc losing objects in multi worktree

In reply to Nguyễn Thái Ngọc Duy

This is basically a rebased version after a lot of changes in nd/files-backend-git-dir. Thanks to test-ref-store.c, I found and fixed a bug in for_each_reflog in v1 code.

This series needs latest nd/worktree-kill-parse-ref [1]. But you can get everything on my github fork, branch prune-in-worktrees-2.

[1] http://public-inbox.org/git/%[email protected]%3E/

Nguyễn Thái Ngọc Duy (12):
revision.h: new flag in struct rev_info wrt. worktree-related refs revision.c: refactor add_index_objects_to_pending() revision.c: --indexed-objects add objects from all worktrees refs.c: refactor get_submodule_ref_store(), share common free block
refs: move submodule slash stripping code to get_submodule_ref_store
refs: add refs_head_ref()
revision.c: use refs_for_each*() instead of for_each_*_submodule()
refs: remove dead for_each_*_submodule()
revision.c: --all adds HEAD from all worktrees
files-backend: make reflog iterator go through per-worktree reflog revision.c: --reflog add HEAD reflog from all worktrees
rev-list: expose and document --single-worktree

 Documentation/rev-list-options.txt |   8 +++
 reachable.c                        |   1 +
 refs.c                             | 110 +++++++++++++++----------------
 refs.h                             |  12 +---
 refs/files-backend.c               |  46 +++++++++----
 revision.c                         | 130 ++++++++++++++++++++++++++++++-------
 revision.h                         |   1 +
 submodule.c                        |   2 +
 t/t1407-worktree-ref-store.sh      |  30 +++++++++
 t/t5304-prune.sh                   |  37 +++++++++++
 10 files changed, 277 insertions(+), 100 deletions(-)

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:12 UTC #18 lore raw

PATCH v2 1/12 revision.h: new flag in struct rev_info wrt. worktree-related refs

When single_worktree is set, only current worktree is considered. When it is not set (which is the default), all worktrees are considered.

The flag will eventually be exposed as a rev-list argument with documents. For now it stays internal until the new behavior is fully implemented.

---
 revision.h | 1 +
 1 file changed, 1 insertion(+)

revision.h +1 -0

index 9fac1a607d..c851b94ad8 100644
@@ -88,6 +88,7 @@ struct rev_info {
 			topo_order:1,
 			simplify_merges:1,
 			simplify_by_decoration:1,
+			single_worktree:1,
 			tag_objects:1,
 			tree_objects:1,
 			blob_objects:1,

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:12 UTC #19 lore raw

PATCH v2 2/12 revision.c: refactor add_index_objects_to_pending()

In reply to Nguyễn Thái Ngọc Duy

The core code is factored out and take 'struct index_state *' instead so that we can reuse it to add objects from index files other than .git/index in the next patch.

---
 revision.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

revision.c +12 -6

index 7ff61ff5f7..98146f179f 100644
@@ -1263,13 +1263,13 @@ static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,
 
 }
 
-void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)
+static void do_add_index_objects_to_pending(struct rev_info *revs,
+					    struct index_state *istate)
 {
 	int i;
 
-	read_cache();
-	for (i = 0; i < active_nr; i++) {
-		struct cache_entry *ce = active_cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 		struct blob *blob;
 
 		if (S_ISGITLINK(ce->ce_mode))

@@ -1282,13 +1282,19 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)
 					     ce->ce_mode, ce->name);
 	}
 
-	if (active_cache_tree) {
+	if (istate->cache_tree) {
 		struct strbuf path = STRBUF_INIT;
-		add_cache_tree(active_cache_tree, revs, &path);
+		add_cache_tree(istate->cache_tree, revs, &path);
 		strbuf_release(&path);
 	}
 }
 
+void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
+{
+	read_cache();
+	do_add_index_objects_to_pending(revs, &the_index);
+}
+
 static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
 			    int exclude_parent)
 {

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:12 UTC #20 lore raw

PATCH v2 3/12 revision.c: --indexed-objects add objects from all worktrees

In reply to Nguyễn Thái Ngọc Duy

This is the result of single_worktree flag never being set (no way to up until now). To get objects from current index only, set single_worktree.

The other add_index_objects_to_pending's caller is mark_reachable_objects() (e.g. "git prune") which also mark objects from all indexes.

---
 revision.c       | 21 +++++++++++++++++++++
 t/t5304-prune.sh |  9 +++++++++
 2 files changed, 30 insertions(+)

revision.c +21 -0

index 98146f179f..db8021ed26 100644
@@ -19,6 +19,7 @@
 #include "dir.h"
 #include "cache-tree.h"
 #include "bisect.h"
+#include "worktree.h"
 
 volatile show_early_output_fn_t show_early_output;

@@ -1291,8 +1292,28 @@ static void do_add_index_objects_to_pending(struct rev_info *revs,
 
 void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
 {
+	struct worktree **worktrees, **p;
+
 	read_cache();
 	do_add_index_objects_to_pending(revs, &the_index);
+
+	if (revs->single_worktree)
+		return;
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+		struct index_state istate = {0};
+
+		if (wt->is_current)
+			continue; /* current index already taken care of */
+
+		if (read_index_from(&istate,
+				    worktree_git_path(wt, "index")) > 0)
+			do_add_index_objects_to_pending(revs, &istate);
+		discard_index(&istate);
+	}
+	free_worktrees(worktrees);
 }
 
 static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,

t/t5304-prune.sh +9 -0

index 133b5842b1..cba45c7be9 100755
@@ -283,4 +283,13 @@ test_expect_success 'prune: handle alternate object database' '
 	git -C B prune
 '
 
+test_expect_success 'prune: handle index in multiple worktrees' '
+	git worktree add second-worktree &&
+	echo "new blob for second-worktree" >second-worktree/blob &&
+	git -C second-worktree add blob &&
+	git prune --expire=now &&
+	git -C second-worktree show :blob >actual &&
+	test_cmp second-worktree/blob actual
+'
+
 test_done

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:12 UTC #21 lore raw

PATCH v2 4/12 refs.c: refactor get_submodule_ref_store(), share common free block

In reply to Nguyễn Thái Ngọc Duy

---
 refs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

refs.c +5 -7

index 6695140cfe..8912798c1d 100644
@@ -1571,25 +1571,23 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 
 	refs = lookup_submodule_ref_store(submodule);
 	if (refs)
-		return refs;
+		goto done;
 
 	strbuf_addstr(&submodule_sb, submodule);
 	ret = is_nonbare_repository_dir(&submodule_sb);
-	strbuf_release(&submodule_sb);
 	if (!ret)
-		return NULL;
+		goto done;
 
 	ret = submodule_to_gitdir(&submodule_sb, submodule);
-	if (ret) {
-		strbuf_release(&submodule_sb);
-		return NULL;
-	}
+	if (ret)
+		goto done;
 
 	/* pretend that add_submodule_odb() has been called */
 	refs = ref_store_init(submodule_sb.buf,
 			      REF_STORE_READ | REF_STORE_ODB);
 	register_submodule_ref_store(refs, submodule);
 
+done:
 	strbuf_release(&submodule_sb);
 	return refs;
 }

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:13 UTC #22 lore raw

PATCH v2 5/12 refs: move submodule slash stripping code to get_submodule_ref_store

In reply to Nguyễn Thái Ngọc Duy

This is a better place that will benefit all submodule callers instead of just resolve_gitlink_ref()

---
 refs.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

refs.c +17 ~~-16~~

index 8912798c1d..a38149d84a 100644
@@ -1422,25 +1422,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 int resolve_gitlink_ref(const char *submodule, const char *refname,
 			unsigned char *sha1)
 {
-	size_t len = strlen(submodule);
 	struct ref_store *refs;
 	int flags;
 
-	while (len && submodule[len - 1] == '/')
-		len--;
-
-	if (!len)
-		return -1;
-
-	if (submodule[len]) {
-		/* We need to strip off one or more trailing slashes */
-		char *stripped = xmemdupz(submodule, len);
-
-		refs = get_submodule_ref_store(stripped);
-		free(stripped);
-	} else {
-		refs = get_submodule_ref_store(submodule);
-	}
+	refs = get_submodule_ref_store(submodule);
 
 	if (!refs)
 		return -1;

@@ -1559,7 +1544,17 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 {
 	struct strbuf submodule_sb = STRBUF_INIT;
 	struct ref_store *refs;
+	char *to_free = NULL;
 	int ret;
+	size_t len;
+
+	if (submodule) {
+		len = strlen(submodule);
+		while (len && submodule[len - 1] == '/')
+			len--;
+		if (!len)
+			submodule = NULL;
+	}
 
 	if (!submodule || !*submodule) {
 		/*

@@ -1569,6 +1564,10 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 		return get_main_ref_store();
 	}
 
+	if (submodule[len])
+		/* We need to strip off one or more trailing slashes */
+		submodule = to_free = xmemdupz(submodule, len);
+
 	refs = lookup_submodule_ref_store(submodule);
 	if (refs)
 		goto done;

@@ -1589,6 +1588,8 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 
 done:
 	strbuf_release(&submodule_sb);
+	free(to_free);
+
 	return refs;
 }

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:12 UTC #23 lore raw

PATCH v2 6/12 refs: add refs_head_ref()

In reply to Nguyễn Thái Ngọc Duy

---
 refs.c | 19 +++++++++----------
 refs.h |  2 ++
 2 files changed, 11 insertions(+), 10 deletions(-)

refs.c +9 ~~-10~~

index a38149d84a..5fc47ff5f0 100644
@@ -1208,27 +1208,26 @@ int refs_rename_ref_available(struct ref_store *refs,
 	return ok;
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
 	struct object_id oid;
 	int flag;
 
-	if (submodule) {
-		if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
-			return fn("HEAD", &oid, 0, cb_data);
-
-		return 0;
-	}
-
-	if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
+	if (!refs_read_ref_full(refs, "HEAD", RESOLVE_REF_READING,
+				oid.hash, &flag))
 		return fn("HEAD", &oid, flag, cb_data);
 
 	return 0;
 }
 
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+{
+	return refs_head_ref(get_submodule_ref_store(submodule), fn, cb_data);
+}
+
 int head_ref(each_ref_fn fn, void *cb_data)
 {
-	return head_ref_submodule(NULL, fn, cb_data);
+	return refs_head_ref(get_main_ref_store(), fn, cb_data);
 }
 
 /*

refs.h +2 -0

index 447381d378..0572473ef7 100644
@@ -233,6 +233,8 @@ typedef int each_ref_fn(const char *refname,
  * modifies the reference also returns a nonzero value to immediately
  * stop the iteration. Returned references are sorted.
  */
+int refs_head_ref(struct ref_store *refs,
+		  each_ref_fn fn, void *cb_data);
 int refs_for_each_ref(struct ref_store *refs,
 		      each_ref_fn fn, void *cb_data);
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:43 UTC #24 lore raw

PATCH v2 7/12 revision.c: use refs_for_each*() instead of for_each_*_submodule()

In reply to Nguyễn Thái Ngọc Duy

---
 revision.c | 48 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 16 deletions(-)

revision.c +32 ~~-16~~

index db8021ed26..da19071960 100644
@@ -1189,12 +1189,19 @@ void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)
 	string_list_append(*ref_excludes_p, exclude);
 }
 
-static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags,
-		int (*for_each)(const char *, each_ref_fn, void *))
+static void handle_refs(struct ref_store *refs,
+			struct rev_info *revs, unsigned flags,
+			int (*for_each)(struct ref_store *, each_ref_fn, void *))
 {
 	struct all_refs_cb cb;
+
+	if (!refs) {
+		/* this could happen with uninitialized submodules */
+		return;
+	}
+
 	init_all_refs_cb(&cb, revs, flags);
-	for_each(submodule, handle_one_ref, &cb);
+	for_each(refs, handle_one_ref, &cb);
 }
 
 static void handle_one_reflog_commit(struct object_id *oid, void *cb_data)

@@ -2067,23 +2074,25 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 	ctx->argc -= n;
 }
 
-static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data, const char *term) {
+static int for_each_bisect_ref(struct ref_store *refs, each_ref_fn fn,
+			       void *cb_data, const char *term)
+{
 	struct strbuf bisect_refs = STRBUF_INIT;
 	int status;
 	strbuf_addf(&bisect_refs, "refs/bisect/%s", term);
-	status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+	status = refs_for_each_ref_in(refs, bisect_refs.buf, fn, cb_data);
 	strbuf_release(&bisect_refs);
 	return status;
 }
 
-static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int for_each_bad_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
-	return for_each_bisect_ref(submodule, fn, cb_data, term_bad);
+	return for_each_bisect_ref(refs, fn, cb_data, term_bad);
 }
 
-static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
-	return for_each_bisect_ref(submodule, fn, cb_data, term_good);
+	return for_each_bisect_ref(refs, fn, cb_data, term_good);
 }
 
 static int handle_revision_pseudo_opt(const char *submodule,

@@ -2092,8 +2101,14 @@ static int handle_revision_pseudo_opt(const char *submodule,
 {
 	const char *arg = argv[0];
 	const char *optarg;
+	struct ref_store *refs;
 	int argcount;
 
+	if (submodule) {
+		refs = get_submodule_ref_store(submodule);
+	} else
+		refs = get_main_ref_store();
+
 	/*
 	 * NOTE!
 	 *

@@ -2105,22 +2120,23 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	 * register it in the list at the top of handle_revision_opt.
 	 */
 	if (!strcmp(arg, "--all")) {
-		handle_refs(submodule, revs, *flags, for_each_ref_submodule);
-		handle_refs(submodule, revs, *flags, head_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_ref);
+		handle_refs(refs, revs, *flags, refs_head_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
-		handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_branch_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--bisect")) {
 		read_bisect_terms(&term_bad, &term_good);
-		handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
-		handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
+		handle_refs(refs, revs, *flags, for_each_bad_bisect_ref);
+		handle_refs(refs, revs, *flags ^ (UNINTERESTING | BOTTOM),
+			    for_each_good_bisect_ref);
 		revs->bisect = 1;
 	} else if (!strcmp(arg, "--tags")) {
-		handle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_tag_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--remotes")) {
-		handle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);
+		handle_refs(refs, revs, *flags, refs_for_each_remote_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:22 UTC #25 lore raw

PATCH v2 8/12 refs: remove dead for_each_*_submodule()

In reply to Nguyễn Thái Ngọc Duy

These are used in revision.c. After the last patch they are replaced with the refs_ version. Delete them (except for_each_remote_ref_submodule which is still used by submodule.c)

---
 refs.c | 24 ------------------------
 refs.h |  9 ---------
 2 files changed, 33 deletions(-)

refs.c +0 ~~-24~~

index 5fc47ff5f0..1f2f870084 100644
@@ -316,12 +316,6 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 	return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
 }
 
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
-	return refs_for_each_tag_ref(get_submodule_ref_store(submodule),
-				     fn, cb_data);
-}
-
 int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
 	return refs_for_each_ref_in(refs, "refs/heads/", fn, cb_data);

@@ -332,12 +326,6 @@ int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 	return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
 }
 
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
-	return refs_for_each_branch_ref(get_submodule_ref_store(submodule),
-					fn, cb_data);
-}
-
 int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
 	return refs_for_each_ref_in(refs, "refs/remotes/", fn, cb_data);

@@ -1263,11 +1251,6 @@ int for_each_ref(each_ref_fn fn, void *cb_data)
 	return refs_for_each_ref(get_main_ref_store(), fn, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
-	return refs_for_each_ref(get_submodule_ref_store(submodule), fn, cb_data);
-}
-
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
 			 each_ref_fn fn, void *cb_data)
 {

@@ -1289,13 +1272,6 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig
 			       prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-			      each_ref_fn fn, void *cb_data)
-{
-	return refs_for_each_ref_in(get_submodule_ref_store(submodule),
-				    prefix, fn, cb_data);
-}
-
 int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(get_main_ref_store(),

refs.h +0 -9

index 0572473ef7..e06db37118 100644
@@ -259,15 +259,6 @@ int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
-int for_each_ref_submodule(const char *submodule,
-			   each_ref_fn fn, void *cb_data);
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-		each_ref_fn fn, void *cb_data);
-int for_each_tag_ref_submodule(const char *submodule,
-			       each_ref_fn fn, void *cb_data);
-int for_each_branch_ref_submodule(const char *submodule,
-				  each_ref_fn fn, void *cb_data);
 int for_each_remote_ref_submodule(const char *submodule,
 				  each_ref_fn fn, void *cb_data);

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:14 UTC #26 lore raw

PATCH v2 9/12 revision.c: --all adds HEAD from all worktrees

In reply to Nguyễn Thái Ngọc Duy

Unless single_worktree is set, --all now adds HEAD from all worktrees.

A new FIXME is added because we would need something like

    int refs_other_head_refs(struct ref_store *, each_ref_fn, cb_data);

in addition to other_head_refs() to handle it, which might require

    int get_submodule_worktrees(const char *submodule, int flags);

It could be a separate topic to reduce the scope of this one.

---
 reachable.c      |  1 +
 refs.c           | 22 ++++++++++++++++++++++
 refs.h           |  1 +
 revision.c       | 13 +++++++++++++
 submodule.c      |  2 ++
 t/t5304-prune.sh | 12 ++++++++++++
 6 files changed, 51 insertions(+)

reachable.c +1 -0

index a8a979bd4f..a3b938b46c 100644
@@ -177,6 +177,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
 
 	/* detached HEAD is not included in the list above */
 	head_ref(add_one_ref, revs);
+	other_head_refs(add_one_ref, revs);
 
 	/* Add all reflog info */
 	if (mark_reflog)

refs.c +22 -0

index 1f2f870084..6006d4fdbc 100644
@@ -1787,3 +1787,25 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
 	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
 }
+
+int other_head_refs(each_ref_fn fn, void *cb_data)
+{
+	struct worktree **worktrees, **p;
+	int ret = 0;
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+		struct ref_store *refs;
+
+		if (wt->is_current)
+			continue;
+
+		refs = get_worktree_ref_store(wt);
+		ret = refs_head_ref(refs, fn, cb_data);
+		if (ret)
+			break;
+	}
+	free_worktrees(worktrees);
+	return ret;
+}

refs.h +1 -0

index e06db37118..cc71b6c7a0 100644
@@ -247,6 +247,7 @@ int refs_for_each_remote_ref(struct ref_store *refs,
 			     each_ref_fn fn, void *cb_data);
 
 int head_ref(each_ref_fn fn, void *cb_data);
+int other_head_refs(each_ref_fn fn, void *cb_data);
 int for_each_ref(each_ref_fn fn, void *cb_data);
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,

revision.c +13 -0

index da19071960..e5d2913b5c 100644
@@ -2105,6 +2105,13 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	int argcount;
 
 	if (submodule) {
+		/*
+		 * We need some something like get_submodule_worktrees()
+		 * before we can go through all worktrees of a submodule,
+		 * .e.g with adding all HEADs from --all, which is not
+		 * supported right now, so stick to single worktree.
+		 */
+		assert(revs->single_worktree != 0);
 		refs = get_submodule_ref_store(submodule);
 	} else
 		refs = get_main_ref_store();

@@ -2122,6 +2129,12 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (!strcmp(arg, "--all")) {
 		handle_refs(refs, revs, *flags, refs_for_each_ref);
 		handle_refs(refs, revs, *flags, refs_head_ref);
+		if (!revs->single_worktree) {
+			struct all_refs_cb cb;
+
+			init_all_refs_cb(&cb, revs, *flags);
+			other_head_refs(handle_one_ref, &cb);
+		}
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
 		handle_refs(refs, revs, *flags, refs_for_each_branch_ref);

submodule.c +2 -0

index 3c445f274e..63a190f0f6 100644
@@ -1225,6 +1225,8 @@ static int find_first_merges(struct object_array *result, const char *path,
 			oid_to_hex(&a->object.oid));
 	init_revisions(&revs, NULL);
 	rev_opts.submodule = path;
+	/* FIXME: can't handle linked worktrees in submodules yet */
+	revs.single_worktree = path != NULL;
 	setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);
 
 	/* save all revisions from the above list that contain b */

t/t5304-prune.sh +12 -0

index cba45c7be9..683bdb031c 100755
@@ -292,4 +292,16 @@ test_expect_success 'prune: handle index in multiple worktrees' '
 	test_cmp second-worktree/blob actual
 '
 
+test_expect_success 'prune: handle HEAD in multiple worktrees' '
+	git worktree add --detach third-worktree &&
+	echo "new blob for third-worktree" >third-worktree/blob &&
+	git -C third-worktree add blob &&
+	git -C third-worktree commit -m "third" &&
+	rm .git/worktrees/third-worktree/index &&
+	test_must_fail git -C third-worktree show :blob &&
+	git prune --expire=now &&
+	git -C third-worktree show HEAD:blob >actual &&
+	test_cmp third-worktree/blob actual
+'
+
 test_done

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:14 UTC #27 lore raw

PATCH v2 10/12 files-backend: make reflog iterator go through per-worktree reflog

In reply to Nguyễn Thái Ngọc Duy

refs/bisect is unfortunately per-worktree, so we need to look in per-worktree logs/refs/bisect in addition to per-repo logs/refs. The current iterator only goes through per-repo logs/refs.

Ideally we should have something like merge_ref_iterator_begin (and maybe with a predicate), but for dir_iterator. Since there's only one use case for this pattern, let's not add a bunch more code for merge_dir_iterator_begin just yet.

PS. Note the unsorted order of for_each_reflog in the test. This is supposed to be OK, for now. If we enforce order on for_each_reflog() then some more work will be required.

---
 refs/files-backend.c          | 46 ++++++++++++++++++++++++++++++++-----------
 t/t1407-worktree-ref-store.sh | 30 ++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 11 deletions(-)

refs/files-backend.c +35 ~~-11~~

index 62d8e0713a..0f423be185 100644
@@ -1172,15 +1172,6 @@ static void files_reflog_path(struct files_ref_store *refs,
 			      struct strbuf *sb,
 			      const char *refname)
 {
-	if (!refname) {
-		/*
-		 * FIXME: of course this is wrong in multi worktree
-		 * setting. To be fixed real soon.
-		 */
-		strbuf_addf(sb, "%s/logs", refs->gitcommondir);
-		return;
-	}
-
 	switch (ref_type(refname)) {
 	case REF_TYPE_PER_WORKTREE:
 	case REF_TYPE_PSEUDOREF:

@@ -3371,6 +3362,7 @@ struct files_reflog_iterator {
 
 	struct ref_store *ref_store;
 	struct dir_iterator *dir_iterator;
+	struct dir_iterator *worktree_dir_iterator;
 	struct object_id oid;
 };

@@ -3391,6 +3383,21 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
 		if (ends_with(diter->basename, ".lock"))
 			continue;
 
+		if (iter->worktree_dir_iterator) {
+			const char *refname = diter->relative_path;
+
+			switch (ref_type(refname)) {
+			case REF_TYPE_PER_WORKTREE:
+			case REF_TYPE_PSEUDOREF:
+				continue;
+			case REF_TYPE_NORMAL:
+				break;
+			default:
+				die("BUG: unknown ref type %d of ref %s",
+				    ref_type(refname), refname);
+			}
+		}
+
 		if (refs_read_ref_full(iter->ref_store,
 				       diter->relative_path, 0,
 				       iter->oid.hash, &flags)) {

@@ -3404,7 +3411,11 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
 		return ITER_OK;
 	}
 
-	iter->dir_iterator = NULL;
+	iter->dir_iterator = iter->worktree_dir_iterator;
+	if (iter->worktree_dir_iterator) {
+		iter->worktree_dir_iterator = NULL;
+		return files_reflog_iterator_advance(ref_iterator);
+	}
 	if (ref_iterator_abort(ref_iterator) == ITER_ERROR)
 		ok = ITER_ERROR;
 	return ok;

@@ -3425,6 +3436,12 @@ static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator)
 	if (iter->dir_iterator)
 		ok = dir_iterator_abort(iter->dir_iterator);
 
+	if (iter->worktree_dir_iterator) {
+		int ok2 = dir_iterator_abort(iter->worktree_dir_iterator);
+		if (ok2 == ITER_ERROR)
+			ok = ok2;
+	}
+
 	base_ref_iterator_free(ref_iterator);
 	return ok;
 }

@@ -3445,10 +3462,17 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
 	struct strbuf sb = STRBUF_INIT;
 
 	base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);
-	files_reflog_path(refs, &sb, NULL);
+	strbuf_addf(&sb, "%s/logs", refs->gitcommondir);
 	iter->dir_iterator = dir_iterator_begin(sb.buf);
 	iter->ref_store = ref_store;
 	strbuf_release(&sb);
+
+	if (strcmp(refs->gitdir, refs->gitcommondir)) {
+		strbuf_addf(&sb, "%s/logs", refs->gitdir);
+		iter->worktree_dir_iterator = dir_iterator_begin(sb.buf);
+		strbuf_release(&sb);
+	}
+
 	return ref_iterator;
 }

t/t1407-worktree-ref-store.sh +30 -0

index 5df06f3556..8842d0329f 100755
@@ -49,4 +49,34 @@ test_expect_success 'create_symref(FOO, refs/heads/master)' '
 	test_cmp expected actual
 '
 
+test_expect_success 'for_each_reflog()' '
+	echo $_z40 > .git/logs/PSEUDO-MAIN &&
+	mkdir -p     .git/logs/refs/bisect &&
+	echo $_z40 > .git/logs/refs/bisect/random &&
+
+	echo $_z40 > .git/worktrees/wt/logs/PSEUDO-WT &&
+	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
+	echo $_z40 > .git/worktrees/wt/logs/refs/bisect/wt-random &&
+
+	$RWT for-each-reflog | cut -c 42- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-WT 0x0
+	refs/bisect/wt-random 0x0
+	refs/heads/master 0x0
+	refs/heads/wt-master 0x0
+	EOF
+	test_cmp expected actual &&
+
+	$RMAIN for-each-reflog | cut -c 42- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-MAIN 0x0
+	refs/bisect/random 0x0
+	refs/heads/master 0x0
+	refs/heads/wt-master 0x0
+	EOF
+	test_cmp expected actual
+'
+
 test_done

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:14 UTC #28 lore raw

PATCH v2 11/12 revision.c: --reflog add HEAD reflog from all worktrees

In reply to Nguyễn Thái Ngọc Duy

Note that add_other_reflogs_to_pending() is a bit inefficient, since it scans reflog for all refs of each worktree, including shared refs, so the shared ref's reflog is scanned over and over again.

add_reflogs_to_pending() is called by reachable.c so by default "git prune" will examine reflog from all worktrees.

---
 revision.c       | 28 +++++++++++++++++++++++++++-
 t/t5304-prune.sh | 16 ++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

revision.c +27 -1

index e5d2913b5c..fcf165bd76 100644
@@ -1134,6 +1134,7 @@ struct all_refs_cb {
 	int warned_bad_reflog;
 	struct rev_info *all_revs;
 	const char *name_for_errormsg;
+	struct ref_store *refs;
 };
 
 int ref_excluded(struct string_list *ref_excludes, const char *path)

@@ -1169,6 +1170,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 {
 	cb->all_revs = revs;
 	cb->all_flags = flags;
+	cb->refs = NULL;
 }
 
 void clear_ref_exclusion(struct string_list **ref_excludes_p)

@@ -1237,17 +1239,41 @@ static int handle_one_reflog(const char *path, const struct object_id *oid,
 	struct all_refs_cb *cb = cb_data;
 	cb->warned_bad_reflog = 0;
 	cb->name_for_errormsg = path;
-	for_each_reflog_ent(path, handle_one_reflog_ent, cb_data);
+	refs_for_each_reflog_ent(cb->refs, path,
+				 handle_one_reflog_ent, cb_data);
 	return 0;
 }
 
+static void add_other_reflogs_to_pending(struct all_refs_cb *cb)
+{
+	struct worktree **worktrees, **p;
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+
+		if (wt->is_current)
+			continue;
+
+		cb->refs = get_worktree_ref_store(wt);
+		refs_for_each_reflog(cb->refs,
+				     handle_one_reflog,
+				     cb);
+	}
+	free_worktrees(worktrees);
+}
+
 void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 {
 	struct all_refs_cb cb;
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
+	cb.refs = get_main_ref_store();
 	for_each_reflog(handle_one_reflog, &cb);
+
+	if (!revs->single_worktree)
+		add_other_reflogs_to_pending(&cb);
 }
 
 static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,

t/t5304-prune.sh +16 -0

index 683bdb031c..6694c19a1e 100755
@@ -304,4 +304,20 @@ test_expect_success 'prune: handle HEAD in multiple worktrees' '
 	test_cmp third-worktree/blob actual
 '
 
+test_expect_success 'prune: handle HEAD reflog in multiple worktrees' '
+	git config core.logAllRefUpdates true &&
+	echo "lost blob for third-worktree" >expected &&
+	(
+		cd third-worktree &&
+		cat ../expected >blob &&
+		git add blob &&
+		git commit -m "second commit in third" &&
+		git reset --hard HEAD^
+	) &&
+	git prune --expire=now &&
+	SHA1=`git hash-object expected` &&
+	git -C third-worktree show "$SHA1" >actual &&
+	test_cmp expected actual
+'
+
 test_done

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

author

2017-03-18 10:14 UTC #29 lore raw

PATCH v2 12/12 rev-list: expose and document --single-worktree

In reply to Nguyễn Thái Ngọc Duy

---
 Documentation/rev-list-options.txt | 8 ++++++++
 revision.c                         | 2 ++
 2 files changed, 10 insertions(+)

Documentation/rev-list-options.txt +8 -0

index a02f7324c0..c71e94b2d0 100644
@@ -179,6 +179,14 @@ explicitly.
 	Pretend as if all objects mentioned by reflogs are listed on the
 	command line as `<commit>`.
 
+--single-worktree::
+	By default, all working trees will be examined by the
+	following options when there are more than one (see
+	linkgit:git-worktree[1]): `--all`, `--reflog` and
+	`--indexed-objects`.
+	This option forces them to examine the current working tree
+	only.
+
 --ignore-missing::
 	Upon seeing an invalid object name in the input, pretend as if
 	the bad input was not given.

revision.c +2 -0

index fcf165bd76..dc32e99c54 100644
@@ -2222,6 +2222,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
 			return error("invalid argument to --no-walk");
 	} else if (!strcmp(arg, "--do-walk")) {
 		revs->no_walk = 0;
+	} else if (!strcmp(arg, "--single-worktree")) {
+		revs->single_worktree = 1;
 	} else {
 		return 0;
 	}

Signature · 1 line

2.11.0.157.gd943d85

Junio C Hamano

2017-03-18 18:00 UTC #30 lore raw

Re: [PATCH v2 12/12] rev-list: expose and document --single-worktree

Quoted 1 line · Nguyễn Thái Ngọc Duy

---

Missing sign-off.

Quoted 9 lines

 Documentation/rev-list-options.txt | 8 ++++++++
 revision.c                         | 2 ++
 2 files changed, 10 insertions(+)

Documentation/rev-list-options.txt +0 -0

index a02f7324c0..c71e94b2d0 100644

Documentation/rev-list-options.txt explicitly. · line 183

Quoted 5 lines

Documentation/rev-list-options.txt +2 -0

@@ -179,6 +179,14 @@ explicitly.
 	Pretend as if all objects mentioned by reflogs are listed on the
 	command line as `<commit>`.
 
+--single-worktree::
+	By default, all working trees will be examined by the

s/working tree/worktree/?

Quoted 22 lines

Documentation/rev-list-options.txt +6 -0

+	following options when there are more than one (see
+	linkgit:git-worktree[1]): `--all`, `--reflog` and
+	`--indexed-objects`.
+	This option forces them to examine the current working tree
+	only.
+
 --ignore-missing::
 	Upon seeing an invalid object name in the input, pretend as if
 	the bad input was not given.

revision.c +2 -0

index fcf165bd76..dc32e99c54 100644
@@ -2222,6 +2222,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
 			return error("invalid argument to --no-walk");
 	} else if (!strcmp(arg, "--do-walk")) {
 		revs->no_walk = 0;
+	} else if (!strcmp(arg, "--single-worktree")) {
+		revs->single_worktree = 1;
 	} else {
 		return 0;
 	}

Duy Nguyen

author

2017-04-19 10:53 UTC #31 lore raw

Re: [PATCH v2 12/12] rev-list: expose and document --single-worktree

Quoted 8 lines · Junio C Hamano

Quoted 8 lines

Documentation/rev-list-options.txt +0 -0

index a02f7324c0..c71e94b2d0 100644
@@ -179,6 +179,14 @@ explicitly.
      Pretend as if all objects mentioned by reflogs are listed on the
      command line as `<commit>`.

Quoted 4 lines

Documentation/rev-list-options.txt explicitly. · line 183
Quoted 2 lines
Documentation/rev-list-options.txt +2 -0
+--single-worktree::
+     By default, all working trees will be examined by the
s/working tree/worktree/?

Nope. It's the "working tree" that we consistently use throughout git-worktree.txt

Signature · 1 line

Duy

Junio C Hamano

2017-04-20 02:21 UTC #32 lore raw

Re: [PATCH v2 12/12] rev-list: expose and document --single-worktree

Quoted 9 lines · Duy Nguyen

Quoted 8 lines · Junio C Hamano

Quoted 8 lines

Documentation/rev-list-options.txt +0 -0

index a02f7324c0..c71e94b2d0 100644
@@ -179,6 +179,14 @@ explicitly.
      Pretend as if all objects mentioned by reflogs are listed on the
      command line as `<commit>`.

Quoted 7 lines

Quoted 4 lines
Documentation/rev-list-options.txt explicitly. · line 183
Quoted 2 lines
Documentation/rev-list-options.txt +2 -0
+--single-worktree::
+     By default, all working trees will be examined by the
s/working tree/worktree/?
Nope. It's the "working tree" that we consistently use throughout git-worktree.txt

OK.