{"thread":{"id":"18966","subject":"[PATCH 0/4] Revisiting cache-tree","startedAt":"2009-04-20T10:58:16Z","lastAt":"2009-04-20T13:08:27Z","messageCount":6,"participants":["Junio C Hamano","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"111714","messageId":"1240225100-29960-1-git-send-email-gitster@pobox.com","threadId":"18966","inReplyTo":null,"subject":"[PATCH 0/4] Revisiting cache-tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-20T10:58:16Z","receivedAt":"2009-04-20T10:58:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This series consists of a fix meant for maint branch, and two performance\nfix.  The second one is a refactoring of the code to support the latter.\n\nI was doing a rather huge import as a multi-step process by doing\nsomething like this:\n\n\t$ rm -f $GIT_DIR/index\n\t$ git add some ;# this one is huge\n\t$ git tag one $(git write-tree)\n\t$ git repack -a -d\n\t$ rm -f $GIT_DIR/index\n\t$ git add other ;# this one is also huge\n\t$ git tag two $(git write-tree)\n\t$ git repack -a -d\n\n\t$ git read-tree some other\n\t$ git tag both $(git write-tree)\n\t$ git repack -a -d\n\nThe binding of two distinct subtrees is done with the read-tree but it\nwrote out an incorrect index (notice the lack of -m; with -m option the\ncommand correctly does a different thing) and resulted in a corrupt tree\nobject.\n\nJunio C Hamano (4):\n  read-tree A B: do not corrupt cache-tree\n  Move prime_cache_tree() to cache-tree.c\n  read-tree -m A B: prime cache-tree from the switched-to tree\n  checkout branch: prime cache-tree fully\n\n builtin-checkout.c  |   10 +++++++++-\n builtin-read-tree.c |   48 ++++++++----------------------------------------\n cache-tree.c        |   34 ++++++++++++++++++++++++++++++++++\n cache-tree.h        |    4 ++++\n 4 files changed, 55 insertions(+), 41 deletions(-)\n"},{"id":"111715","messageId":"1240225100-29960-2-git-send-email-gitster@pobox.com","threadId":"18966","inReplyTo":"1240225100-29960-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/4] read-tree A B: do not corrupt cache-tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-20T10:58:17Z","receivedAt":"2009-04-20T10:58:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"An earlier commit aab3b9a (read-tree A B C: do not create a bogus index\nand do not segfault, 2009-03-12) resurrected the support for an obscure\n(but useful) feature to read and overlay more than one tree into the index\nwithout the -m (merge) option.  But the fix was not enough.\n\nExercising this feature exposes a longstanding bug in the code that primes\nthe cache-tree in the index from the tree that was read.  The intention\nwas that when we know that the index must exactly match the tree we just\nread, we prime the entire cache-tree with it.\n\nHowever, the logic to detect that case incorrectly triggered if you read\ntwo trees without -m.  This resulted in a corrupted cache-tree, and\nwrite-tree would have produced an incorrect tree object out of such an\nindex.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * Meant for maint.\n\n builtin-read-tree.c |    6 ++----\n 1 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 38fef34..e4e0e71 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -211,7 +211,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\tcase 3:\n \t\tdefault:\n \t\t\topts.fn = threeway_merge;\n-\t\t\tcache_tree_free(&active_cache_tree);\n \t\t\tbreak;\n \t\t}\n \n@@ -221,6 +220,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t\topts.head_idx = 1;\n \t}\n \n+\tcache_tree_free(&active_cache_tree);\n \tfor (i = 0; i < nr_trees; i++) {\n \t\tstruct tree *tree = trees[i];\n \t\tparse_tree(tree);\n@@ -235,10 +235,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t * valid cache-tree because the index must match exactly\n \t * what came from the tree.\n \t */\n-\tif (nr_trees && !opts.prefix && (!opts.merge || (stage == 2))) {\n-\t\tcache_tree_free(&active_cache_tree);\n+\tif (nr_trees == 1 && !opts.prefix)\n \t\tprime_cache_tree();\n-\t}\n \n \tif (write_cache(newfd, active_cache, active_nr) ||\n \t    commit_locked_index(&lock_file))\n-- \n1.6.3.rc1.18.g66996\n"},{"id":"111716","messageId":"1240225100-29960-3-git-send-email-gitster@pobox.com","threadId":"18966","inReplyTo":"1240225100-29960-2-git-send-email-gitster@pobox.com","subject":"[PATCH 2/4] Move prime_cache_tree() to cache-tree.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-20T10:58:18Z","receivedAt":"2009-04-20T10:58:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The interface to build cache-tree belongs there.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-checkout.c  |    1 +\n builtin-read-tree.c |   37 +------------------------------------\n cache-tree.c        |   34 ++++++++++++++++++++++++++++++++++\n cache-tree.h        |    4 ++++\n 4 files changed, 40 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex b121fe5..ffdb33a 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -5,6 +5,7 @@\n #include \"commit.h\"\n #include \"tree.h\"\n #include \"tree-walk.h\"\n+#include \"cache-tree.h\"\n #include \"unpack-trees.h\"\n #include \"dir.h\"\n #include \"run-command.h\"\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex e4e0e71..9cd7d07 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -29,41 +29,6 @@ static int list_tree(unsigned char *sha1)\n \treturn 0;\n }\n \n-static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)\n-{\n-\tstruct tree_desc desc;\n-\tstruct name_entry entry;\n-\tint cnt;\n-\n-\thashcpy(it->sha1, tree->object.sha1);\n-\tinit_tree_desc(&desc, tree->buffer, tree->size);\n-\tcnt = 0;\n-\twhile (tree_entry(&desc, &entry)) {\n-\t\tif (!S_ISDIR(entry.mode))\n-\t\t\tcnt++;\n-\t\telse {\n-\t\t\tstruct cache_tree_sub *sub;\n-\t\t\tstruct tree *subtree = lookup_tree(entry.sha1);\n-\t\t\tif (!subtree->object.parsed)\n-\t\t\t\tparse_tree(subtree);\n-\t\t\tsub = cache_tree_sub(it, entry.path);\n-\t\t\tsub->cache_tree = cache_tree();\n-\t\t\tprime_cache_tree_rec(sub->cache_tree, subtree);\n-\t\t\tcnt += sub->cache_tree->entry_count;\n-\t\t}\n-\t}\n-\tit->entry_count = cnt;\n-}\n-\n-static void prime_cache_tree(void)\n-{\n-\tif (!nr_trees)\n-\t\treturn;\n-\tactive_cache_tree = cache_tree();\n-\tprime_cache_tree_rec(active_cache_tree, trees[0]);\n-\n-}\n-\n static const char read_tree_usage[] = \"git read-tree (<sha> | [[-m [--trivial] [--aggressive] | --reset | --prefix=<prefix>] [-u | -i]] [--exclude-per-directory=<gitignore>] [--index-output=<file>] <sha1> [<sha2> [<sha3>]])\";\n \n static struct lock_file lock_file;\n@@ -236,7 +201,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t * what came from the tree.\n \t */\n \tif (nr_trees == 1 && !opts.prefix)\n-\t\tprime_cache_tree();\n+\t\tprime_cache_tree(&active_cache_tree, trees[0]);\n \n \tif (write_cache(newfd, active_cache, active_nr) ||\n \t    commit_locked_index(&lock_file))\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 3d8f218..37bf35e 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"tree.h\"\n+#include \"tree-walk.h\"\n #include \"cache-tree.h\"\n \n #ifndef DEBUG\n@@ -591,3 +592,36 @@ int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix)\n \n \treturn 0;\n }\n+\n+static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)\n+{\n+\tstruct tree_desc desc;\n+\tstruct name_entry entry;\n+\tint cnt;\n+\n+\thashcpy(it->sha1, tree->object.sha1);\n+\tinit_tree_desc(&desc, tree->buffer, tree->size);\n+\tcnt = 0;\n+\twhile (tree_entry(&desc, &entry)) {\n+\t\tif (!S_ISDIR(entry.mode))\n+\t\t\tcnt++;\n+\t\telse {\n+\t\t\tstruct cache_tree_sub *sub;\n+\t\t\tstruct tree *subtree = lookup_tree(entry.sha1);\n+\t\t\tif (!subtree->object.parsed)\n+\t\t\t\tparse_tree(subtree);\n+\t\t\tsub = cache_tree_sub(it, entry.path);\n+\t\t\tsub->cache_tree = cache_tree();\n+\t\t\tprime_cache_tree_rec(sub->cache_tree, subtree);\n+\t\t\tcnt += sub->cache_tree->entry_count;\n+\t\t}\n+\t}\n+\tit->entry_count = cnt;\n+}\n+\n+void prime_cache_tree(struct cache_tree **it, struct tree *tree)\n+{\n+\tcache_tree_free(it);\n+\t*it = cache_tree();\n+\tprime_cache_tree_rec(*it, tree);\n+}\ndiff --git a/cache-tree.h b/cache-tree.h\nindex cf8b790..e958835 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -1,6 +1,8 @@\n #ifndef CACHE_TREE_H\n #define CACHE_TREE_H\n \n+#include \"tree.h\"\n+\n struct cache_tree;\n struct cache_tree_sub {\n \tstruct cache_tree *cache_tree;\n@@ -33,4 +35,6 @@ int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int)\n #define WRITE_TREE_PREFIX_ERROR (-3)\n \n int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix);\n+void prime_cache_tree(struct cache_tree **, struct tree *);\n+\n #endif\n-- \n1.6.3.rc1.18.g66996\n"},{"id":"111718","messageId":"1240225100-29960-4-git-send-email-gitster@pobox.com","threadId":"18966","inReplyTo":"1240225100-29960-3-git-send-email-gitster@pobox.com","subject":"[PATCH 3/4] read-tree -m A B: prime cache-tree from the switched-to tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-20T10:58:19Z","receivedAt":"2009-04-20T10:58:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When switching to a new branch with \"read-tree -m A B\", the resulting\nindex must match tree B and we can prime the cache tree with it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-read-tree.c |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 9cd7d07..391d709 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -199,9 +199,14 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t * \"-m ent\" or \"--reset ent\" form), we can obtain a fully\n \t * valid cache-tree because the index must match exactly\n \t * what came from the tree.\n+\t *\n+\t * The same holds true if we are switching between two trees\n+\t * using read-tree -m A B.  The index must match B after that.\n \t */\n \tif (nr_trees == 1 && !opts.prefix)\n \t\tprime_cache_tree(&active_cache_tree, trees[0]);\n+\telse if (nr_trees == 2 && opts.merge)\n+\t\tprime_cache_tree(&active_cache_tree, trees[1]);\n \n \tif (write_cache(newfd, active_cache, active_nr) ||\n \t    commit_locked_index(&lock_file))\n-- \n1.6.3.rc1.18.g66996\n"},{"id":"111717","messageId":"1240225100-29960-5-git-send-email-gitster@pobox.com","threadId":"18966","inReplyTo":"1240225100-29960-4-git-send-email-gitster@pobox.com","subject":"[PATCH 4/4] checkout branch: prime cache-tree fully","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-20T10:58:20Z","receivedAt":"2009-04-20T10:58:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When switching to another branch, the earlier code relied on incremental\ninvalidation of cache-tree entries to degrade it.  While it is not wrong\nper-se, we know that the resulting index must fully match the branch we\nare switching to unless the -m (merge) option is used.\n\nWe should simply fully re-prime the cache-tree using the new tree object\nin such a case.  And for safety, invalidate the cache-tree as a whole in\nother cases.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-checkout.c |    9 ++++++++-\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex ffdb33a..efa1ebf 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -368,14 +368,17 @@ static int merge_working_tree(struct checkout_opts *opts,\n \tint ret;\n \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n \tint newfd = hold_locked_index(lock_file, 1);\n+\tint reprime_cache_tree = 0;\n \n \tif (read_cache() < 0)\n \t\treturn error(\"corrupt index file\");\n \n+\tcache_tree_free(&active_cache_tree);\n \tif (opts->force) {\n \t\tret = reset_tree(new->commit->tree, opts, 1);\n \t\tif (ret)\n \t\t\treturn ret;\n+\t\treprime_cache_tree = 1;\n \t} else {\n \t\tstruct tree_desc trees[2];\n \t\tstruct tree *tree;\n@@ -411,7 +414,9 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\tinit_tree_desc(&trees[1], tree->buffer, tree->size);\n \n \t\tret = unpack_trees(2, trees, &topts);\n-\t\tif (ret == -1) {\n+\t\tif (ret != -1) {\n+\t\t\treprime_cache_tree = 1;\n+\t\t} else {\n \t\t\t/*\n \t\t\t * Unpack couldn't do a trivial merge; either\n \t\t\t * give up or do a real merge, depending on\n@@ -455,6 +460,8 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\t}\n \t}\n \n+\tif (reprime_cache_tree)\n+\t\tprime_cache_tree(&active_cache_tree, new->commit->tree);\n \tif (write_cache(newfd, active_cache, active_nr) ||\n \t    commit_locked_index(lock_file))\n \t\tdie(\"unable to write new index file\");\n-- \n1.6.3.rc1.18.g66996\n"},{"id":"111728","messageId":"81b0412b0904200608r7b986160p15ec0afb826ee1f6@mail.gmail.com","threadId":"18966","inReplyTo":"1240225100-29960-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/4] Revisiting cache-tree","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-04-20T13:08:27Z","receivedAt":"2009-04-20T13:08:27Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/4/20 Junio C Hamano <gitster@pobox.com>:\n> This series consists of a fix meant for maint branch, and two performance\n> fix.  The second one is a refactoring of the code to support the latter.\n>\n> I was doing a rather huge import as a multi-step process by doing\n> something like this:\n>\n>        $ rm -f $GIT_DIR/index\n>        $ git add some ;# this one is huge\n>        $ git tag one $(git write-tree)\n>        $ git repack -a -d\n>        $ rm -f $GIT_DIR/index\n>        $ git add other ;# this one is also huge\n>        $ git tag two $(git write-tree)\n>        $ git repack -a -d\n>\n>        $ git read-tree some other\n\n\"git read-tree one two\"\n"}]}