{"thread":{"id":"37064","subject":"[PATCH v4 1/4] cache-tree: Create/update cache-tree on checkout","startedAt":"2014-07-06T04:06:56Z","lastAt":"2014-07-09T01:58:31Z","messageCount":15,"participants":["David Turner","Eric Sunshine","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":4,"patchTotal":4},"messages":[{"id":"245432","messageId":"1404619619-4774-1-git-send-email-dturner@twitter.com","threadId":"37064","inReplyTo":null,"subject":"[PATCH v4 1/4] cache-tree: Create/update cache-tree on checkout","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-07-06T04:06:56Z","receivedAt":"2014-07-06T04:06:56Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"When git checkout checks out a branch, create or update the\ncache-tree so that subsequent operations are faster.\n\nupdate_main_cache_tree learned a new flag, WRITE_TREE_REPAIR.  When\nWRITE_TREE_REPAIR is set, portions of the cache-tree which do not\ncorrespond to existing tree objects are invalidated (and portions which\ndo are marked as valid).  No new tree objects are created.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n builtin/checkout.c    |  8 ++++++++\n cache-tree.c          | 12 +++++++++++-\n cache-tree.h          |  1 +\n t/t0090-cache-tree.sh | 19 ++++++++++++++++---\n 4 files changed, 36 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 07cf555..054214f 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -553,6 +553,14 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\t}\n \t}\n \n+\tif (!active_cache_tree)\n+\t\tactive_cache_tree = cache_tree();\n+\n+\tif (!cache_tree_fully_valid(active_cache_tree))\n+\t\tcache_tree_update(active_cache_tree,\n+\t\t\t\t  (const struct cache_entry * const *)active_cache,\n+\t\t\t\t  active_nr, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);\n+\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\"));\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 7fa524a..f951d7d 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -239,9 +239,12 @@ static int update_one(struct cache_tree *it,\n \tstruct strbuf buffer;\n \tint missing_ok = flags & WRITE_TREE_MISSING_OK;\n \tint dryrun = flags & WRITE_TREE_DRY_RUN;\n+\tint repair = flags & WRITE_TREE_REPAIR;\n \tint to_invalidate = 0;\n \tint i;\n \n+\tassert(!(dryrun && repair));\n+\n \t*skip_count = 0;\n \n \tif (0 <= it->entry_count && has_sha1_file(it->sha1))\n@@ -374,7 +377,14 @@ static int update_one(struct cache_tree *it,\n #endif\n \t}\n \n-\tif (dryrun)\n+\tif (repair) {\n+\t\tunsigned char sha1[20];\n+\t\thash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);\n+\t\tif (has_sha1_file(sha1))\n+\t\t\thashcpy(it->sha1, sha1);\n+\t\telse\n+\t\t\tto_invalidate = 1;\n+\t} else if (dryrun)\n \t\thash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);\n \telse if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) {\n \t\tstrbuf_release(&buffer);\ndiff --git a/cache-tree.h b/cache-tree.h\nindex f1923ad..666d18f 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -39,6 +39,7 @@ int update_main_cache_tree(int);\n #define WRITE_TREE_IGNORE_CACHE_TREE 2\n #define WRITE_TREE_DRY_RUN 4\n #define WRITE_TREE_SILENT 8\n+#define WRITE_TREE_REPAIR 16\n \n /* error return codes */\n #define WRITE_TREE_UNREADABLE_INDEX (-1)\ndiff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\nindex 6c33e28..98fb1ab 100755\n--- a/t/t0090-cache-tree.sh\n+++ b/t/t0090-cache-tree.sh\n@@ -44,14 +44,14 @@ test_expect_success 'read-tree HEAD establishes cache-tree' '\n \n test_expect_success 'git-add invalidates cache-tree' '\n \ttest_when_finished \"git reset --hard; git read-tree HEAD\" &&\n-\techo \"I changed this file\" > foo &&\n+\techo \"I changed this file\" >foo &&\n \tgit add foo &&\n \ttest_invalid_cache_tree\n '\n \n test_expect_success 'update-index invalidates cache-tree' '\n \ttest_when_finished \"git reset --hard; git read-tree HEAD\" &&\n-\techo \"I changed this file\" > foo &&\n+\techo \"I changed this file\" >foo &&\n \tgit update-index --add foo &&\n \ttest_invalid_cache_tree\n '\n@@ -85,9 +85,22 @@ test_expect_success 'reset --hard without index gives cache-tree' '\n \ttest_shallow_cache_tree\n '\n \n-test_expect_failure 'checkout gives cache-tree' '\n+test_expect_success 'checkout gives cache-tree' '\n+\tgit tag current &&\n \tgit checkout HEAD^ &&\n \ttest_shallow_cache_tree\n '\n \n+test_expect_success 'checkout -b gives cache-tree' '\n+\tgit checkout current &&\n+\tgit checkout -b prev HEAD^ &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_expect_success 'checkout -B gives cache-tree' '\n+\tgit checkout current &&\n+\tgit checkout -B prev HEAD^ &&\n+\ttest_shallow_cache_tree\n+'\n+\n test_done\n-- \n2.0.0.390.gcb682f8\n"},{"id":"245433","messageId":"1404619619-4774-2-git-send-email-dturner@twitter.com","threadId":"37064","inReplyTo":"1404619619-4774-1-git-send-email-dturner@twitter.com","subject":"[PATCH v4 2/4] test-dump-cache-tree: invalid trees are not errors","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-07-06T04:06:57Z","receivedAt":"2014-07-06T04:06:57Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Do not treat known-invalid trees as errors even when their count is\nincorrect.  Because git already knows that these trees are invalid,\nnothing depends on the count field.\n\nAdd a couple of comments.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n test-dump-cache-tree.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c\nindex 47eab97..cbbbd8e 100644\n--- a/test-dump-cache-tree.c\n+++ b/test-dump-cache-tree.c\n@@ -26,16 +26,16 @@ static int dump_cache_tree(struct cache_tree *it,\n \t\treturn 0;\n \n \tif (it->entry_count < 0) {\n+\t\t/* invalid */\n \t\tdump_one(it, pfx, \"\");\n \t\tdump_one(ref, pfx, \"#(ref) \");\n-\t\tif (it->subtree_nr != ref->subtree_nr)\n-\t\t\terrs = 1;\n \t}\n \telse {\n \t\tdump_one(it, pfx, \"\");\n \t\tif (hashcmp(it->sha1, ref->sha1) ||\n \t\t    ref->entry_count != it->entry_count ||\n \t\t    ref->subtree_nr != it->subtree_nr) {\n+\t\t\t/* claims to be valid but is lying */\n \t\t\tdump_one(ref, pfx, \"#(ref) \");\n \t\t\terrs = 1;\n \t\t}\n-- \n2.0.0.390.gcb682f8\n"},{"id":"245434","messageId":"1404619619-4774-3-git-send-email-dturner@twitter.com","threadId":"37064","inReplyTo":"1404619619-4774-1-git-send-email-dturner@twitter.com","subject":"[PATCH v4 3/4] cache-tree: subdirectory tests","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-07-06T04:06:58Z","receivedAt":"2014-07-06T04:06:58Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Add tests to confirm that invalidation of subdirectories nether over-\nnor under-invalidates.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n t/t0090-cache-tree.sh | 28 +++++++++++++++++++++++++---\n 1 file changed, 25 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\nindex 98fb1ab..8437c5f 100755\n--- a/t/t0090-cache-tree.sh\n+++ b/t/t0090-cache-tree.sh\n@@ -21,10 +21,13 @@ test_shallow_cache_tree () {\n \tcmp_cache_tree expect\n }\n \n+# Test that the cache-tree for a given directory is invalid.\n+# If no directory is given, check that the root is invalid\n test_invalid_cache_tree () {\n-\techo \"invalid                                   (0 subtrees)\" >expect &&\n-\tprintf \"SHA #(ref)  (%d entries, 0 subtrees)\\n\" $(git ls-files|wc -l) >>expect &&\n-\tcmp_cache_tree expect\n+\ttest-dump-cache-tree >actual &&\n+\tsed -e \"s/$_x40/SHA/\" -e \"s/[0-9]* subtrees//g\" <actual >filtered &&\n+\texpect=$(printf \"invalid                                  $1 ()\\n\") &&\n+\tfgrep \"$expect\" filtered\n }\n \n test_no_cache_tree () {\n@@ -49,6 +52,25 @@ test_expect_success 'git-add invalidates cache-tree' '\n \ttest_invalid_cache_tree\n '\n \n+test_expect_success 'git-add in subdir invalidates cache-tree' '\n+\ttest_when_finished \"git reset --hard; git read-tree HEAD\" &&\n+\tmkdir dirx &&\n+\techo \"I changed this file\" >dirx/foo &&\n+\tgit add dirx/foo &&\n+\ttest_invalid_cache_tree\n+'\n+\n+test_expect_success 'git-add in subdir does not invalidate sibling cache-tree' '\n+\tgit tag no-children &&\n+\ttest_when_finished \"git reset --hard no-children; git read-tree HEAD\" &&\n+\tmkdir dir1 dir2 &&\n+\ttest_commit dir1/a &&\n+\ttest_commit dir2/b &&\n+\techo \"I changed this file\" >dir1/a &&\n+\tgit add dir1/a &&\n+\ttest_invalid_cache_tree dir1/\n+'\n+\n test_expect_success 'update-index invalidates cache-tree' '\n \ttest_when_finished \"git reset --hard; git read-tree HEAD\" &&\n \techo \"I changed this file\" >foo &&\n-- \n2.0.0.390.gcb682f8\n"},{"id":"245435","messageId":"1404619619-4774-4-git-send-email-dturner@twitter.com","threadId":"37064","inReplyTo":"1404619619-4774-1-git-send-email-dturner@twitter.com","subject":"[PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-07-06T04:06:59Z","receivedAt":"2014-07-06T04:06:59Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"During the commit process, update the cache-tree. Write this updated\ncache-tree so that it's ready for subsequent commands.\n\nAdd test code which demonstrates that git commit now writes the cache\ntree.  Make all tests test the entire cache-tree, not just the root\nlevel.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n builtin/commit.c      | 31 +++++++++++-------\n t/t0090-cache-tree.sh | 87 ++++++++++++++++++++++++++++++++++++++++++---------\n 2 files changed, 91 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 9cfef6c..5981755 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \n \t\tdiscard_cache();\n \t\tread_cache_from(index_lock.filename);\n+\t\tif (update_main_cache_tree(WRITE_TREE_SILENT) >= 0)\n+\t\t\twrite_cache(fd, active_cache, active_nr);\n \n \t\tcommit_style = COMMIT_NORMAL;\n \t\treturn index_lock.filename;\n@@ -363,10 +365,18 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, 0);\n \t\trefresh_cache_or_die(refresh_flags);\n-\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n-\t\tif (write_cache(fd, active_cache, active_nr) ||\n-\t\t    close_lock_file(&index_lock))\n-\t\t\tdie(_(\"unable to write new_index file\"));\n+\t\tif (active_cache_changed\n+\t\t    || !cache_tree_fully_valid(active_cache_tree)) {\n+\t\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n+\t\t\tactive_cache_changed = 1;\n+\t\t}\n+\t\tif (active_cache_changed) {\n+\t\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t\t    close_lock_file(&index_lock))\n+\t\t\t\tdie(_(\"unable to write new_index file\"));\n+\t\t} else {\n+\t\t\trollback_lock_file(&index_lock);\n+\t\t}\n \t\tcommit_style = COMMIT_NORMAL;\n \t\treturn index_lock.filename;\n \t}\n@@ -383,14 +393,10 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \tif (!only && !pathspec.nr) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\trefresh_cache_or_die(refresh_flags);\n-\t\tif (active_cache_changed) {\n-\t\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n-\t\t\tif (write_cache(fd, active_cache, active_nr) ||\n-\t\t\t    commit_locked_index(&index_lock))\n-\t\t\t\tdie(_(\"unable to write new_index file\"));\n-\t\t} else {\n-\t\t\trollback_lock_file(&index_lock);\n-\t\t}\n+\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n+\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t    commit_locked_index(&index_lock))\n+\t\t\tdie(_(\"unable to write new_index file\"));\n \t\tcommit_style = COMMIT_AS_IS;\n \t\treturn get_index_file();\n \t}\n@@ -435,6 +441,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \tfd = hold_locked_index(&index_lock, 1);\n \tadd_remove_files(&partial);\n \trefresh_cache(REFRESH_QUIET);\n+\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n \tif (write_cache(fd, active_cache, active_nr) ||\n \t    close_lock_file(&index_lock))\n \t\tdie(_(\"unable to write new_index file\"));\ndiff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\nindex 8437c5f..15f1484 100755\n--- a/t/t0090-cache-tree.sh\n+++ b/t/t0090-cache-tree.sh\n@@ -7,8 +7,14 @@ cache-tree extension.\n \"\n  . ./test-lib.sh\n \n+grep_nonmatch_ok () {\n+    grep $@\n+    test \"$?\" = \"2\" && return 1\n+    return 0\n+}\n+\n cmp_cache_tree () {\n-\ttest-dump-cache-tree >actual &&\n+\ttest-dump-cache-tree | grep_nonmatch_ok -v \\#\\(ref\\) >actual &&\n \tsed \"s/$_x40/SHA/\" <actual >filtered &&\n \ttest_cmp \"$1\" filtered\n }\n@@ -16,15 +22,33 @@ cmp_cache_tree () {\n # We don't bother with actually checking the SHA1:\n # test-dump-cache-tree already verifies that all existing data is\n # correct.\n-test_shallow_cache_tree () {\n-\tprintf \"SHA  (%d entries, 0 subtrees)\\n\" $(git ls-files|wc -l) >expect &&\n+generate_expected_cache_tree () {\n+\tdir=\"$1${1:+/}\" &&\n+\tparent=\"$2\" &&\n+\t# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux\n+\t# We want to count only foo because it's the only direct child\n+\tsubtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) &&\n+\tsubtree_count=$(echo \"$subtrees\"|grep_nonmatch_ok -c .) &&\n+\tentries=$(git ls-files|wc -l) &&\n+\tprintf \"SHA $dir (%d entries, %d subtrees)\\n\" $entries $subtree_count &&\n+\tfor subtree in $subtrees\n+\tdo\n+\t    cd \"$subtree\"\n+\t    generate_expected_cache_tree \"$dir$subtree\" $dir || return 1\n+\t    cd ..\n+\tdone &&\n+\tdir=$parent\n+}\n+\n+test_cache_tree () {\n+\tgenerate_expected_cache_tree >expect &&\n \tcmp_cache_tree expect\n }\n \n # Test that the cache-tree for a given directory is invalid.\n # If no directory is given, check that the root is invalid\n test_invalid_cache_tree () {\n-\ttest-dump-cache-tree >actual &&\n+\ttest-dump-cache-tree | grep_nonmatch_ok -v \\#\\(ref\\) >actual &&\n \tsed -e \"s/$_x40/SHA/\" -e \"s/[0-9]* subtrees//g\" <actual >filtered &&\n \texpect=$(printf \"invalid                                  $1 ()\\n\") &&\n \tfgrep \"$expect\" filtered\n@@ -35,14 +59,14 @@ test_no_cache_tree () {\n \tcmp_cache_tree expect\n }\n \n-test_expect_failure 'initial commit has cache-tree' '\n+test_expect_success 'initial commit has cache-tree' '\n \ttest_commit foo &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'read-tree HEAD establishes cache-tree' '\n \tgit read-tree HEAD &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'git-add invalidates cache-tree' '\n@@ -60,6 +84,18 @@ test_expect_success 'git-add in subdir invalidates cache-tree' '\n \ttest_invalid_cache_tree\n '\n \n+cat >before <<\\EOF\n+SHA  (3 entries, 2 subtrees)\n+SHA dir1/ (1 entries, 0 subtrees)\n+SHA dir2/ (1 entries, 0 subtrees)\n+EOF\n+\n+cat >expect <<\\EOF\n+invalid                                   (2 subtrees)\n+invalid                                  dir1/ (0 subtrees)\n+SHA dir2/ (1 entries, 0 subtrees)\n+EOF\n+\n test_expect_success 'git-add in subdir does not invalidate sibling cache-tree' '\n \tgit tag no-children &&\n \ttest_when_finished \"git reset --hard no-children; git read-tree HEAD\" &&\n@@ -67,8 +103,10 @@ test_expect_success 'git-add in subdir does not invalidate sibling cache-tree' '\n \ttest_commit dir1/a &&\n \ttest_commit dir2/b &&\n \techo \"I changed this file\" >dir1/a &&\n+\tcmp_cache_tree before &&\n+\techo \"I changed this file\" >dir1/a &&\n \tgit add dir1/a &&\n-\ttest_invalid_cache_tree dir1/\n+\tcmp_cache_tree expect\n '\n \n test_expect_success 'update-index invalidates cache-tree' '\n@@ -81,7 +119,7 @@ test_expect_success 'update-index invalidates cache-tree' '\n test_expect_success 'write-tree establishes cache-tree' '\n \ttest-scrap-cache-tree &&\n \tgit write-tree &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'test-scrap-cache-tree works' '\n@@ -92,37 +130,56 @@ test_expect_success 'test-scrap-cache-tree works' '\n \n test_expect_success 'second commit has cache-tree' '\n \ttest_commit bar &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n+'\n+\n+test_expect_success 'commit in child dir has cache-tree' '\n+\tmkdir dir &&\n+\t>dir/child.t &&\n+\tgit add dir/child.t &&\n+\tgit commit -m dir/child.t &&\n+\ttest_cache_tree\n '\n \n test_expect_success 'reset --hard gives cache-tree' '\n \ttest-scrap-cache-tree &&\n \tgit reset --hard &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'reset --hard without index gives cache-tree' '\n \trm -f .git/index &&\n \tgit reset --hard &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'checkout gives cache-tree' '\n \tgit tag current &&\n \tgit checkout HEAD^ &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'checkout -b gives cache-tree' '\n \tgit checkout current &&\n \tgit checkout -b prev HEAD^ &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'checkout -B gives cache-tree' '\n \tgit checkout current &&\n \tgit checkout -B prev HEAD^ &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n+'\n+\n+test_expect_success 'partial commit gives cache-tree' '\n+\tgit checkout -b partial no-children &&\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\techo \"some change\" >one.t &&\n+\tgit add one.t &&\n+\techo \"some other change\" >two.t &&\n+\tgit commit two.t -m partial &&\n+\ttest_cache_tree\n '\n \n test_done\n-- \n2.0.0.390.gcb682f8\n"},{"id":"245439","messageId":"CAPig+cScqL6qEo8e9QKgDEZT5cxnt8qCrVwpuhiJt-MmcKEuXQ@mail.gmail.com","threadId":"37064","inReplyTo":"1404619619-4774-3-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 3/4] cache-tree: subdirectory tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-07-06T08:10:50Z","receivedAt":"2014-07-06T08:10:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jul 6, 2014 at 12:06 AM, David Turner <dturner@twopensource.com> wrote:\n> Add tests to confirm that invalidation of subdirectories nether over-\n\ns/nether/neither/\n\n> nor under-invalidates.\n>\n> Signed-off-by: David Turner <dturner@twitter.com>\n> ---\n"},{"id":"245487","messageId":"xmqqmwclc7xq.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"1404619619-4774-1-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 1/4] cache-tree: Create/update cache-tree on checkout","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-07T18:58:25Z","receivedAt":"2014-07-07T18:58: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> When git checkout checks out a branch, create or update the\n> cache-tree so that subsequent operations are faster.\n>\n> update_main_cache_tree learned a new flag, WRITE_TREE_REPAIR.  When\n> WRITE_TREE_REPAIR is set, portions of the cache-tree which do not\n> correspond to existing tree objects are invalidated (and portions which\n> do are marked as valid).  No new tree objects are created.\n>\n> Signed-off-by: David Turner <dturner@twitter.com>\n> ---\n\nLooks safe, if a bit wasteful, to me.\n\nThanks; will queue.\n"},{"id":"245488","messageId":"xmqqegxxc75w.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"1404619619-4774-3-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 3/4] cache-tree: subdirectory tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-07T19:15:07Z","receivedAt":"2014-07-07T19:15:07Z","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> Add tests to confirm that invalidation of subdirectories nether over-\n> nor under-invalidates.\n>\n> Signed-off-by: David Turner <dturner@twitter.com>\n> ---\n>  t/t0090-cache-tree.sh | 28 +++++++++++++++++++++++++---\n>  1 file changed, 25 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\n> index 98fb1ab..8437c5f 100755\n> --- a/t/t0090-cache-tree.sh\n> +++ b/t/t0090-cache-tree.sh\n> @@ -21,10 +21,13 @@ test_shallow_cache_tree () {\n>  \tcmp_cache_tree expect\n>  }\n>  \n> +# Test that the cache-tree for a given directory is invalid.\n> +# If no directory is given, check that the root is invalid\n>  test_invalid_cache_tree () {\n> -\techo \"invalid                                   (0 subtrees)\" >expect &&\n> -\tprintf \"SHA #(ref)  (%d entries, 0 subtrees)\\n\" $(git ls-files|wc -l) >>expect &&\n> -\tcmp_cache_tree expect\n> +\ttest-dump-cache-tree >actual &&\n> +\tsed -e \"s/$_x40/SHA/\" -e \"s/[0-9]* subtrees//g\" <actual >filtered &&\n> +\texpect=$(printf \"invalid                                  $1 ()\\n\") &&\n\nIt would be saner to do 'printf \"string %s more string\" \"$1\"' than\nembedding caller-supplied \"$1\" inside the format specifier.\n\n> +\tfgrep \"$expect\" filtered\n\nWe'd actually want to see fewer uses of 'fgrep' in the tests for two\nreasons.\n\nIs having an entry that is invalidated the only thing we care about\nin this test?  Shouldn't the caller expect \"These subtrees and\nnothing else must be invalidated\", in which case the helper should\ncheck not just the expected \"invalid dir1/\" appears in the output\nbut no other unexpected \"invalid somethingelse/\" appears (and this\n\"no other unexpected output\" makes use of grep family in tests like\nthis less desirable).\n\nIn other words, wouldn't it be better to do the helper along the\nlines of:\n\n\ttest_invalidated_cache_tree () {\n        \tif test $# != 0\n\t\tthen\n\t        \tprintf \"invalid %s ()\\n\" \"\" \"$@\"\n                fi >expect &&\n\t\ttest-dump-cache-tree |\n                sed -n -e '/^invalid /p' >actual &&\n                test_cmp expect actual\n\t}\n\nand use\n\n\ttest_invalidated_cache_tree dir1\n\nwhen we expect only dir1 and dir2 (but not dir2 or anything else) is\ninvalidated?\n\nThanks.\n"},{"id":"245489","messageId":"xmqqa98lc6l4.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"1404619619-4774-2-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 2/4] test-dump-cache-tree: invalid trees are not errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-07T19:27:35Z","receivedAt":"2014-07-07T19:27:35Z","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> Do not treat known-invalid trees as errors even when their count is\n> incorrect.  Because git already knows that these trees are invalid,\n> nothing depends on the count field.\n\ns/count/subtree_nr/; they are different.\n\n\"nothing depends on\" is not quite correct.  The field keeps track of\nthe number of subdirectories in the directory recorded in the\ncache-tree, and it *must* be maintained correctly (we iterate over\nthe it->down[] array up to that number).\n\nThe number of subdirectories the directory actually has, which we\ncan discover by counting entries in the main part of the index, may\nbe different from subtree_nr, if we haven't run update_one() on it,\ne.g. we may have added a path in a new subdirectory, at which time\nwe would have invalidated the directory and its it->down[] array does\nnot know the new subdirectory.\n\nWhile reading 3/4, I wondered if it makes sense to show the (N\nsubtrees) for invalidated node, but as a debugging measure it helped\nme often while developping the framework, so we may not want to drop\nthe subtree_nr from the output for invalidated entries.\n\nThe change itself looks good.\n\nThanks.\n"},{"id":"245491","messageId":"xmqq61j9c4xb.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"1404619619-4774-4-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-07T20:03:28Z","receivedAt":"2014-07-07T20:03:28Z","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> During the commit process, update the cache-tree. Write this updated\n> cache-tree so that it's ready for subsequent commands.\n>\n> Add test code which demonstrates that git commit now writes the cache\n> tree.  Make all tests test the entire cache-tree, not just the root\n> level.\n>\n> Signed-off-by: David Turner <dturner@twitter.com>\n> ---\n>  builtin/commit.c      | 31 +++++++++++-------\n>  t/t0090-cache-tree.sh | 87 ++++++++++++++++++++++++++++++++++++++++++---------\n>  2 files changed, 91 insertions(+), 27 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 9cfef6c..5981755 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n>  \n>  \t\tdiscard_cache();\n>  \t\tread_cache_from(index_lock.filename);\n> +\t\tif (update_main_cache_tree(WRITE_TREE_SILENT) >= 0)\n> +\t\t\twrite_cache(fd, active_cache, active_nr);\n>  \n>  \t\tcommit_style = COMMIT_NORMAL;\n>  \t\treturn index_lock.filename;\n> @@ -363,10 +365,18 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n>  \t\tfd = hold_locked_index(&index_lock, 1);\n>  \t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, 0);\n>  \t\trefresh_cache_or_die(refresh_flags);\n> -\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n> -\t\tif (write_cache(fd, active_cache, active_nr) ||\n> -\t\t    close_lock_file(&index_lock))\n> -\t\t\tdie(_(\"unable to write new_index file\"));\n> +\t\tif (active_cache_changed\n> +\t\t    || !cache_tree_fully_valid(active_cache_tree)) {\n> +\t\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n> +\t\t\tactive_cache_changed = 1;\n> +\t\t}\n> +\t\tif (active_cache_changed) {\n> +\t\t\tif (write_cache(fd, active_cache, active_nr) ||\n> +\t\t\t    close_lock_file(&index_lock))\n> +\t\t\t\tdie(_(\"unable to write new_index file\"));\n> +\t\t} else {\n> +\t\t\trollback_lock_file(&index_lock);\n> +\t\t}\n>  \t\tcommit_style = COMMIT_NORMAL;\n>  \t\treturn index_lock.filename;\n>  \t}\n> @@ -383,14 +393,10 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n>  \tif (!only && !pathspec.nr) {\n>  \t\tfd = hold_locked_index(&index_lock, 1);\n>  \t\trefresh_cache_or_die(refresh_flags);\n> -\t\tif (active_cache_changed) {\n> -\t\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n> -\t\t\tif (write_cache(fd, active_cache, active_nr) ||\n> -\t\t\t    commit_locked_index(&index_lock))\n> -\t\t\t\tdie(_(\"unable to write new_index file\"));\n> -\t\t} else {\n> -\t\t\trollback_lock_file(&index_lock);\n> -\t\t}\n> +\t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n> +\t\tif (write_cache(fd, active_cache, active_nr) ||\n> +\t\t    commit_locked_index(&index_lock))\n> +\t\t\tdie(_(\"unable to write new_index file\"));\n>  \t\tcommit_style = COMMIT_AS_IS;\n>  \t\treturn get_index_file();\n>  \t}\n\nHmph.  The above two hunks somehow feel the other way around.  When\ndoing a non-partial, non as-is commit, i.e. \"git commit $paths\", the\noriginal used to call 'write-cache' unconditionally, because it is\nvery unlikely for us to see !active_cache_changed after calling the\nadd-files-to-cache with user-supplied pathspec (if there is nothing\nto change, the user is being silly to say \"git commit $paths\" in the\nfirst place).  I would have expected that the patch would have left\nthat code path alone (it seems to be doing the right thing already).\n\nOn the other hand, \"git commit\" to commit the contents of the index\nas-is is being cautious not to write things out unnecessarily, but\nas you found out, it would be a good idea to fully populate the\ncache-tree in this code path and write the otherwise already-good\nindex file out, even if we see !active_cache_changed after we called\nrefresh_cache_or_die().  So I would have expected that the patch\nwould have kept the \"write only necessary\" carefulness instead of\ncalling write-cache unconditionally.\n\nThat is, something like:\n\n\tfd = hold_locked_index();\n        refresh_cache_or_die();\n+\tif (!cache_tree_fully_valid())\n+               active_cache_changed = 1;\n\tif (active_cache_changed) {\n\t\tupdate_main_cache_tree();\n\n> diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\n> index 8437c5f..15f1484 100755\n> --- a/t/t0090-cache-tree.sh\n> +++ b/t/t0090-cache-tree.sh\n> @@ -7,8 +7,14 @@ cache-tree extension.\n>  \"\n>   . ./test-lib.sh\n>  \n> +grep_nonmatch_ok () {\n> +    grep $@\n\ndq around it, i.e. \"$@\".\n\n> +    test \"$?\" = \"2\" && return 1\n\nPOSIX.1 only specifies >1 not necessarily 2 as an error status.  \n\n> +    return 0\n> +}\n\nHaving said all that I do not think the helper is unnecessary.  Just\nuse something like\n\n\tsed -e '/#(ref)/d'\n\n>  cmp_cache_tree () {\n> -\ttest-dump-cache-tree >actual &&\n> +\ttest-dump-cache-tree | grep_nonmatch_ok -v \\#\\(ref\\) >actual &&\n"},{"id":"245502","messageId":"xmqq7g3obsqm.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"xmqq61j9c4xb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-08T00:26:41Z","receivedAt":"2014-07-08T00:26:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> diff --git a/builtin/commit.c b/builtin/commit.c\n>> index 9cfef6c..5981755 100644\n>> --- a/builtin/commit.c\n>> +++ b/builtin/commit.c\n>> @@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n>>  \n>>  \t\tdiscard_cache();\n>>  \t\tread_cache_from(index_lock.filename);\n>> +\t\tif (update_main_cache_tree(WRITE_TREE_SILENT) >= 0)\n>> +\t\t\twrite_cache(fd, active_cache, active_nr);\n>>  \n>>  \t\tcommit_style = COMMIT_NORMAL;\n>>  \t\treturn index_lock.filename;\n\nI'll push out a tentative result of merging this series (with some\nproposed fix-ups) sometime later today, but this part interacts with\nDuy's split-index topic in a funny way, so I'd exclude it from the\nmerge result for now.\n\nThanks.\n"},{"id":"245516","messageId":"CACsJy8C20oFdATHKTLK=9U3_kHu1QsuS4i74RPgQn0aTwVCC8w@mail.gmail.com","threadId":"37064","inReplyTo":"xmqq7g3obsqm.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-08T10:32:20Z","receivedAt":"2014-07-08T10:32:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jul 8, 2014 at 7:26 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>>> diff --git a/builtin/commit.c b/builtin/commit.c\n>>> index 9cfef6c..5981755 100644\n>>> --- a/builtin/commit.c\n>>> +++ b/builtin/commit.c\n>>> @@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n>>>\n>>>              discard_cache();\n>>>              read_cache_from(index_lock.filename);\n>>> +            if (update_main_cache_tree(WRITE_TREE_SILENT) >= 0)\n>>> +                    write_cache(fd, active_cache, active_nr);\n>>>\n>>>              commit_style = COMMIT_NORMAL;\n>>>              return index_lock.filename;\n\nI wonder if we need to update_main_cache_tree() so many times in this\nfunction. If I read the code correctly, all roads must lead to\nupdate_main_cache_tree(0) in prepare_to_commit(). If we find out that\nwe have incomplete cache-tree before that call, we could write the\nindex one more time after that call, instead of spreading\nupdate_main_cache_tree() all over prepare_index(). I know the\n\"index_file\" in prepare_to_commit() is probably \"index.lock\" or\nsomething, but that does not stop us from locking again\n(\"index.lock.lock\") if we want to update it.\n\nWriting cache tree early in prepare_index() does help hooks, but I\nwould say hooks are uncommon case and we could add an option to\nupdate-index to explicitly rebuild cache-tree, then hooks that do diff\na lot (or other operations that use cache-tree) could rebuild\ncache-tree by themselves. When the index file is large, every write\npays high penalty so I think trying to write less often is good.\n-- \nDuy\n"},{"id":"245540","messageId":"xmqqwqbnaii3.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"CACsJy8C20oFdATHKTLK=9U3_kHu1QsuS4i74RPgQn0aTwVCC8w@mail.gmail.com","subject":"Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-08T17:05:24Z","receivedAt":"2014-07-08T17:05:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> I wonder if we need to update_main_cache_tree() so many times in this\n> function. If I read the code correctly, all roads must lead to\n> update_main_cache_tree(0) in prepare_to_commit().\n\nI think prepare-to-commit is too late; it does not want to know if\nthe index it was given to create a tree out of is the one that the\nuser will keep using after this invocation of \"git commit\" or just a\ntemporary one used for partial commit.  The cache-tree rebuilt there\nis what is recorded with commit_tree_extended() in cmd_commit(), but\nif you are doing a partial commit, these generic code paths are\ngiven a temporary index file on the filesystem to work on, and\ncache-tree in that will not help user's later operation.\n\nFor three main uses of \"git commit\", prepare_index() does these:\n\n - \"git commit -a\" and \"git commit -i $paths\" update the index with\n   the new contents from the working tree, fully builds cache-tree\n   in-core to write out the tree objects, and writes the index file\n   to the filesystem.  Because this index is the one used after this\n   invocation of \"git commit\" returns, we have a fully populated\n   cache-tree after this happens.  This code path is perfect and\n   there is no need to change.\n\n - \"git commit\" commits the contents of the index as-is, so\n   technically there is no reason for it to update the index on the\n   filesystem at all, but it refreshes the cached stat data to help\n   the \"status\" part of the command, and if it finds that stat data\n   was stale, updates the index on the filesystem because it is\n   wasteful not to do so.  As we would be spending I/O cycles to\n   update the index file in that case anyway, we also rebuild the\n   cache-tree and include that in the updated index.\n\n   When the cached stat data was already up-to-date, however, we do\n   not update the index on the filesystem, so the series under\n   discussion will change the trade-off by doing one more I/O to\n   write out a new index to the filesystem only to update cache-tree.\n\n - \"git commit $paths\" updates the \"real\" index with given $paths\n   and writes it out to the filesystem first.  This is the index the\n   user will use after \"git commit\" finishes; traditionally our\n   trade-off was \"populate cache-tree only when we do not have to\n   spend any cycle only to do so (i.e. when we are writing trees\n   anyway, or when we read from a tree), and let it degrade as paths\n   are added, removed and modified\" and we avoided populating\n   cache-tree in this codepath.  The series under discussion will\n   change the trade-off here, too.\n\n   After it updates this \"real\" index, it builds another temporary\n   index that represents the tree state to be committed (starting\n   from HEAD and updates with the given $paths), but that will be\n   discarded and we do not want to spend any extra cycle to do\n   anything only to make its later use more efficient (like writing\n   updated cache-tree to it).\n\n> If we find out that\n> we have incomplete cache-tree before that call, we could write the\n> index one more time after that call,\n\nand that will make an extra I/O only to write out cache-tree to the\ntemporary index that we will discard immediately after for a partial\ncommit.\n"},{"id":"245543","messageId":"xmqqfvibaegr.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"CACsJy8C20oFdATHKTLK=9U3_kHu1QsuS4i74RPgQn0aTwVCC8w@mail.gmail.com","subject":"Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-08T18:32:36Z","receivedAt":"2014-07-08T18:32:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> ... I know the\n> \"index_file\" in prepare_to_commit() is probably \"index.lock\" or\n> something, but that does not stop us from locking again\n> (\"index.lock.lock\") if we want to update it.\n\nWe grabbed the lock on the real index and we have written out the\nresult of \"update-index --refresh\" to it (and closed), but we still\nwant to and do keep the lock while \"add -i\" works on it.  And then\nafter \"add -i\" returns, we still have the lock on the real index and\nthe patch wants to write to it again to store the refreshed cache-tree\nunder that lock.\n\nIt may be the case that the API suite currently lacks a way to allow\nthe caller to reopen the same \"index.lock\" file after calling\nwrite-locked-index(CLOSE_LOCK), and taking a lock on \"index.lock\" to\nwrite into \"index.lock.lock\" and renaming it to \"index.lock\" could\nbe a workaround for it, but doesn't that sound a wrong workaround?\n"},{"id":"245545","messageId":"xmqq61j7achx.fsf@gitster.dls.corp.google.com","threadId":"37064","inReplyTo":"CACsJy8C20oFdATHKTLK=9U3_kHu1QsuS4i74RPgQn0aTwVCC8w@mail.gmail.com","subject":"Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-08T19:15:06Z","receivedAt":"2014-07-08T19:15:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> Writing cache tree early in prepare_index() does help hooks, but I\n> would say hooks are uncommon case and we could add an option to\n> update-index to explicitly rebuild cache-tree, then hooks that do diff\n> a lot (or other operations that use cache-tree) could rebuild\n> cache-tree by themselves.\n\nYes, \"update-index --update-cache-tree\" would be a good addition for\ncompleteness; scripts working with plumbing should be able to do\nwhat built-in Porcelains can.  They can of course do \"write-tree\" in\nthe meantime so I do not see it as a very high priority, though.\n\nThis should apply on top of 'master', and if the series under\ndiscussion turns out to be a good idea, the new call to\nupdate-main-cache-tree I added to this code path should use the\noption added by the series that only repairs parts of cache-trees\nthat can be repaird without writing out new trees, so it is just to\ngive hints to future people (iow I am not going to apply this patch\nmyself right now).\n\n builtin/update-index.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex ebea285..1ce2274 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -26,6 +26,7 @@ static int allow_remove;\n static int allow_replace;\n static int info_only;\n static int force_remove;\n+static int update_cache_tree;\n static int verbose;\n static int mark_valid_only;\n static int mark_skip_worktree_only;\n@@ -762,6 +763,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"unmerged\", &refresh_args.flags,\n \t\t\tN_(\"refresh even if index contains unmerged entries\"),\n \t\t\tREFRESH_UNMERGED),\n+\t\tOPT_BOOL(0, \"update-cache-tree\", &update_cache_tree,\n+\t\t\t N_(\"update cache-tree before writing the result out\")),\n \t\t{OPTION_CALLBACK, 0, \"refresh\", &refresh_args, NULL,\n \t\t\tN_(\"refresh stat information\"),\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n@@ -918,6 +921,11 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_release(&buf);\n \t}\n \n+\tif (update_cache_tree && !unmerged_cache()) {\n+\t\tupdate_main_cache_tree(0);\n+\t\tactive_cache_changed = 1; /* force write-out */\n+\t}\n+\n \tif (active_cache_changed) {\n \t\tif (newfd < 0) {\n \t\t\tif (refresh_args.flags & REFRESH_QUIET)\n"},{"id":"245564","messageId":"CACsJy8Dqe7Ts7y0wxknq61rYUtaWfTNBqVa3i+db=qGG0vQgVw@mail.gmail.com","threadId":"37064","inReplyTo":"xmqqwqbnaii3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-09T01:58:31Z","receivedAt":"2014-07-09T01:58:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 9, 2014 at 12:05 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Duy Nguyen <pclouds@gmail.com> writes:\n>\n>> I wonder if we need to update_main_cache_tree() so many times in this\n>> function. If I read the code correctly, all roads must lead to\n>> update_main_cache_tree(0) in prepare_to_commit().\n>\n> I think prepare-to-commit is too late; it does not want to know if\n> the index it was given to create a tree out of is the one that the\n> user will keep using after this invocation of \"git commit\" or just a\n> temporary one used for partial commit.  The cache-tree rebuilt there\n> is what is recorded with commit_tree_extended() in cmd_commit(), but\n> if you are doing a partial commit, these generic code paths are\n> given a temporary index file on the filesystem to work on, and\n> cache-tree in that will not help user's later operation.\n\nRight. Thanks for checking.\n-- \nDuy\n"}]}