{"thread":{"id":"40076","subject":"[PATCH v3 1/4] refs: clean up common_list","startedAt":"2015-08-12T21:57:22Z","lastAt":"2015-08-17T15:55:38Z","messageCount":24,"participants":["David Turner","Junio C Hamano","Michael Haggerty","Eric Sunshine","Duy Nguyen"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"267952","messageId":"1439416645-19173-1-git-send-email-dturner@twopensource.com","threadId":"40076","inReplyTo":null,"subject":"[PATCH v3 1/4] refs: clean up common_list","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-12T21:57:22Z","receivedAt":"2015-08-12T21:57:22Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Instead of common_list having formatting like ! and /, use a struct to\nhold common_list data in a structured form.\n\nWe don't use 'exclude' yet; instead, we keep the old codepath that\nhandles info/sparse-checkout and logs/HEAD.  Later, we will use exclude.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n\nJunio was worried about the performance of common_list and the weird\nstring parsing bits of update_common_dir, so this version of the patch\nseries begins by cleaning and optimizing those bits.\n\nAdditionally, I incorporated Junio's suggestion to use\nis_per_worktree_ref, and his formatting suggestions.\n\nThere is now a hack so that git for-each-ref works on per-worktree\nrefs.\n\nI also added git-bisect.sh, which I had overzealously reverted during\nmy proofreading step last time.\n\n---\n path.c | 58 +++++++++++++++++++++++++++++++++++++---------------------\n 1 file changed, 37 insertions(+), 21 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex 10f4cbf..236f797 100644\n--- a/path.c\n+++ b/path.c\n@@ -91,35 +91,51 @@ static void replace_dir(struct strbuf *buf, int len, const char *newdir)\n \t\tbuf->buf[newlen] = '/';\n }\n \n-static const char *common_list[] = {\n-\t\"/branches\", \"/hooks\", \"/info\", \"!/logs\", \"/lost-found\",\n-\t\"/objects\", \"/refs\", \"/remotes\", \"/worktrees\", \"/rr-cache\", \"/svn\",\n-\t\"config\", \"!gc.pid\", \"packed-refs\", \"shallow\",\n-\tNULL\n+struct common_dir {\n+\tconst char *dirname;\n+\t/* Not considered garbage for report_linked_checkout_garbage */\n+\tunsigned ignore_garbage:1;\n+\tunsigned is_dir:1;\n+\t/* Not common even though its parent is */\n+\tunsigned exclude:1;\n+};\n+\n+struct common_dir common_list[] = {\n+\t{ \"branches\", 0, 1, 0 },\n+\t{ \"hooks\", 0, 1, 0 },\n+\t{ \"info\", 0, 1, 0 },\n+\t{ \"info/sparse-checkout\", 0, 0, 1 },\n+\t{ \"logs\", 1, 1, 0 },\n+\t{ \"logs/HEAD\", 1, 1, 1 },\n+\t{ \"lost-found\", 0, 1, 0 },\n+\t{ \"objects\", 0, 1, 0 },\n+\t{ \"refs\", 0, 1, 0 },\n+\t{ \"remotes\", 0, 1, 0 },\n+\t{ \"worktrees\", 0, 1, 0 },\n+\t{ \"rr-cache\", 0, 1, 0 },\n+\t{ \"svn\", 0, 1, 0 },\n+\t{ \"config\", 0, 0, 0 },\n+\t{ \"gc.pid\", 1, 0, 0 },\n+\t{ \"packed-refs\", 0, 0, 0 },\n+\t{ \"shallow\", 0, 0, 0 },\n+\t{ NULL, 0, 0, 0 }\n };\n \n static void update_common_dir(struct strbuf *buf, int git_dir_len)\n {\n \tchar *base = buf->buf + git_dir_len;\n-\tconst char **p;\n+\tconst struct common_dir *p;\n \n \tif (is_dir_file(base, \"logs\", \"HEAD\") ||\n \t    is_dir_file(base, \"info\", \"sparse-checkout\"))\n \t\treturn;\t/* keep this in $GIT_DIR */\n-\tfor (p = common_list; *p; p++) {\n-\t\tconst char *path = *p;\n-\t\tint is_dir = 0;\n-\t\tif (*path == '!')\n-\t\t\tpath++;\n-\t\tif (*path == '/') {\n-\t\t\tpath++;\n-\t\t\tis_dir = 1;\n-\t\t}\n-\t\tif (is_dir && dir_prefix(base, path)) {\n+\tfor (p = common_list; p->dirname; p++) {\n+\t\tconst char *path = p->dirname;\n+\t\tif (p->is_dir && dir_prefix(base, path)) {\n \t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n \t\t\treturn;\n \t\t}\n-\t\tif (!is_dir && !strcmp(base, path)) {\n+\t\tif (!p->is_dir && !strcmp(base, path)) {\n \t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n \t\t\treturn;\n \t\t}\n@@ -129,16 +145,16 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len)\n void report_linked_checkout_garbage(void)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n-\tconst char **p;\n+\tconst struct common_dir *p;\n \tint len;\n \n \tif (!git_common_dir_env)\n \t\treturn;\n \tstrbuf_addf(&sb, \"%s/\", get_git_dir());\n \tlen = sb.len;\n-\tfor (p = common_list; *p; p++) {\n-\t\tconst char *path = *p;\n-\t\tif (*path == '!')\n+\tfor (p = common_list; p->dirname; p++) {\n+\t\tconst char *path = p->dirname;\n+\t\tif (p->ignore_garbage)\n \t\t\tcontinue;\n \t\tstrbuf_setlen(&sb, len);\n \t\tstrbuf_addstr(&sb, path);\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"267953","messageId":"1439416645-19173-2-git-send-email-dturner@twopensource.com","threadId":"40076","inReplyTo":"1439416645-19173-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 2/4] path: optimize common dir checking","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-12T21:57:23Z","receivedAt":"2015-08-12T21:57:23Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Instead of a linear search over common_list to check whether\na path is common, use a trie.  The trie search operates on\npath prefixes, and handles excludes.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n\nProbably overkill, but maybe we could later use it for making exclude\nor sparse-checkout matching faster (or maybe we have to go all the way\nto McNaughton-Yamada for that to be truly worthwhile).\n\n---\n path.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 201 insertions(+), 14 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex 236f797..21a4ce7 100644\n--- a/path.c\n+++ b/path.c\n@@ -121,25 +121,212 @@ struct common_dir common_list[] = {\n \t{ NULL, 0, 0, 0 }\n };\n \n-static void update_common_dir(struct strbuf *buf, int git_dir_len)\n+/*\n+ * A compressed trie.  A trie node consists of zero or more characters that\n+ * are common to all elements with this prefix, optionally followed by some\n+ * children.  If value is not NULL, the trie node is a terminal node.\n+ *\n+ * For example, consider the following set of strings:\n+ * abc\n+ * def\n+ * definite\n+ * definition\n+ *\n+ * The trie would look look like:\n+ * root: len = 0, value = (something), children a and d non-NULL.\n+ *    a: len = 2, contents = bc\n+ *    d: len = 2, contents = ef, children i non-NULL, value = (something)\n+ *       i: len = 3, contents = nit, children e and i non-NULL, value = NULL\n+ *           e: len = 0, children all NULL, value = (something)\n+ *           i: len = 2, contents = on, children all NULL, value = (something)\n+ */\n+struct trie {\n+\tstruct trie *children[256];\n+\tint len;\n+\tchar *contents;\n+\tvoid *value;\n+};\n+\n+static struct trie *make_trie_node(const char *key, void *value)\n {\n-\tchar *base = buf->buf + git_dir_len;\n-\tconst struct common_dir *p;\n+\tstruct trie *new_node = xcalloc(1, sizeof(*new_node));\n+\tnew_node->len = strlen(key);\n+\tif (new_node->len) {\n+\t\tnew_node->contents = xmalloc(new_node->len);\n+\t\tmemcpy(new_node->contents, key, new_node->len);\n+\t}\n+\tnew_node->value = value;\n+\treturn new_node;\n+}\n \n-\tif (is_dir_file(base, \"logs\", \"HEAD\") ||\n-\t    is_dir_file(base, \"info\", \"sparse-checkout\"))\n-\t\treturn;\t/* keep this in $GIT_DIR */\n-\tfor (p = common_list; p->dirname; p++) {\n-\t\tconst char *path = p->dirname;\n-\t\tif (p->is_dir && dir_prefix(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n-\t\t\treturn;\n+/*\n+ * Add a key/value pair to a trie.  The key is assumed to be \\0-terminated.\n+ * If there was an existing value for this key, return it.\n+ */\n+static void *add_to_trie(struct trie *root, const char *key, void *value)\n+{\n+\tstruct trie *child;\n+\tvoid *old;\n+\tint i;\n+\n+\tif (!*key) {\n+\t\t/* we have reached the end of the key */\n+\t\told = root->value;\n+\t\troot->value = value;\n+\t\treturn old;\n+\t}\n+\n+\tfor (i = 0; i < root->len; ++i) {\n+\t\tif (root->contents[i] == key[i])\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * Split this node: child will contain this node's\n+\t\t * existing children.\n+\t\t */\n+\t\tchild = malloc(sizeof(*child));\n+\t\tmemcpy(child->children, root->children, sizeof(root->children));\n+\n+\t\tchild->len = root->len - i - 1;\n+\t\tif (child->len) {\n+\t\t\tchild->contents = strndup(root->contents + i + 1,\n+\t\t\t\t\t\t   child->len);\n \t\t}\n-\t\tif (!p->is_dir && !strcmp(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n-\t\t\treturn;\n+\t\tchild->value = root->value;\n+\t\troot->value = NULL;\n+\t\troot->len = i;\n+\n+\t\tmemset(root->children, 0, sizeof(root->children));\n+\t\troot->children[(unsigned char)root->contents[i]] = child;\n+\n+\t\t/* This is the newly-added child. */\n+\t\troot->children[(unsigned char)key[i]] =\n+\t\t\tmake_trie_node(key + i + 1, value);\n+\t\treturn NULL;\n+\t}\n+\n+\t/* We have matched the entire compressed section */\n+\tif (key[i]) {\n+\t\tchild = root->children[(unsigned char)key[root->len]];\n+\t\tif (child) {\n+\t\t\treturn add_to_trie(child, key + root->len + 1, value);\n+\t\t} else {\n+\t\t\tchild = make_trie_node(key + root->len + 1, value);\n+\t\t\troot->children[(unsigned char)key[root->len]] = child;\n+\t\t\treturn NULL;\n \t\t}\n \t}\n+\n+\told = root->value;\n+\troot->value = value;\n+\treturn old;\n+}\n+\n+typedef int (*match_fn)(const char *unmatched, void *data, void *baton);\n+\n+/*\n+ * Search a trie for some key.  Find the longest /-or-\\0-terminated\n+ * prefix of the key for which the trie contains a value.  Call fn\n+ * with the unmatched portion of the key and the found value, and\n+ * return its return value.  If there is no such prefix, return -1.\n+ *\n+ * For example, consider the trie containing only [refs,\n+ * refs/worktree] (both with values).\n+ *\n+ * | key             | unmatched  | val from node | return value |\n+ * |-----------------|------------|---------------|--------------|\n+ * | a               | not called | n/a           | -1           |\n+ * | refs            | \\0         | refs          | as per fn    |\n+ * | refs/           | /          | refs          | as per fn    |\n+ * | refs/w          | /w         | refs          | as per fn    |\n+ * | refs/worktree   | \\0         | refs/worktree | as per fn    |\n+ * | refs/worktree/  | /          | refs/worktree | as per fn    |\n+ * | refs/worktree/a | /a         | refs/worktree | as per fn    |\n+ * |-----------------|------------|---------------|--------------|\n+ *\n+ */\n+static int trie_find(struct trie *root, const char *key, match_fn fn,\n+\t\t     void *baton)\n+{\n+\tint i;\n+\tint result;\n+\tstruct trie *child;\n+\n+\tif (!*key) {\n+\t\t/* we have reached the end of the key */\n+\t\tif (root->value && !root->len)\n+\t\t\treturn fn(key, root->value, baton);\n+\t\telse\n+\t\t\treturn -1;\n+\t}\n+\n+\tfor (i = 0; i < root->len; ++i) {\n+\t\tif (root->contents[i] != key[i])\n+\t\t\treturn -1;\n+\t}\n+\n+\t/* Matched the entire compressed section */\n+\tkey += i;\n+\tif (!*key)\n+\t\t/* End of key */\n+\t\treturn fn(key, root->value, baton);\n+\n+\tchild = root->children[(unsigned char)*key];\n+\tif (child)\n+\t\tresult = trie_find(child, key + 1, fn, baton);\n+\telse\n+\t\tresult = -1;\n+\n+\tif (result >= 0 || (*key != '/' && *key != 0))\n+\t\treturn result;\n+\tif (root->value)\n+\t\treturn fn(key, root->value, baton);\n+\telse\n+\t\treturn -1;\n+}\n+\n+static struct trie common_trie;\n+static int common_trie_done_setup;\n+\n+static void init_common_trie(void)\n+{\n+\tstruct common_dir *p;\n+\n+\tif (common_trie_done_setup)\n+\t\treturn;\n+\n+\tfor (p = common_list; p->dirname; p++)\n+\t\tadd_to_trie(&common_trie, p->dirname, p);\n+\n+\tcommon_trie_done_setup = 1;\n+}\n+\n+/*\n+ * Helper function for update_common_dir: returns 1 if the dir\n+ * prefix is common.\n+ */\n+static int check_common(const char *unmatched, void *value, void *baton)\n+{\n+\tstruct common_dir *dir = value;\n+\n+\tif (!dir)\n+\t\treturn 0;\n+\n+\tif (dir->is_dir && (unmatched[0] == 0 || unmatched[0] == '/'))\n+\t\treturn !dir->exclude;\n+\n+\tif (!dir->is_dir && unmatched[0] == 0)\n+\t\treturn !dir->exclude;\n+\n+\treturn 0;\n+}\n+\n+static void update_common_dir(struct strbuf *buf, int git_dir_len)\n+{\n+\tchar *base = buf->buf + git_dir_len;\n+\tinit_common_trie();\n+\tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n+\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n }\n \n void report_linked_checkout_garbage(void)\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"267954","messageId":"1439416645-19173-3-git-send-email-dturner@twopensource.com","threadId":"40076","inReplyTo":"1439416645-19173-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-12T21:57:24Z","receivedAt":"2015-08-12T21:57:24Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"We need a place to stick refs for bisects in progress that is not\nshared between worktrees.  So we use the refs/worktree/ hierarchy.\n\nThe is_per_worktree_ref function and associated docs learn that\nrefs/worktree/ is per-worktree, as does the git_path code in path.c\n\nThe ref-packing functions learn that per-worktree refs should not be\npacked (since packed-refs is common rather than per-worktree).\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n Documentation/glossary-content.txt |  5 +++--\n path.c                             |  1 +\n refs.c                             | 32 +++++++++++++++++++++++++++++++-\n t/t0060-path-utils.sh              |  1 +\n t/t1400-update-ref.sh              | 18 ++++++++++++++++++\n t/t3210-pack-refs.sh               |  7 +++++++\n 6 files changed, 61 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex 8c6478b..5c707e6 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -413,8 +413,9 @@ exclude;;\n \n [[def_per_worktree_ref]]per-worktree ref::\n \tRefs that are per-<<def_working_tree,worktree>>, rather than\n-\tglobal.  This is presently only <<def_HEAD,HEAD>>, but might\n-\tlater include other unusual refs.\n+\tglobal.  This is presently only <<def_HEAD,HEAD>> and any refs\n+\tthat start with `refs/worktree/`, but might later include other\n+\tunusual refs.\n \n [[def_pseudoref]]pseudoref::\n \tPseudorefs are a class of files under `$GIT_DIR` which behave\ndiff --git a/path.c b/path.c\nindex 21a4ce7..c53f732 100644\n--- a/path.c\n+++ b/path.c\n@@ -110,6 +110,7 @@ struct common_dir common_list[] = {\n \t{ \"lost-found\", 0, 1, 0 },\n \t{ \"objects\", 0, 1, 0 },\n \t{ \"refs\", 0, 1, 0 },\n+\t{ \"refs/worktree\", 0, 1, 1 },\n \t{ \"remotes\", 0, 1, 0 },\n \t{ \"worktrees\", 0, 1, 0 },\n \t{ \"rr-cache\", 0, 1, 0 },\ndiff --git a/refs.c b/refs.c\nindex e6fc3fe..30331bc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -298,6 +298,11 @@ struct ref_entry {\n };\n \n static void read_loose_refs(const char *dirname, struct ref_dir *dir);\n+static int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len);\n+static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache,\n+\t\t\t\t\t  const char *dirname, size_t len,\n+\t\t\t\t\t  int incomplete);\n+static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);\n \n static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n {\n@@ -306,6 +311,24 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \tdir = &entry->u.subdir;\n \tif (entry->flag & REF_INCOMPLETE) {\n \t\tread_loose_refs(entry->name, dir);\n+\n+\t\t/*\n+\t\t * Manually add refs/worktree, which, being\n+\t\t * per-worktree, might not appear in the directory\n+\t\t * listing for refs/ in the main repo.\n+\t\t */\n+\t\tif (!strcmp(entry->name, \"refs/\")) {\n+\t\t\tint pos = search_ref_dir(dir, \"refs/worktree/\", 14);\n+\t\t\tif (pos < 0) {\n+\t\t\t\tstruct ref_entry *child_entry;\n+\t\t\t\tchild_entry = create_dir_entry(dir->ref_cache,\n+\t\t\t\t\t\t\t       \"refs/worktree/\",\n+\t\t\t\t\t\t\t       14, 1);\n+\t\t\t\tadd_entry_to_dir(dir, child_entry);\n+\t\t\t\tread_loose_refs(\"refs/worktree\",\n+\t\t\t\t\t\t&child_entry->u.subdir);\n+\t\t\t}\n+\t\t}\n \t\tentry->flag &= ~REF_INCOMPLETE;\n \t}\n \treturn dir;\n@@ -2643,6 +2666,8 @@ struct pack_refs_cb_data {\n \tstruct ref_to_prune *ref_to_prune;\n };\n \n+static int is_per_worktree_ref(const char *refname);\n+\n /*\n  * An each_ref_entry_fn that is run over loose references only.  If\n  * the loose reference can be packed, add an entry in the packed ref\n@@ -2656,6 +2681,10 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \tstruct ref_entry *packed_entry;\n \tint is_tag_ref = starts_with(entry->name, \"refs/tags/\");\n \n+\t/* Do not pack per-worktree refs: */\n+\tif (is_per_worktree_ref(entry->name))\n+\t\treturn 0;\n+\n \t/* ALWAYS pack tags */\n \tif (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref)\n \t\treturn 0;\n@@ -2850,7 +2879,8 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n \n static int is_per_worktree_ref(const char *refname)\n {\n-\treturn !strcmp(refname, \"HEAD\");\n+\treturn !strcmp(refname, \"HEAD\") ||\n+\t\tstarts_with(refname, \"refs/worktree/\");\n }\n \n static int is_pseudoref_syntax(const char *refname)\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 93605f4..28e6dff 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -275,6 +275,7 @@ test_git_path GIT_COMMON_DIR=bar remotes/bar              bar/remotes/bar\n test_git_path GIT_COMMON_DIR=bar branches/bar             bar/branches/bar\n test_git_path GIT_COMMON_DIR=bar logs/refs/heads/master   bar/logs/refs/heads/master\n test_git_path GIT_COMMON_DIR=bar refs/heads/master        bar/refs/heads/master\n+test_git_path GIT_COMMON_DIR=bar refs/worktree/foo        .git/refs/worktree/foo\n test_git_path GIT_COMMON_DIR=bar hooks/me                 bar/hooks/me\n test_git_path GIT_COMMON_DIR=bar config                   bar/config\n test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 9d21c19..7ecd33b 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1131,4 +1131,22 @@ test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches\n )\n '\n \n+test_expect_success 'handle per-worktree refs in refs/worktree' '\n+\tgit commit --allow-empty -m \"initial commit\" &&\n+\tgit worktree add -b branch worktree &&\n+\t(\n+\t\tcd worktree &&\n+\t\tgit commit --allow-empty -m \"test commit\"  &&\n+\t\tgit for-each-ref | test_must_fail grep refs/worktree &&\n+\t\tgit update-ref refs/worktree/something HEAD &&\n+\t\tgit rev-parse refs/worktree/something >../worktree-head &&\n+\t\tgit for-each-ref | grep refs/worktree/something\n+\t) &&\n+\ttest_path_is_missing .git/refs/worktree &&\n+\ttest_must_fail git rev-parse refs/worktree/something &&\n+\tgit update-ref refs/worktree/something HEAD &&\n+\tgit rev-parse refs/worktree/something >main-head &&\n+\t! test_cmp main-head worktree-head\n+'\n+\n test_done\ndiff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh\nindex 8aae98d..c54cd29 100755\n--- a/t/t3210-pack-refs.sh\n+++ b/t/t3210-pack-refs.sh\n@@ -160,6 +160,13 @@ test_expect_success 'pack ref directly below refs/' '\n \ttest_path_is_missing .git/refs/top\n '\n \n+test_expect_success 'do not pack ref in refs/worktree' '\n+\tgit update-ref refs/worktree/local HEAD &&\n+\tgit pack-refs --all --prune &&\n+\t! grep refs/worktree/local .git/packed-refs >/dev/null &&\n+\ttest_path_is_file .git/refs/worktree/local\n+'\n+\n test_expect_success 'disable reflogs' '\n \tgit config core.logallrefupdates false &&\n \trm -rf .git/logs\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"267955","messageId":"1439416645-19173-4-git-send-email-dturner@twopensource.com","threadId":"40076","inReplyTo":"1439416645-19173-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 4/4] bisect: make bisection refs per-worktree","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-12T21:57:25Z","receivedAt":"2015-08-12T21:57:25Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Using the new refs/worktree/ refs, make bisection per-worktree.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n Documentation/git-bisect.txt       |  4 ++--\n Documentation/rev-list-options.txt | 14 +++++++-------\n bisect.c                           |  2 +-\n builtin/rev-parse.c                |  6 ++++--\n git-bisect.sh                      | 14 +++++++-------\n revision.c                         |  2 +-\n t/t6030-bisect-porcelain.sh        | 20 ++++++++++----------\n 7 files changed, 32 insertions(+), 30 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex e97f2de..f0c52d1 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -82,7 +82,7 @@ to ask for the next commit that needs testing.\n \n Eventually there will be no more revisions left to inspect, and the\n command will print out a description of the first bad commit. The\n-reference `refs/bisect/bad` will be left pointing at that commit.\n+reference `refs/worktree/bisect/bad` will be left pointing at that commit.\n \n \n Bisect reset\n@@ -373,7 +373,7 @@ on a single line.\n ------------\n $ git bisect start HEAD <known-good-commit> [ <boundary-commit> ... ] --no-checkout\n $ git bisect run sh -c '\n-\tGOOD=$(git for-each-ref \"--format=%(objectname)\" refs/bisect/good-*) &&\n+\tGOOD=$(git for-each-ref \"--format=%(objectname)\" refs/worktree/bisect/good-*) &&\n \tgit rev-list --objects BISECT_HEAD --not $GOOD >tmp.$$ &&\n \tgit pack-objects --stdout >/dev/null <tmp.$$\n \trc=$?\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex a9b808f..1175960 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -183,9 +183,9 @@ explicitly.\n \n ifndef::git-rev-list[]\n --bisect::\n-\tPretend as if the bad bisection ref `refs/bisect/bad`\n+\tPretend as if the bad bisection ref `refs/worktree/bisect/bad`\n \twas listed and as if it was followed by `--not` and the good\n-\tbisection refs `refs/bisect/good-*` on the command\n+\tbisection refs `refs/worktree/bisect/good-*` on the command\n \tline. Cannot be combined with --first-parent.\n endif::git-rev-list[]\n \n@@ -548,10 +548,10 @@ Bisection Helpers\n --bisect::\n \tLimit output to the one commit object which is roughly halfway between\n \tincluded and excluded commits. Note that the bad bisection ref\n-\t`refs/bisect/bad` is added to the included commits (if it\n-\texists) and the good bisection refs `refs/bisect/good-*` are\n+\t`refs/worktree/bisect/bad` is added to the included commits (if it\n+\texists) and the good bisection refs `refs/worktree/bisect/good-*` are\n \tadded to the excluded commits (if they exist). Thus, supposing there\n-\tare no refs in `refs/bisect/`, if\n+\tare no refs in `refs/worktree/bisect/`, if\n +\n -----------------------------------------------------------------------\n \t$ git rev-list --bisect foo ^bar ^baz\n@@ -571,7 +571,7 @@ one. Cannot be combined with --first-parent.\n \n --bisect-vars::\n \tThis calculates the same as `--bisect`, except that refs in\n-\t`refs/bisect/` are not used, and except that this outputs\n+\t`refs/worktree/bisect/` are not used, and except that this outputs\n \ttext ready to be eval'ed by the shell. These lines will assign the\n \tname of the midpoint revision to the variable `bisect_rev`, and the\n \texpected number of commits to be tested after `bisect_rev` is tested\n@@ -584,7 +584,7 @@ one. Cannot be combined with --first-parent.\n --bisect-all::\n \tThis outputs all the commit objects between the included and excluded\n \tcommits, ordered by their distance to the included and excluded\n-\tcommits. Refs in `refs/bisect/` are not used. The farthest\n+\tcommits. Refs in `refs/worktree/bisect/` are not used. The farthest\n \tfrom them is displayed first. (This is the only one displayed by\n \t`--bisect`.)\n +\ndiff --git a/bisect.c b/bisect.c\nindex 33ac88d..dbe3461 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -425,7 +425,7 @@ static int register_ref(const char *refname, const struct object_id *oid,\n \n static int read_bisect_refs(void)\n {\n-\treturn for_each_ref_in(\"refs/bisect/\", register_ref, NULL);\n+\treturn for_each_ref_in(\"refs/worktree/bisect/\", register_ref, NULL);\n }\n \n static void read_bisect_paths(struct argv_array *array)\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 02d747d..3240ddf 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -663,8 +663,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--bisect\")) {\n-\t\t\t\tfor_each_ref_in(\"refs/bisect/bad\", show_reference, NULL);\n-\t\t\t\tfor_each_ref_in(\"refs/bisect/good\", anti_reference, NULL);\n+\t\t\t\tfor_each_ref_in(\"refs/worktree/bisect/bad\",\n+\t\t\t\t\t\tshow_reference, NULL);\n+\t\t\t\tfor_each_ref_in(\"refs/worktree/bisect/good\",\n+\t\t\t\t\t\tanti_reference, NULL);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (starts_with(arg, \"--branches=\")) {\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex ea63223..c16433d 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -210,7 +210,7 @@ bisect_write() {\n \t\t*)\n \t\t\tdie \"$(eval_gettext \"Bad bisect_write argument: \\$state\")\" ;;\n \tesac\n-\tgit update-ref \"refs/bisect/$tag\" \"$rev\" || exit\n+\tgit update-ref \"refs/worktree/bisect/$tag\" \"$rev\" || exit\n \techo \"# $state: $(git show-branch $rev)\" >>\"$GIT_DIR/BISECT_LOG\"\n \ttest -n \"$nolog\" || echo \"git bisect $state $rev\" >>\"$GIT_DIR/BISECT_LOG\"\n }\n@@ -282,8 +282,8 @@ bisect_state() {\n \n bisect_next_check() {\n \tmissing_good= missing_bad=\n-\tgit show-ref -q --verify refs/bisect/$TERM_BAD || missing_bad=t\n-\ttest -n \"$(git for-each-ref \"refs/bisect/$TERM_GOOD-*\")\" || missing_good=t\n+\tgit show-ref -q --verify refs/worktree/bisect/$TERM_BAD || missing_bad=t\n+\ttest -n \"$(git for-each-ref \"refs/worktree/bisect/$TERM_GOOD-*\")\" || missing_good=t\n \n \tcase \"$missing_good,$missing_bad,$1\" in\n \t,,*)\n@@ -341,15 +341,15 @@ bisect_next() {\n \t# Check if we should exit because bisection is finished\n \tif test $res -eq 10\n \tthen\n-\t\tbad_rev=$(git show-ref --hash --verify refs/bisect/$TERM_BAD)\n+\t\tbad_rev=$(git show-ref --hash --verify refs/worktree/bisect/$TERM_BAD)\n \t\tbad_commit=$(git show-branch $bad_rev)\n \t\techo \"# first $TERM_BAD commit: $bad_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n \t\texit 0\n \telif test $res -eq 2\n \tthen\n \t\techo \"# only skipped commits left to test\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/bisect/$TERM_GOOD-*\")\n-\t\tfor skipped in $(git rev-list refs/bisect/$TERM_BAD --not $good_revs)\n+\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/worktree/bisect/$TERM_GOOD-*\")\n+\t\tfor skipped in $(git rev-list refs/worktree/bisect/$TERM_BAD --not $good_revs)\n \t\tdo\n \t\t\tskipped_commit=$(git show-branch $skipped)\n \t\t\techo \"# possible first $TERM_BAD commit: $skipped_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n@@ -412,7 +412,7 @@ Try 'git bisect reset <commit>'.\")\"\n \n bisect_clean_state() {\n \t# There may be some refs packed during bisection.\n-\tgit for-each-ref --format='%(refname) %(objectname)' refs/bisect/\\* |\n+\tgit for-each-ref --format='%(refname) %(objectname)' refs/worktree/bisect/\\* |\n \twhile read ref hash\n \tdo\n \t\tgit update-ref -d $ref $hash || exit\ndiff --git a/revision.c b/revision.c\nindex b6b2cf7..974a11f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2084,7 +2084,7 @@ extern void read_bisect_terms(const char **bad, const char **good);\n static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data, const char *term) {\n \tstruct strbuf bisect_refs = STRBUF_INIT;\n \tint status;\n-\tstrbuf_addf(&bisect_refs, \"refs/bisect/%s\", term);\n+\tstrbuf_addf(&bisect_refs, \"refs/worktree/bisect/%s\", term);\n \tstatus = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);\n \tstrbuf_release(&bisect_refs);\n \treturn status;\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 9e2c203..bfd5463 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -68,7 +68,7 @@ test_expect_success 'bisect fails if given any junk instead of revs' '\n \tgit bisect reset &&\n \ttest_must_fail git bisect start foo $HASH1 -- &&\n \ttest_must_fail git bisect start $HASH4 $HASH1 bar -- &&\n-\ttest -z \"$(git for-each-ref \"refs/bisect/*\")\" &&\n+\ttest -z \"$(git for-each-ref \"refs/worktree/bisect/*\")\" &&\n \ttest -z \"$(ls .git/BISECT_* 2>/dev/null)\" &&\n \tgit bisect start &&\n \ttest_must_fail git bisect good foo $HASH1 &&\n@@ -77,7 +77,7 @@ test_expect_success 'bisect fails if given any junk instead of revs' '\n \ttest_must_fail git bisect bad $HASH3 $HASH4 &&\n \ttest_must_fail git bisect skip bar $HASH3 &&\n \ttest_must_fail git bisect skip $HASH1 foo &&\n-\ttest -z \"$(git for-each-ref \"refs/bisect/*\")\" &&\n+\ttest -z \"$(git for-each-ref \"refs/worktree/bisect/*\")\" &&\n \tgit bisect good $HASH1 &&\n \tgit bisect bad $HASH4\n '\n@@ -115,7 +115,7 @@ test_expect_success 'bisect reset removes packed refs' '\n \tgit pack-refs --all --prune &&\n \tgit bisect next &&\n \tgit bisect reset &&\n-\ttest -z \"$(git for-each-ref \"refs/bisect/*\")\" &&\n+\ttest -z \"$(git for-each-ref \"refs/worktree/bisect/*\")\" &&\n \ttest -z \"$(git for-each-ref \"refs/heads/bisect\")\"\n '\n \n@@ -126,7 +126,7 @@ test_expect_success 'bisect reset removes bisect state after --no-checkout' '\n \tgit bisect bad $HASH3 &&\n \tgit bisect next &&\n \tgit bisect reset &&\n-\ttest -z \"$(git for-each-ref \"refs/bisect/*\")\" &&\n+\ttest -z \"$(git for-each-ref \"refs/worktree/bisect/*\")\" &&\n \ttest -z \"$(git for-each-ref \"refs/heads/bisect\")\" &&\n \ttest -z \"$(git for-each-ref \"BISECT_HEAD\")\"\n '\n@@ -176,7 +176,7 @@ test_expect_success 'bisect start: no \".git/BISECT_START\" if checkout error' '\n \tgit branch > branch.output &&\n \tgrep \"* other\" branch.output > /dev/null &&\n \ttest_must_fail test -e .git/BISECT_START &&\n-\ttest -z \"$(git for-each-ref \"refs/bisect/*\")\" &&\n+\ttest -z \"$(git for-each-ref \"refs/worktree/bisect/*\")\" &&\n \tgit checkout HEAD hello\n '\n \n@@ -671,7 +671,7 @@ test_expect_success 'bisect: --no-checkout - target before breakage' '\n \tgit bisect bad BISECT_HEAD &&\n \tcheck_same BROKEN_HASH5 BISECT_HEAD &&\n \tgit bisect bad BISECT_HEAD &&\n-\tcheck_same BROKEN_HASH5 bisect/bad &&\n+\tcheck_same BROKEN_HASH5 refs/worktree/bisect/bad &&\n \tgit bisect reset\n '\n \n@@ -682,7 +682,7 @@ test_expect_success 'bisect: --no-checkout - target in breakage' '\n \tgit bisect bad BISECT_HEAD &&\n \tcheck_same BROKEN_HASH5 BISECT_HEAD &&\n \tgit bisect good BISECT_HEAD &&\n-\tcheck_same BROKEN_HASH6 bisect/bad &&\n+\tcheck_same BROKEN_HASH6 refs/worktree/bisect/bad &&\n \tgit bisect reset\n '\n \n@@ -693,7 +693,7 @@ test_expect_success 'bisect: --no-checkout - target after breakage' '\n \tgit bisect good BISECT_HEAD &&\n \tcheck_same BROKEN_HASH8 BISECT_HEAD &&\n \tgit bisect good BISECT_HEAD &&\n-\tcheck_same BROKEN_HASH9 bisect/bad &&\n+\tcheck_same BROKEN_HASH9 refs/worktree/bisect/bad &&\n \tgit bisect reset\n '\n \n@@ -702,13 +702,13 @@ test_expect_success 'bisect: demonstrate identification of damage boundary' \"\n \tgit checkout broken &&\n \tgit bisect start broken master --no-checkout &&\n \tgit bisect run \\\"\\$SHELL_PATH\\\" -c '\n-\t\tGOOD=\\$(git for-each-ref \\\"--format=%(objectname)\\\" refs/bisect/good-*) &&\n+\t\tGOOD=\\$(git for-each-ref \\\"--format=%(objectname)\\\" refs/worktree/bisect/good-*) &&\n \t\tgit rev-list --objects BISECT_HEAD --not \\$GOOD >tmp.\\$\\$ &&\n \t\tgit pack-objects --stdout >/dev/null < tmp.\\$\\$\n \t\trc=\\$?\n \t\trm -f tmp.\\$\\$\n \t\ttest \\$rc = 0' &&\n-\tcheck_same BROKEN_HASH6 bisect/bad &&\n+\tcheck_same BROKEN_HASH6 refs/worktree/bisect/bad &&\n \tgit bisect reset\n \"\n \n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"267960","messageId":"xmqqio8kjf7q.fsf@gitster.dls.corp.google.com","threadId":"40076","inReplyTo":"1439416645-19173-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-12T22:48:25Z","receivedAt":"2015-08-12T22:48:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> Instead of a linear search over common_list to check whether\n> a path is common, use a trie.  The trie search operates on\n> path prefixes, and handles excludes.\n>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n>\n> Probably overkill, but maybe we could later use it for making exclude\n> or sparse-checkout matching faster (or maybe we have to go all the way\n> to McNaughton-Yamada for that to be truly worthwhile).\n\nThis is why I love this list.  A mere mention of \"something better\nthan linear list\" immediately is answered by a trie and a mention of\nMcNaughton-Yamada ;-).\n"},{"id":"267975","messageId":"55CC5DED.5050304@alum.mit.edu","threadId":"40076","inReplyTo":"1439416645-19173-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-08-13T09:05:49Z","receivedAt":"2015-08-13T09:05:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/12/2015 11:57 PM, David Turner wrote:\n> Instead of a linear search over common_list to check whether\n> a path is common, use a trie.  The trie search operates on\n> path prefixes, and handles excludes.\n> \n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n> \n> Probably overkill, but maybe we could later use it for making exclude\n> or sparse-checkout matching faster (or maybe we have to go all the way\n> to McNaughton-Yamada for that to be truly worthwhile).\n\nLet's take a step back.\n\nWe have always had a ton of code that uses `git_path()` and friends to\nconvert abstract things into filesystem paths. Let's take the\nreference-handling code as an example:\n\n`git_path(\"refs/heads/master\")` returns something like\n\".git/refs/heads/master\", which happens to be the place where we would\nstore a loose reference with that name. But in reality,\n\"refs/heads/master\" is a reference name, not a fragment of a path. It's\njust that the reference code knows that the transformation done by\n`git_path()` *accidentally* happens to convert a reference name into the\nname of the path of the corresponding loose reference file.\n\nIn fact, the reference code is even smarter than that. It knows that\nwithin submodules, `git_path()` does *not* do the right mapping. In\nthose cases it calls `git_path_submodule()` instead.\n\nBut now we have workspaces, and things have become more complicated.\nSome references are stored in `$GIT_DIR`, while others are stored in\n`$GIT_COMMON_DIR`. Who should know all of these details?\n\nThe current answer is that the reference-handling code remains (mostly)\nignorant of workspaces. It just stupidly calls `git_path()` (or\n`git_path_submodule()`) regardless of the reference name. It is\n`git_path()` that has grown the global insight to know which files are\nnow stored in `$GIT_COMMON_DIR` vs `$GIT_DIR`. Now it helpfully\ntransforms \"refs/heads/master\" into \"$GIT_COMMON_DIR/refs/heads/master\"\nbut transforms \"refs/worktree/foo\" into \"$GIT_DIR/refs/worktree/foo\". It\nhas developed similar insight into lots of other file types. IT KNOWS\nTOO MUCH. And because of that, it become a lot more complicated and\nmight even be a performance problem.\n\nThis seems crazy to me. It is the *reference* code that should know\nwhether a particular reference should be stored under `$GIT_DIR` or\n`$GIT_COMMON_DIR`, or indeed whether it should be stored in a database.\n\nWe should have two *stupid* functions, `git_workspace_path()` and\n`git_common_path()`, and have the *callers* decide which one to call.\n\nThe only reason to retain a knows-everything `git_path()` function is as\na crutch for 3rd-party applications that think they are clever enough to\ngrub around in `$GIT_DIR` at the filesystem level. But that should be\nhighly discouraged, and we should make it our mission to provide\ncommands that make it unnecessary.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"267999","messageId":"CAPig+cRM-J-5SHJ9JVsHPR+B2Y05nwFrRQWsd0M56pm9jS-mTQ@mail.gmail.com","threadId":"40076","inReplyTo":"1439416645-19173-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-13T17:15:28Z","receivedAt":"2015-08-13T17:15:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Aug 12, 2015 at 5:57 PM, David Turner <dturner@twopensource.com> wrote:\n> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n> index 93605f4..28e6dff 100755\n> --- a/t/t0060-path-utils.sh\n> +++ b/t/t0060-path-utils.sh\n> +test_expect_success 'handle per-worktree refs in refs/worktree' '\n> +       git commit --allow-empty -m \"initial commit\" &&\n> +       git worktree add -b branch worktree &&\n> +       (\n> +               cd worktree &&\n> +               git commit --allow-empty -m \"test commit\"  &&\n> +               git for-each-ref | test_must_fail grep refs/worktree &&\n\ns/test_must_fail/!/\n\n>From t/README:\n\n   On the other hand, don't use test_must_fail for running regular\n   platform commands; just use '! cmd'.  We are not in the business\n   of verifying that the world given to us sanely works.\n\n> +               git update-ref refs/worktree/something HEAD &&\n> +               git rev-parse refs/worktree/something >../worktree-head &&\n> +               git for-each-ref | grep refs/worktree/something\n> +       ) &&\n> +       test_path_is_missing .git/refs/worktree &&\n> +       test_must_fail git rev-parse refs/worktree/something &&\n> +       git update-ref refs/worktree/something HEAD &&\n> +       git rev-parse refs/worktree/something >main-head &&\n> +       ! test_cmp main-head worktree-head\n> +'\n"},{"id":"268001","messageId":"1439487708.8855.2.camel@twopensource.com","threadId":"40076","inReplyTo":"CAPig+cRM-J-5SHJ9JVsHPR+B2Y05nwFrRQWsd0M56pm9jS-mTQ@mail.gmail.com","subject":"Re: [PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-13T17:41:48Z","receivedAt":"2015-08-13T17:41:48Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2015-08-13 at 13:15 -0400, Eric Sunshine wrote:\n> On Wed, Aug 12, 2015 at 5:57 PM, David Turner <dturner@twopensource.com> wrote:\n> > diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n> > index 93605f4..28e6dff 100755\n> > --- a/t/t0060-path-utils.sh\n> > +++ b/t/t0060-path-utils.sh\n> > +test_expect_success 'handle per-worktree refs in refs/worktree' '\n> > +       git commit --allow-empty -m \"initial commit\" &&\n> > +       git worktree add -b branch worktree &&\n> > +       (\n> > +               cd worktree &&\n> > +               git commit --allow-empty -m \"test commit\"  &&\n> > +               git for-each-ref | test_must_fail grep refs/worktree &&\n> \n> s/test_must_fail/!/\n> \n> From t/README:\n> \n>    On the other hand, don't use test_must_fail for running regular\n>    platform commands; just use '! cmd'.  We are not in the business\n>    of verifying that the world given to us sanely works.\n\nWhen I make that change, my test fails with:\n\nFATAL: Unexpected exit with code 2\n\nApparently, you can't use ! in pipelines like that.  So that's why I\nused test_must_fail.\n"},{"id":"268026","messageId":"55CCFB32.30608@alum.mit.edu","threadId":"40076","inReplyTo":"1439487708.8855.2.camel@twopensource.com","subject":"Re: [PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-08-13T20:16:50Z","receivedAt":"2015-08-13T20:16:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/13/2015 07:41 PM, David Turner wrote:\n> On Thu, 2015-08-13 at 13:15 -0400, Eric Sunshine wrote:\n>> On Wed, Aug 12, 2015 at 5:57 PM, David Turner <dturner@twopensource.com> wrote:\n>>> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n>>> index 93605f4..28e6dff 100755\n>>> --- a/t/t0060-path-utils.sh\n>>> +++ b/t/t0060-path-utils.sh\n>>> +test_expect_success 'handle per-worktree refs in refs/worktree' '\n>>> +       git commit --allow-empty -m \"initial commit\" &&\n>>> +       git worktree add -b branch worktree &&\n>>> +       (\n>>> +               cd worktree &&\n>>> +               git commit --allow-empty -m \"test commit\"  &&\n>>> +               git for-each-ref | test_must_fail grep refs/worktree &&\n>>\n>> s/test_must_fail/!/\n>>\n>> From t/README:\n>>\n>>    On the other hand, don't use test_must_fail for running regular\n>>    platform commands; just use '! cmd'.  We are not in the business\n>>    of verifying that the world given to us sanely works.\n> \n> When I make that change, my test fails with:\n> \n> FATAL: Unexpected exit with code 2\n> \n> Apparently, you can't use ! in pipelines like that.  So that's why I\n> used test_must_fail.\n\nYou would have to negate the whole pipeline, like\n\n    ! git for-each-ref | grep refs/worktree\n\nThe result of a pipeline is taken from the last command.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"268028","messageId":"1439497946.8855.22.camel@twopensource.com","threadId":"40076","inReplyTo":"55CCFB32.30608@alum.mit.edu","subject":"Re: [PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-13T20:32:26Z","receivedAt":"2015-08-13T20:32:26Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2015-08-13 at 22:16 +0200, Michael Haggerty wrote:\n> On 08/13/2015 07:41 PM, David Turner wrote:\n> > On Thu, 2015-08-13 at 13:15 -0400, Eric Sunshine wrote:\n> >> On Wed, Aug 12, 2015 at 5:57 PM, David Turner <dturner@twopensource.com> wrote:\n> >>> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n> >>> index 93605f4..28e6dff 100755\n> >>> --- a/t/t0060-path-utils.sh\n> >>> +++ b/t/t0060-path-utils.sh\n> >>> +test_expect_success 'handle per-worktree refs in refs/worktree' '\n> >>> +       git commit --allow-empty -m \"initial commit\" &&\n> >>> +       git worktree add -b branch worktree &&\n> >>> +       (\n> >>> +               cd worktree &&\n> >>> +               git commit --allow-empty -m \"test commit\"  &&\n> >>> +               git for-each-ref | test_must_fail grep refs/worktree &&\n> >>\n> >> s/test_must_fail/!/\n> >>\n> >> From t/README:\n> >>\n> >>    On the other hand, don't use test_must_fail for running regular\n> >>    platform commands; just use '! cmd'.  We are not in the business\n> >>    of verifying that the world given to us sanely works.\n> > \n> > When I make that change, my test fails with:\n> > \n> > FATAL: Unexpected exit with code 2\n> > \n> > Apparently, you can't use ! in pipelines like that.  So that's why I\n> > used test_must_fail.\n> \n> You would have to negate the whole pipeline, like\n> \n>     ! git for-each-ref | grep refs/worktree\n> \n> The result of a pipeline is taken from the last command.\n\nYes, but that would pass if for-each-ref fails, which I do not want.  \n\nJacob's suggestion of parentheses around (! grep refs/worktree) seems to\nwork.\n"},{"id":"268032","messageId":"55CDA445.9090503@alum.mit.edu","threadId":"40076","inReplyTo":"1439497946.8855.22.camel@twopensource.com","subject":"Re: [PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-08-14T08:18:13Z","receivedAt":"2015-08-14T08:18:13Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/13/2015 10:32 PM, David Turner wrote:\n> On Thu, 2015-08-13 at 22:16 +0200, Michael Haggerty wrote:\n>> On 08/13/2015 07:41 PM, David Turner wrote:\n>>> On Thu, 2015-08-13 at 13:15 -0400, Eric Sunshine wrote:\n>>>> On Wed, Aug 12, 2015 at 5:57 PM, David Turner <dturner@twopensource.com> wrote:\n>>>>> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n>>>>> index 93605f4..28e6dff 100755\n>>>>> --- a/t/t0060-path-utils.sh\n>>>>> +++ b/t/t0060-path-utils.sh\n>>>>> +test_expect_success 'handle per-worktree refs in refs/worktree' '\n>>>>> +       git commit --allow-empty -m \"initial commit\" &&\n>>>>> +       git worktree add -b branch worktree &&\n>>>>> +       (\n>>>>> +               cd worktree &&\n>>>>> +               git commit --allow-empty -m \"test commit\"  &&\n>>>>> +               git for-each-ref | test_must_fail grep refs/worktree &&\n>>>>\n>>>> s/test_must_fail/!/\n>>>>\n>>>> From t/README:\n>>>>\n>>>>    On the other hand, don't use test_must_fail for running regular\n>>>>    platform commands; just use '! cmd'.  We are not in the business\n>>>>    of verifying that the world given to us sanely works.\n>>>\n>>> When I make that change, my test fails with:\n>>>\n>>> FATAL: Unexpected exit with code 2\n>>>\n>>> Apparently, you can't use ! in pipelines like that.  So that's why I\n>>> used test_must_fail.\n>>\n>> You would have to negate the whole pipeline, like\n>>\n>>     ! git for-each-ref | grep refs/worktree\n>>\n>> The result of a pipeline is taken from the last command.\n> \n> Yes, but that would pass if for-each-ref fails, which I do not want.  \n> \n> Jacob's suggestion of parentheses around (! grep refs/worktree) seems to\n> work.\n\nI don't see how that can help. The result of a pipeline is taken from\nthe last command. The exit codes of earlier commands in the pipeline are\nlost in the sands of time:\n\n    $ false | true\n    $ echo $?\n    0\n    $ false | ( ! false )\n    $ echo $?\n    0\n\nWorking around this POSIX shell limitation is surprisingly awkward in a\ngeneral-purpose script. But in this case you could use a temporary file:\n\n    git for-each-ref >refs-actual &&\n    ! grep refs/worktree <refs-actual && [...]\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"268046","messageId":"xmqqtws1iyxn.fsf@gitster.dls.corp.google.com","threadId":"40076","inReplyTo":"55CC5DED.5050304@alum.mit.edu","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-14T17:04:36Z","receivedAt":"2015-08-14T17:04:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> Let's take a step back.\n>\n> We have always had a ton of code that uses `git_path()` and friends to\n> convert abstract things into filesystem paths. Let's take the\n> reference-handling code as an example:\n> ...\n> This seems crazy to me. It is the *reference* code that should know\n> whether a particular reference should be stored under `$GIT_DIR` or\n> `$GIT_COMMON_DIR`, or indeed whether it should be stored in a database.\n\nIt is more like:\n\n 1. The system as a whole should decide if HEAD and refs/heads/\n    should be per workspace or shared across a repository (and we say\n    the former should be per workspace, the latter should be shared).\n\n 2. The reference code should decide which ref-backend is used to\n    store refs.\n\n 3. And any ref-backend should follow the decision made by the\n    system as a whole in 1.\n\nI'd imagine that David's ref-backend code inherited from Ronnie\nwould still accept the string \"refs/heads/master\" from the rest of\nthe system (i.e. callers that call into the ref API) to mean \"the\nref that represents the 'master' branch\", and uses that as the key\nto decide \"ok, that is shared across workspaces\" to honor the\nsystem-wide decision made in 1.  The outside callers wouldn't pass\nthe result of calling git_path(\"refs/heads/master\") into the ref\nAPI, which may expand to \"$somewhere_else/refs/heads/master\" when\nrun in a secondary workspace to point at the common location.\n\nI'd also imagine that the workspace API would give ways for the\nimplementation of the reference API to ask these questions:\n\n - which workspace am I operating for?  where is the \"common\" thing?\n   how would I identify this workspace among the ones that share the\n   same \"common\" thing?\n\n - is this ref (or ref-like thing) supposed to be in common or per\n   workspace?\n\nI agree with you that there needs an intermediate level, between\nwhat Duy and David's git_path() does (i.e. which gives the final\nresult of deciding where in the filesystem the thing should go) and\nyour two stupid functions would do (i.e. knowing which kind the\nthing is, give you the final location in the filesystem).  That is,\nto let the caller know if the thing is supposed to be shared or per\nworkspace in the first place.\n\nWith that intermediate level function, a database-based ref-backend\ncould make ('common', ref/heads/master) as the key for the current\nvalue of the master branch and (workspace-name, HEAD) as the key for\nthe current value of the HEAD for a given workspace.\n\n> We should have two *stupid* functions, `git_workspace_path()` and\n> `git_common_path()`, and have the *callers* decide which one to call.\n\nSo I think we should have *three* functions:\n\n - git_workspace_name(void) returns some name that uniquely\n   identifies the current workspace among the workspaces linked to\n   the same repository.\n\n - is_common_thing(const char *) takes a path (that is relative to\n   $GIT_DIR where the thing would have lived at in a pre-workspace\n   world), and tells if it is a common thing or a per-workspace\n   thing.\n\n - git_path() can stay the external interface and can be thought of:\n\n\tgit_path(const char *path)\n        {\n\t\tif (is_common_thing(path))\n\t\t\treturn git_common_path(path);\n\t\treturn git_workspace_path(git_workspace_name(), path);\n\t}\n\n   if you think in terms of your two helpers.\n\nBut I do not think that git_common_path() and git_workspace_path()\nneed to be called from any other place in the system, and that is\nthe reason why I did not say we should have four functions (or five,\ncounting git_path() itself).\n"},{"id":"268047","messageId":"xmqqpp2piyo3.fsf@gitster.dls.corp.google.com","threadId":"40076","inReplyTo":"55CDA445.9090503@alum.mit.edu","subject":"Re: [PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-14T17:10:20Z","receivedAt":"2015-08-14T17:10:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> I don't see how that can help. The result of a pipeline is taken from\n> the last command. The exit codes of earlier commands in the pipeline are\n> lost in the sands of time:\n>\n>     $ false | true\n>     $ echo $?\n>     0\n>     $ false | ( ! false )\n>     $ echo $?\n>     0\n>\n> Working around this POSIX shell limitation is surprisingly awkward in a\n> general-purpose script. But in this case you could use a temporary file:\n>\n>     git for-each-ref >refs-actual &&\n>     ! grep refs/worktree <refs-actual && [...]\n\nIt is not just \"you could\", but that is what you \"should\" do, if you\ncared the exit status from for-each-ref.\n\nThanks.\n"},{"id":"268069","messageId":"1439582644.8855.89.camel@twopensource.com","threadId":"40076","inReplyTo":"xmqqtws1iyxn.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-14T20:04:04Z","receivedAt":"2015-08-14T20:04:04Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2015-08-14 at 10:04 -0700, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n> > Let's take a step back.\n> >\n> > We have always had a ton of code that uses `git_path()` and friends to\n> > convert abstract things into filesystem paths. Let's take the\n> > reference-handling code as an example:\n> > ...\n> > This seems crazy to me. It is the *reference* code that should know\n> > whether a particular reference should be stored under `$GIT_DIR` or\n> > `$GIT_COMMON_DIR`, or indeed whether it should be stored in a database.\n> \n> It is more like:\n> \n>  1. The system as a whole should decide if HEAD and refs/heads/\n>     should be per workspace or shared across a repository (and we say\n>     the former should be per workspace, the latter should be shared).\n> \n>  2. The reference code should decide which ref-backend is used to\n>     store refs.\n> \n>  3. And any ref-backend should follow the decision made by the\n>     system as a whole in 1.\n> \n> I'd imagine that David's ref-backend code inherited from Ronnie\n> would still accept the string \"refs/heads/master\" from the rest of\n> the system (i.e. callers that call into the ref API) to mean \"the\n> ref that represents the 'master' branch\", and uses that as the key\n> to decide \"ok, that is shared across workspaces\" to honor the\n> system-wide decision made in 1.  The outside callers wouldn't pass\n> the result of calling git_path(\"refs/heads/master\") into the ref\n> API, which may expand to \"$somewhere_else/refs/heads/master\" when\n> run in a secondary workspace to point at the common location.\n> \n> I'd also imagine that the workspace API would give ways for the\n> implementation of the reference API to ask these questions:\n> \n>  - which workspace am I operating for?  where is the \"common\" thing?\n>    how would I identify this workspace among the ones that share the\n>    same \"common\" thing?\n> \n>  - is this ref (or ref-like thing) supposed to be in common or per\n>    workspace?\n> \n> I agree with you that there needs an intermediate level, between\n> what Duy and David's git_path() does (i.e. which gives the final\n> result of deciding where in the filesystem the thing should go) and\n> your two stupid functions would do (i.e. knowing which kind the\n> thing is, give you the final location in the filesystem).  That is,\n> to let the caller know if the thing is supposed to be shared or per\n> workspace in the first place.\n> \n> With that intermediate level function, a database-based ref-backend\n> could make ('common', ref/heads/master) as the key for the current\n> value of the master branch and (workspace-name, HEAD) as the key for\n> the current value of the HEAD for a given workspace.\n> \n> > We should have two *stupid* functions, `git_workspace_path()` and\n> > `git_common_path()`, and have the *callers* decide which one to call.\n> \n> So I think we should have *three* functions:\n> \n>  - git_workspace_name(void) returns some name that uniquely\n>    identifies the current workspace among the workspaces linked to\n>    the same repository.\n\nRandom side note: the present workspace path name component is not\nacceptable for this if alternate ref backends use a single db for\nstorage across all workspaces.  That's because you might create a\nworkspace at foo, then manually rm -r it, and then create a new one also\nnamed foo.  The database wouldn't know about this series of events, and\nwould then have stale per-workspace refs for foo.\n\nThat said, with my lmdb backend, I've been falling back to the files\nbackend for per-workspace refs.  This also means I don't have to worry\nabout expiring per-workspace refs when a workspace is removed. \n\nI could change this, but IIRC, there are a fair number of things that\ncare about the existence of a file called HEAD, so the fallback was\neasier.  (That is, the other way was a giant hassle).\n\n>  - is_common_thing(const char *) takes a path (that is relative to\n>    $GIT_DIR where the thing would have lived at in a pre-workspace\n>    world), and tells if it is a common thing or a per-workspace\n>    thing.\n> \n>  - git_path() can stay the external interface and can be thought of:\n> \n> \tgit_path(const char *path)\n>         {\n> \t\tif (is_common_thing(path))\n> \t\t\treturn git_common_path(path);\n> \t\treturn git_workspace_path(git_workspace_name(), path);\n> \t}\n> \n>    if you think in terms of your two helpers.\n> \n> But I do not think that git_common_path() and git_workspace_path()\n> need to be called from any other place in the system, and that is\n> the reason why I did not say we should have four functions (or five,\n> counting git_path() itself).\n\nI wrote an email arguing for Michael's position on this, and by the time\nI was done writing it, I had come around to more-or-less Junio's\nposition.\n\nMy argument for keeping git_path as the external interface is this: in\nthe multiple worktree world, $GIT_DIR is effectively an overlay\nfilesystem.  It's not a complete overlay API: e.g. we don't have a\ngit_path_opendir function that special-cases refs/ to add in\nrefs/worktree.  But it handles the common cases.\n\nIt is true that even if we had a complete overlay API, it would not be\nsufficient to hide all of the complexity of per-worktree refs (the files\nref backend still needs to know not to pack per-worktree refs).  But\nthat is equally true if we have the refs code call\ngit_common_path/git_worktree_path.  So we're not successfully hiding\ndetails by using git_common_path/git_worktree_path in refs.c.\n\nFor this patch series, I don't think we need to change anything (except\nthat I realized that I forgot to add logs/refs/worktree to refs, and\npeople will probably find some issues once they start reviewing the\ndetails of my code). In the present code, adjust_git_path already\nhandles the git_common_path vs git_workspace_path thing; it just does it\nin a slightly less elegant way than Junio's proposal.  Implementing\nJunio's proposal would not affect this series; it would just be an\nadditional patch on top (or beforehand).\n\nThere is one case where the refs code will probably need to directly\ncall git_workspace_path: when we fix git prune to handle detached HEADs\nin workspaces.  It could just set the workspace and then call git_path,\nbut that is less elegant.  So I think when we fix that (which should\nprobably wait on for_each_worktree), we can implement Junio's proposal.\n"},{"id":"268074","messageId":"xmqq1tf5ipju.fsf@gitster.dls.corp.google.com","threadId":"40076","inReplyTo":"1439582644.8855.89.camel@twopensource.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-14T20:27:17Z","receivedAt":"2015-08-14T20:27:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> Random side note: the present workspace path name component is not\n> acceptable for this if alternate ref backends use a single db for\n> storage across all workspaces.  That's because you might create a\n> workspace at foo, then manually rm -r it, and then create a new one also\n> named foo.  The database wouldn't know about this series of events, and\n> would then have stale per-workspace refs for foo.\n\nThe users can do \"Create, manuallly rm -r and recreate\" dance all\nthey want, but the result must still honor the invariant:\n\n    For any $workspace, $workspace/.git is a \"gitdir:\" file that\n    points at one subdirectory in $GIT_COMMON_DIR/worktrees/.\n\nThe \"name\" I had in mind was the names of the directories in\n$GIT_COMMON_DIR/worktrees/ that by definition has to be unique.\n\nAnother invariant \n\n    $GIT_COMMON_DIR/worktrees/$that_subdirectory has commondir file\n    that points at the $GIT_COMMON_DIR/.\n\nmust also be preserved by \"Create, manuallly rm -r and recreate\"\ndance, but it is not important to define what the workspace ID is.\n\n> That said, with my lmdb backend, I've been falling back to the files\n> backend for per-workspace refs.\n\nI think that is perfectly fine and within the 3-bullet design\nguideline we saw earlier.  Your backend is honoring the decision\nmade by the system as a whole what is private and what is shared,\nwhich is the only thing that counts in the larger picture.  It is up\nto individual ref-backend implementations how they do so, and it is\nperfectly fine that your backend takes advantage of the ref layout\nby storing some stuff in lmdb storage (which I presume will live in\n\"common\" part of the filesystem storage) and some other stuff\ndirectly in the filesystem.\n\n> There is one case where the refs code will probably need to directly\n> call git_workspace_path: when we fix git prune to handle detached HEADs\n> in workspaces.  It could just set the workspace and then call git_path,\n> but that is less elegant.  So I think when we fix that (which should\n> probably wait on for_each_worktree), we can implement Junio's proposal.\n\nYeah, I said *three* helper functions, but we do need the \"enumerate\nall workspaces\" if we want to go that route, and we do need to\nexpose Michael's common_path() and workspace_path() to the code that\nneeds such an enumeration.\n"},{"id":"268084","messageId":"1439585660.8855.102.camel@twopensource.com","threadId":"40076","inReplyTo":"xmqq1tf5ipju.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-14T20:54:20Z","receivedAt":"2015-08-14T20:54:20Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2015-08-14 at 13:27 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> > Random side note: the present workspace path name component is not\n> > acceptable for this if alternate ref backends use a single db for\n> > storage across all workspaces.  That's because you might create a\n> > workspace at foo, then manually rm -r it, and then create a new one also\n> > named foo.  The database wouldn't know about this series of events, and\n> > would then have stale per-workspace refs for foo.\n> \n> The users can do \"Create, manuallly rm -r and recreate\" dance all\n> they want, but the result must still honor the invariant:\n> \n>     For any $workspace, $workspace/.git is a \"gitdir:\" file that\n>     points at one subdirectory in $GIT_COMMON_DIR/worktrees/.\n> \n> The \"name\" I had in mind was the names of the directories in\n> $GIT_COMMON_DIR/worktrees/ that by definition has to be unique.\n> \n> Another invariant \n>\n>    $GIT_COMMON_DIR/worktrees/$that_subdirectory has commondir file\n>    that points at the $GIT_COMMON_DIR/.\n>\n> must also be preserved by \"Create, manuallly rm -r and recreate\"\n> dance, but it is not important to define what the workspace ID is.\n\nMy worry was that workspace would get deleted and\nrecreated without the refs db finding out.  Then the refs db would\nretain per-workspace references from the deleted version of the\nworskace.  That is, these names are unique at any given time, but not\nacross time.\n\nMy idea was just to have `git workspace add` create per-workspace\n\"workspace_id\" file with a long random number in it. The id would be\n$that_subdirectory/$random_number; we would know to split it at / and\ndouble-check that the workspace id is still current before assuming that\nour refs data was valid for the workspace.  \n\nAlternately, git workspace add could inform the refs backend that a\nworkspace is being added, which would solve the problem in a different\nway.\n\nBut given my refs lmdb backend design, I can actually ignore this for\nnow, so that's what I'm going to do.\n"},{"id":"268113","messageId":"CACsJy8CE8OA7AvT+BTPodRokX7qxAOkbY1UmR4qbtE8rUSoTqQ@mail.gmail.com","threadId":"40076","inReplyTo":"1439416645-19173-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 1/4] refs: clean up common_list","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-15T07:44:13Z","receivedAt":"2015-08-15T07:44:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 13, 2015 at 4:57 AM, David Turner <dturner@twopensource.com> wrote:\n> +struct common_dir common_list[] = {\n> +       { \"branches\", 0, 1, 0 },\n> +       { \"hooks\", 0, 1, 0 },\n> +       { \"info\", 0, 1, 0 },\n> +       { \"info/sparse-checkout\", 0, 0, 1 },\n> +       { \"logs\", 1, 1, 0 },\n> +       { \"logs/HEAD\", 1, 1, 1 },\n> +       { \"lost-found\", 0, 1, 0 },\n> +       { \"objects\", 0, 1, 0 },\n> +       { \"refs\", 0, 1, 0 },\n> +       { \"remotes\", 0, 1, 0 },\n> +       { \"worktrees\", 0, 1, 0 },\n> +       { \"rr-cache\", 0, 1, 0 },\n> +       { \"svn\", 0, 1, 0 },\n> +       { \"config\", 0, 0, 0 },\n> +       { \"gc.pid\", 1, 0, 0 },\n> +       { \"packed-refs\", 0, 0, 0 },\n> +       { \"shallow\", 0, 0, 0 },\n> +       { NULL, 0, 0, 0 }\n>  };\n\nNit. If you make dirname the last field, we would have aligned\nnumbers, which might help reading.\n-- \nDuy\n"},{"id":"268114","messageId":"CACsJy8BCr7StbtcrgsbTiosTX1RkjwwWyRqddz2XDhFn5R+zAw@mail.gmail.com","threadId":"40076","inReplyTo":"1439416645-19173-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-15T07:59:16Z","receivedAt":"2015-08-15T07:59:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 13, 2015 at 4:57 AM, David Turner <dturner@twopensource.com> wrote:\n> Instead of a linear search over common_list to check whether\n> a path is common, use a trie.  The trie search operates on\n> path prefixes, and handles excludes.\n\nJust be careful that the given key from git_path is not normalized. I\nthink you assume it is in the code, but I haven't read carefully. We\ncould of course optimize for the good case: assume normalized and\nsearch, then fall back to explicit normalizing and search again.\n-- \nDuy\n"},{"id":"268115","messageId":"CACsJy8DSD2orHh0EqfF249fR_dgdZyABhgHubFW=LZ4VEpz+7w@mail.gmail.com","threadId":"40076","inReplyTo":"1439416645-19173-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 3/4] refs: make refs/worktree/* per-worktree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-15T08:04:11Z","receivedAt":"2015-08-15T08:04:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 13, 2015 at 4:57 AM, David Turner <dturner@twopensource.com> wrote:\n> We need a place to stick refs for bisects in progress that is not\n> shared between worktrees.  So we use the refs/worktree/ hierarchy.\n>\n> The is_per_worktree_ref function and associated docs learn that\n> refs/worktree/ is per-worktree, as does the git_path code in path.c\n\nI assume you want to iterate over all these per-worktree refs later\non. Otherwise just moving bisect refs outside refs/ would\nautomatically make them per-worktree. But then it would polute\n$GIT_DIR some more, so probably not the best move. So yeah,\nrefs/worktree is probably for the best.\n-- \nDuy\n"},{"id":"268139","messageId":"55CF811A.8060106@alum.mit.edu","threadId":"40076","inReplyTo":"xmqqtws1iyxn.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-08-15T18:12:42Z","receivedAt":"2015-08-15T18:12:42Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/14/2015 07:04 PM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> Let's take a step back.\n>>\n>> We have always had a ton of code that uses `git_path()` and friends to\n>> convert abstract things into filesystem paths. Let's take the\n>> reference-handling code as an example:\n>> ...\n>> This seems crazy to me. It is the *reference* code that should know\n>> whether a particular reference should be stored under `$GIT_DIR` or\n>> `$GIT_COMMON_DIR`, or indeed whether it should be stored in a database.\n> \n> It is more like:\n> \n>  1. The system as a whole should decide if HEAD and refs/heads/\n>     should be per workspace or shared across a repository (and we say\n>     the former should be per workspace, the latter should be shared).\n> \n>  2. The reference code should decide which ref-backend is used to\n>     store refs.\n> \n>  3. And any ref-backend should follow the decision made by the\n>     system as a whole in 1.\n\nIf I understand correctly, you consider the decision of where a\nparticular reference should be stored to be a kind of \"business logic\"\ndecision that should live outside of the refs module. I don't think it\nis so important whether this knowledge is inside or outside of the refs\nmodule (I can live with it either way).\n\n> I'd imagine that David's ref-backend code inherited from Ronnie\n> would still accept the string \"refs/heads/master\" from the rest of\n> the system (i.e. callers that call into the ref API) to mean \"the\n> ref that represents the 'master' branch\", and uses that as the key\n> to decide \"ok, that is shared across workspaces\" to honor the\n> system-wide decision made in 1.  The outside callers wouldn't pass\n> the result of calling git_path(\"refs/heads/master\") into the ref\n> API, which may expand to \"$somewhere_else/refs/heads/master\" when\n> run in a secondary workspace to point at the common location.\n\nDefinitely agreed.\n\n> I'd also imagine that the workspace API would give ways for the\n> implementation of the reference API to ask these questions:\n> \n>  - which workspace am I operating for?  where is the \"common\" thing?\n>    how would I identify this workspace among the ones that share the\n>    same \"common\" thing?\n> \n>  - is this ref (or ref-like thing) supposed to be in common or per\n>    workspace?\n\nYes, I especially like this last idea. For example, suppose the function\nis \"is_common_reference(refname)\". It's nice that this function doesn't\nhave to know about all possible \"things\" like your is_common_thing()\nfunction. Therefore it can be simpler. Almost always the caller (and in\nthis case the caller would usually be within the refs module) will know\nthat the \"thing\" it wants to inquire about is a reference name, so it\ncan spare the extra expense of calling is_common_thing().\n\nIf we still need is_common_thing() (e.g., for the implementation of `git\nrev-parse --git-path`), its definition would become something like\n\n    return is_common_reference(thing) ||\n           is_common_file(thing) ||\n           is_common_flurg(thing) || ...;\n\nIn this construction I think it is is clear that is_common_reference()\nwould fit pretty well in the refs module, though elsewhere would be OK too.\n\n> [...]\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"268140","messageId":"55CF82DB.8010204@alum.mit.edu","threadId":"40076","inReplyTo":"1439582644.8855.89.camel@twopensource.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-08-15T18:20:11Z","receivedAt":"2015-08-15T18:20:11Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/14/2015 10:04 PM, David Turner wrote:\n> On Fri, 2015-08-14 at 10:04 -0700, Junio C Hamano wrote:\n>> [...]\n>> So I think we should have *three* functions:\n>>\n>>  - git_workspace_name(void) returns some name that uniquely\n>>    identifies the current workspace among the workspaces linked to\n>>    the same repository.\n> \n> Random side note: the present workspace path name component is not\n> acceptable for this if alternate ref backends use a single db for\n> storage across all workspaces.  That's because you might create a\n> workspace at foo, then manually rm -r it, and then create a new one also\n> named foo.  The database wouldn't know about this series of events, and\n> would then have stale per-workspace refs for foo.\n\nThe reference backend should clearly have functions that are called on\nthe creation and destruction of a workspace. In these functions the\nbackend could initialize / clean up any reference-related resources\nassociated with that workspace.\n\nThese functions can be empty for the filesystem backend.\n\n> That said, with my lmdb backend, I've been falling back to the files\n> backend for per-workspace refs.  This also means I don't have to worry\n> about expiring per-workspace refs when a workspace is removed. \n> \n> I could change this, but IIRC, there are a fair number of things that\n> care about the existence of a file called HEAD, so the fallback was\n> easier.  (That is, the other way was a giant hassle).\n\nOK, so the functions can be empty for the lmdb backend, too :-)\n\n> [...]\n> For this patch series, I don't think we need to change anything [...]\n> Implementing\n> Junio's proposal would not affect this series; it would just be an\n> additional patch on top (or beforehand).\n\nOK.\n\n> [...]\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"268142","messageId":"loom.20150816T070037-969@post.gmane.org","threadId":"40076","inReplyTo":"CACsJy8BCr7StbtcrgsbTiosTX1RkjwwWyRqddz2XDhFn5R+zAw@mail.gmail.com","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-16T05:04:22Z","receivedAt":"2015-08-16T05:04:22Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n> On Thu, Aug 13, 2015 at 4:57 AM, David Turner <dturner@twopensource.com> \nwrote:\n> > Instead of a linear search over common_list to check whether\n> > a path is common, use a trie.  The trie search operates on\n> > path prefixes, and handles excludes.\n> \n> Just be careful that the given key from git_path is not normalized. I\n> think you assume it is in the code, but I haven't read carefully. We\n> could of course optimize for the good case: assume normalized and\n> search, then fall back to explicit normalizing and search again.\n\nWhat does it mean for a key to be normalized?  Do you mean normalized in \nterms of upper/lowercase on case-insensitive filesystems?  If so, I think the \nassumption here is that this will be called with paths generated by git, \nwhich will always use the lowercase path.\n"},{"id":"268152","messageId":"CACsJy8B5X=JH5p6C=Ho-spLJEcOtQd8e95ED63FLp=gxrk2srQ@mail.gmail.com","threadId":"40076","inReplyTo":"loom.20150816T070037-969@post.gmane.org","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-16T12:20:23Z","receivedAt":"2015-08-16T12:20:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Aug 16, 2015 at 12:04 PM, David Turner <dturner@twopensource.com> wrote:\n> Duy Nguyen <pclouds@gmail.com> writes:\n>> On Thu, Aug 13, 2015 at 4:57 AM, David Turner <dturner@twopensource.com>\n> wrote:\n>> > Instead of a linear search over common_list to check whether\n>> > a path is common, use a trie.  The trie search operates on\n>> > path prefixes, and handles excludes.\n>>\n>> Just be careful that the given key from git_path is not normalized. I\n>> think you assume it is in the code, but I haven't read carefully. We\n>> could of course optimize for the good case: assume normalized and\n>> search, then fall back to explicit normalizing and search again.\n>\n> What does it mean for a key to be normalized?  Do you mean normalized in\n> terms of upper/lowercase on case-insensitive filesystems?  If so, I think the\n> assumption here is that this will be called with paths generated by git,\n> which will always use the lowercase path.\n\nMostly about duplicated slashes, \"abc//def\" instead of \"abc/def\".\nTechnically nothing forbids git_path(\"refs/../refs/foo\"), but that\nwould fool even current code.\n-- \nDuy\n"},{"id":"268188","messageId":"xmqqegj1gb9h.fsf@gitster.dls.corp.google.com","threadId":"40076","inReplyTo":"55CF811A.8060106@alum.mit.edu","subject":"Re: [PATCH v3 2/4] path: optimize common dir checking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-17T15:55:38Z","receivedAt":"2015-08-17T15:55:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> If I understand correctly, you consider the decision of where a\n> particular reference should be stored to be a kind of \"business logic\"\n> decision that should live outside of the refs module. I don't think it\n> is so important whether this knowledge is inside or outside of the refs\n> module (I can live with it either way).\n\nI think I misspoke.  The decision to make a particular reference,\ne.g. HEAD or 'master' branch, a per-workspace one or a repo-wide\none, should not be made by individual refs backend (i.e. lower-half\nof the refs module).  It could still be the responsibility of the\nupper-half of the refs module and that _feels_ more kosher.\n\nBut without actual implementation of the interface between upper-\nand lower-half of the refs module yet (as we only have fs based\nbackend that is tightly integrated within the refs module and\nnothing else right now), I do not yet see a clear implementation for\nthe \"is this thing common?\" table Duy did that decides where things\ngo for everything except \"refs/\" part while letting the upper-half\nof the refs module to take responsiblity of that decision for refs.\nFor one thing, \"the refs module decides what is in refs/, Duy's\ntable decides everything else \" is not even the right partition, in\nthe presence of things like HEAD, logs/, etc.\n\nSo...\n"}]}