{"thread":{"id":"12554","subject":"[PATCH 0/4] Make unpack_trees() do separate source and destination indexes","startedAt":"2008-03-06T20:26:14Z","lastAt":"2008-03-07T21:48:40Z","messageCount":11,"participants":["Linus Torvalds","Junio C Hamano","Johannes Schindelin","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"71312","messageId":"58b39914e50e0cb91e026a6561803e9f039b8fa2.1204856187.git.torvalds@linux-foundation.org","threadId":"12554","inReplyTo":"cover.1204856187.git.torvalds@linux-foundation.org","subject":"[PATCH 3/4] Make 'unpack_trees()' take the index to work on as an argument","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-06T20:26:14Z","receivedAt":"2008-03-06T20:26:14Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"This is just a very mechanical conversion, and makes everybody set it to\n'&the_index' before calling, but at least it makes it more explicit\nwhere we work with the index.\n\nThe next stage would be to split that index usage up into a 'source' and\na 'destination' index, so that we can unpack into a different index than\nwe started out from.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n builtin-checkout.c        |    6 +++\n builtin-commit.c          |    1 +\n builtin-merge-recursive.c |    1 +\n builtin-read-tree.c       |    1 +\n diff-lib.c                |    2 +\n unpack-trees.c            |   79 +++++++++++++++++++++++----------------------\n unpack-trees.h            |    1 +\n 7 files changed, 52 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 6b08016..9bdb623 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -152,6 +152,7 @@ static int reset_to_new(struct tree *tree, int quiet)\n {\n \tstruct unpack_trees_options opts;\n \tstruct tree_desc tree_desc;\n+\n \tmemset(&opts, 0, sizeof(opts));\n \topts.head_idx = -1;\n \topts.update = 1;\n@@ -159,6 +160,7 @@ static int reset_to_new(struct tree *tree, int quiet)\n \topts.merge = 1;\n \topts.fn = oneway_merge;\n \topts.verbose_update = !quiet;\n+\topts.index = &the_index;\n \tparse_tree(tree);\n \tinit_tree_desc(&tree_desc, tree->buffer, tree->size);\n \tif (unpack_trees(1, &tree_desc, &opts))\n@@ -170,6 +172,7 @@ static void reset_clean_to_new(struct tree *tree, int quiet)\n {\n \tstruct unpack_trees_options opts;\n \tstruct tree_desc tree_desc;\n+\n \tmemset(&opts, 0, sizeof(opts));\n \topts.head_idx = -1;\n \topts.skip_unmerged = 1;\n@@ -177,6 +180,7 @@ static void reset_clean_to_new(struct tree *tree, int quiet)\n \topts.merge = 1;\n \topts.fn = oneway_merge;\n \topts.verbose_update = !quiet;\n+\topts.index = &the_index;\n \tparse_tree(tree);\n \tinit_tree_desc(&tree_desc, tree->buffer, tree->size);\n \tif (unpack_trees(1, &tree_desc, &opts))\n@@ -224,8 +228,10 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\tstruct tree_desc trees[2];\n \t\tstruct tree *tree;\n \t\tstruct unpack_trees_options topts;\n+\n \t\tmemset(&topts, 0, sizeof(topts));\n \t\ttopts.head_idx = -1;\n+\t\ttopts.index = &the_index;\n \n \t\trefresh_cache(REFRESH_QUIET);\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex f49c22e..38a5422 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -198,6 +198,7 @@ static void create_base_index(void)\n \topts.head_idx = 1;\n \topts.index_only = 1;\n \topts.merge = 1;\n+\topts.index = &the_index;\n \n \topts.fn = oneway_merge;\n \ttree = parse_tree_indirect(head_sha1);\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 6fe4102..50b3896 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -213,6 +213,7 @@ static int git_merge_trees(int index_only,\n \topts.merge = 1;\n \topts.head_idx = 2;\n \topts.fn = threeway_merge;\n+\topts.index = &the_index;\n \n \tinit_tree_desc_from_tree(t+0, common);\n \tinit_tree_desc_from_tree(t+1, head);\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 0138f5a..d004e90 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -102,6 +102,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \n \tmemset(&opts, 0, sizeof(opts));\n \topts.head_idx = -1;\n+\topts.index = &the_index;\n \n \tgit_config(git_default_config);\n \ndiff --git a/diff-lib.c b/diff-lib.c\nindex 4581b59..e359058 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -734,6 +734,7 @@ int run_diff_index(struct rev_info *revs, int cached)\n \topts.merge = 1;\n \topts.fn = oneway_diff;\n \topts.unpack_data = revs;\n+\topts.index = &the_index;\n \n \tinit_tree_desc(&t, tree->buffer, tree->size);\n \tif (unpack_trees(1, &t, &opts))\n@@ -787,6 +788,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \topts.merge = 1;\n \topts.fn = oneway_diff;\n \topts.unpack_data = &revs;\n+\topts.index = &the_index;\n \n \tinit_tree_desc(&t, tree->buffer, tree->size);\n \tif (unpack_trees(1, &t, &opts))\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ee9be29..cb8f847 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1,3 +1,4 @@\n+#define NO_THE_INDEX_COMPATIBILITY_MACROS\n #include \"cache.h\"\n #include \"dir.h\"\n #include \"tree.h\"\n@@ -7,10 +8,10 @@\n #include \"progress.h\"\n #include \"refs.h\"\n \n-static inline void remove_entry(int remove)\n+static inline void remove_entry(int remove, struct unpack_trees_options *o)\n {\n \tif (remove >= 0)\n-\t\tremove_cache_entry_at(remove);\n+\t\tremove_index_entry_at(o->index, remove);\n }\n \n /* Unlink the last component and attempt to remove leading\n@@ -53,8 +54,8 @@ static void check_updates(struct unpack_trees_options *o)\n \tint i;\n \n \tif (o->update && o->verbose_update) {\n-\t\tfor (total = cnt = 0; cnt < active_nr; cnt++) {\n-\t\t\tstruct cache_entry *ce = active_cache[cnt];\n+\t\tfor (total = cnt = 0; cnt < o->index->cache_nr; cnt++) {\n+\t\t\tstruct cache_entry *ce = o->index->cache[cnt];\n \t\t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\t\ttotal++;\n \t\t}\n@@ -65,15 +66,15 @@ static void check_updates(struct unpack_trees_options *o)\n \t}\n \n \t*last_symlink = '\\0';\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n+\tfor (i = 0; i < o->index->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = o->index->cache[i];\n \n \t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\tif (ce->ce_flags & CE_REMOVE) {\n \t\t\tif (o->update)\n \t\t\t\tunlink_entry(ce->name, last_symlink);\n-\t\t\tremove_cache_entry_at(i);\n+\t\t\tremove_index_entry_at(o->index, i);\n \t\t\ti--;\n \t\t\tcontinue;\n \t\t}\n@@ -105,7 +106,7 @@ static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_option\n \t\tif (o->skip_unmerged) {\n \t\t\to->pos++;\n \t\t} else {\n-\t\t\tremove_entry(o->pos);\n+\t\t\tremove_entry(o->pos, o);\n \t\t}\n \t\treturn 0;\n \t}\n@@ -242,9 +243,9 @@ static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmas\n \t\treturn call_unpack_fn(src, o, remove);\n \n \tn += o->merge;\n-\tremove_entry(remove);\n+\tremove_entry(remove, o);\n \tfor (i = 0; i < n; i++)\n-\t\tadd_cache_entry(src[i], ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);\n+\t\tadd_index_entry(o->index, src[i], ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);\n \treturn 0;\n }\n \n@@ -261,8 +262,8 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \n \t/* Are we supposed to look at the index too? */\n \tif (o->merge) {\n-\t\twhile (o->pos < active_nr) {\n-\t\t\tstruct cache_entry *ce = active_cache[o->pos];\n+\t\twhile (o->pos < o->index->cache_nr) {\n+\t\t\tstruct cache_entry *ce = o->index->cache[o->pos];\n \t\t\tint cmp = compare_entry(ce, info, p);\n \t\t\tif (cmp < 0) {\n \t\t\t\tif (unpack_index_entry(ce, o) < 0)\n@@ -277,7 +278,7 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \t\t\t\t\t */\n \t\t\t\t\tif (o->skip_unmerged)\n \t\t\t\t\t\treturn mask;\n-\t\t\t\t\tremove_entry(o->pos);\n+\t\t\t\t\tremove_entry(o->pos, o);\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \t\t\t\tsrc[0] = ce;\n@@ -312,8 +313,8 @@ static int unpack_failed(struct unpack_trees_options *o, const char *message)\n \t\t\treturn error(message);\n \t\treturn -1;\n \t}\n-\tdiscard_cache();\n-\tread_cache();\n+\tdiscard_index(o->index);\n+\tread_index(o->index);\n \treturn -1;\n }\n \n@@ -349,8 +350,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \n \t/* Any left-over entries in the index? */\n \tif (o->merge) {\n-\t\twhile (o->pos < active_nr) {\n-\t\t\tstruct cache_entry *ce = active_cache[o->pos];\n+\t\twhile (o->pos < o->index->cache_nr) {\n+\t\t\tstruct cache_entry *ce = o->index->cache[o->pos];\n \t\t\tif (unpack_index_entry(ce, o) < 0)\n \t\t\t\treturn unpack_failed(o, NULL);\n \t\t}\n@@ -395,7 +396,7 @@ static int verify_uptodate(struct cache_entry *ce,\n \t\treturn 0;\n \n \tif (!lstat(ce->name, &st)) {\n-\t\tunsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID);\n+\t\tunsigned changed = ie_match_stat(o->index, ce, &st, CE_MATCH_IGNORE_VALID);\n \t\tif (!changed)\n \t\t\treturn 0;\n \t\t/*\n@@ -415,10 +416,10 @@ static int verify_uptodate(struct cache_entry *ce,\n \t\terror(\"Entry '%s' not uptodate. Cannot merge.\", ce->name);\n }\n \n-static void invalidate_ce_path(struct cache_entry *ce)\n+static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o)\n {\n \tif (ce)\n-\t\tcache_tree_invalidate_path(active_cache_tree, ce->name);\n+\t\tcache_tree_invalidate_path(o->index->cache_tree, ce->name);\n }\n \n /*\n@@ -463,12 +464,12 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \t * in that directory.\n \t */\n \tnamelen = strlen(ce->name);\n-\tpos = cache_name_pos(ce->name, namelen);\n+\tpos = index_name_pos(o->index, ce->name, namelen);\n \tif (0 <= pos)\n \t\treturn cnt; /* we have it as nondirectory */\n \tpos = -pos - 1;\n-\tfor (i = pos; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n+\tfor (i = pos; i < o->index->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = o->index->cache[i];\n \t\tint len = ce_namelen(ce);\n \t\tif (len < namelen ||\n \t\t    strncmp(ce->name, ce->name, namelen) ||\n@@ -566,9 +567,9 @@ static int verify_absent(struct cache_entry *ce, const char *action,\n \t\t * delete this path, which is in a subdirectory that\n \t\t * is being replaced with a blob.\n \t\t */\n-\t\tcnt = cache_name_pos(ce->name, strlen(ce->name));\n+\t\tcnt = index_name_pos(o->index, ce->name, strlen(ce->name));\n \t\tif (0 <= cnt) {\n-\t\t\tstruct cache_entry *ce = active_cache[cnt];\n+\t\t\tstruct cache_entry *ce = o->index->cache[cnt];\n \t\t\tif (ce->ce_flags & CE_REMOVE)\n \t\t\t\treturn 0;\n \t\t}\n@@ -597,17 +598,17 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\t} else {\n \t\t\tif (verify_uptodate(old, o))\n \t\t\t\treturn -1;\n-\t\t\tinvalidate_ce_path(old);\n+\t\t\tinvalidate_ce_path(old, o);\n \t\t}\n \t}\n \telse {\n \t\tif (verify_absent(merge, \"overwritten\", o))\n \t\t\treturn -1;\n-\t\tinvalidate_ce_path(merge);\n+\t\tinvalidate_ce_path(merge, o);\n \t}\n \n \tmerge->ce_flags &= ~CE_STAGEMASK;\n-\tadd_cache_entry(merge, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n+\tadd_index_entry(o->index, merge, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \treturn 1;\n }\n \n@@ -621,14 +622,14 @@ static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,\n \t\tif (verify_absent(ce, \"removed\", o))\n \t\t\treturn -1;\n \tce->ce_flags |= CE_REMOVE;\n-\tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n-\tinvalidate_ce_path(ce);\n+\tadd_index_entry(o->index, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n+\tinvalidate_ce_path(ce, o);\n \treturn 1;\n }\n \n static int keep_entry(struct cache_entry *ce, struct unpack_trees_options *o)\n {\n-\tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD);\n+\tadd_index_entry(o->index, ce, ADD_CACHE_OK_TO_ADD);\n \treturn 1;\n }\n \n@@ -728,7 +729,7 @@ int threeway_merge(struct cache_entry **stages,\n \n \t/* #1 */\n \tif (!head && !remote && any_anc_missing) {\n-\t\tremove_entry(remove);\n+\t\tremove_entry(remove, o);\n \t\treturn 0;\n \t}\n \n@@ -762,7 +763,7 @@ int threeway_merge(struct cache_entry **stages,\n \t\tif ((head_deleted && remote_deleted) ||\n \t\t    (head_deleted && remote && remote_match) ||\n \t\t    (remote_deleted && head && head_match)) {\n-\t\t\tremove_entry(remove);\n+\t\t\tremove_entry(remove, o);\n \t\t\tif (index)\n \t\t\t\treturn deleted_entry(index, index, o);\n \t\t\telse if (ce && !head_deleted) {\n@@ -788,7 +789,7 @@ int threeway_merge(struct cache_entry **stages,\n \t\t\treturn -1;\n \t}\n \n-\tremove_entry(remove);\n+\tremove_entry(remove, o);\n \to->nontrivial_merge = 1;\n \n \t/* #2, #3, #4, #6, #7, #9, #10, #11. */\n@@ -853,7 +854,7 @@ int twoway_merge(struct cache_entry **src,\n \t\t}\n \t\telse if (oldtree && !newtree && same(current, oldtree)) {\n \t\t\t/* 10 or 11 */\n-\t\t\tremove_entry(remove);\n+\t\t\tremove_entry(remove, o);\n \t\t\treturn deleted_entry(oldtree, current, o);\n \t\t}\n \t\telse if (oldtree && newtree &&\n@@ -863,7 +864,7 @@ int twoway_merge(struct cache_entry **src,\n \t\t}\n \t\telse {\n \t\t\t/* all other failures */\n-\t\t\tremove_entry(remove);\n+\t\t\tremove_entry(remove, o);\n \t\t\tif (oldtree)\n \t\t\t\treturn o->gently ? -1 : reject_merge(oldtree);\n \t\t\tif (current)\n@@ -875,7 +876,7 @@ int twoway_merge(struct cache_entry **src,\n \t}\n \telse if (newtree)\n \t\treturn merged_entry(newtree, current, o);\n-\tremove_entry(remove);\n+\tremove_entry(remove, o);\n \treturn deleted_entry(oldtree, current, o);\n }\n \n@@ -922,14 +923,14 @@ int oneway_merge(struct cache_entry **src,\n \t\t\t     o->merge_size);\n \n \tif (!a) {\n-\t\tremove_entry(remove);\n+\t\tremove_entry(remove, o);\n \t\treturn deleted_entry(old, old, o);\n \t}\n \tif (old && same(old, a)) {\n \t\tif (o->reset) {\n \t\t\tstruct stat st;\n \t\t\tif (lstat(old->name, &st) ||\n-\t\t\t    ce_match_stat(old, &st, CE_MATCH_IGNORE_VALID))\n+\t\t\t    ie_match_stat(o->index, old, &st, CE_MATCH_IGNORE_VALID))\n \t\t\t\told->ce_flags |= CE_UPDATE;\n \t\t}\n \t\treturn keep_entry(old, o);\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex a2df544..65add16 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -28,6 +28,7 @@ struct unpack_trees_options {\n \n \tstruct cache_entry *df_conflict_entry;\n \tvoid *unpack_data;\n+\tstruct index_state *index;\n };\n \n extern int unpack_trees(unsigned n, struct tree_desc *t,\n-- \n1.5.4.3.452.g67136\n\n\n"},{"id":"71310","messageId":"041234e39ea23286e4793a894b2444df2cb7dc8b.1204856187.git.torvalds@linux-foundation.org","threadId":"12554","inReplyTo":"cover.1204856187.git.torvalds@linux-foundation.org","subject":"[PATCH 2/4] Add 'const' where appropriate to index handling functions","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-06T20:46:09Z","receivedAt":"2008-03-06T20:46:09Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"This is in an effort to make the source index of 'unpack_trees()' as\nbeing const, and thus making the compiler help us verify that we only\naccess it for reading.\n\nThe constification also extended to some of the hashing helpers that get\ncalled indirectly.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n cache.h      |   10 +++++-----\n hash.c       |    6 +++---\n hash.h       |    4 ++--\n read-cache.c |   12 ++++++------\n 4 files changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 6eb16cb..b519b7a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -346,12 +346,12 @@ extern void verify_non_filename(const char *prefix, const char *name);\n /* Initialize and use the cache information */\n extern int read_index(struct index_state *);\n extern int read_index_from(struct index_state *, const char *path);\n-extern int write_index(struct index_state *, int newfd);\n+extern int write_index(const struct index_state *, int newfd);\n extern int discard_index(struct index_state *);\n-extern int unmerged_index(struct index_state *);\n+extern int unmerged_index(const struct index_state *);\n extern int verify_path(const char *path);\n extern int index_name_exists(struct index_state *istate, const char *name, int namelen);\n-extern int index_name_pos(struct index_state *, const char *name, int namelen);\n+extern int index_name_pos(const struct index_state *, const char *name, int namelen);\n #define ADD_CACHE_OK_TO_ADD 1\t\t/* Ok to add */\n #define ADD_CACHE_OK_TO_REPLACE 2\t/* Ok to replace file/directory */\n #define ADD_CACHE_SKIP_DFCHECK 4\t/* Ok to skip DF conflict checks */\n@@ -368,8 +368,8 @@ extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);\n #define CE_MATCH_IGNORE_VALID\t\t01\n /* do not check the contents but report dirty on racily-clean entries */\n #define CE_MATCH_RACY_IS_DIRTY\t02\n-extern int ie_match_stat(struct index_state *, struct cache_entry *, struct stat *, unsigned int);\n-extern int ie_modified(struct index_state *, struct cache_entry *, struct stat *, unsigned int);\n+extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);\n+extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);\n \n extern int ce_path_match(const struct cache_entry *ce, const char **pathspec);\n extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path);\ndiff --git a/hash.c b/hash.c\nindex d9ec82f..1cd4c9d 100644\n--- a/hash.c\n+++ b/hash.c\n@@ -9,7 +9,7 @@\n  * the existing entry, or the empty slot if none existed. The caller\n  * can then look at the (*ptr) to see whether it existed or not.\n  */\n-static struct hash_table_entry *lookup_hash_entry(unsigned int hash, struct hash_table *table)\n+static struct hash_table_entry *lookup_hash_entry(unsigned int hash, const struct hash_table *table)\n {\n \tunsigned int size = table->size, nr = hash % size;\n \tstruct hash_table_entry *array = table->array;\n@@ -66,7 +66,7 @@ static void grow_hash_table(struct hash_table *table)\n \tfree(old_array);\n }\n \n-void *lookup_hash(unsigned int hash, struct hash_table *table)\n+void *lookup_hash(unsigned int hash, const struct hash_table *table)\n {\n \tif (!table->array)\n \t\treturn NULL;\n@@ -81,7 +81,7 @@ void **insert_hash(unsigned int hash, void *ptr, struct hash_table *table)\n \treturn insert_hash_entry(hash, ptr, table);\n }\n \n-int for_each_hash(struct hash_table *table, int (*fn)(void *))\n+int for_each_hash(const struct hash_table *table, int (*fn)(void *))\n {\n \tint sum = 0;\n \tunsigned int i;\ndiff --git a/hash.h b/hash.h\nindex a8b0fbb..69e33a4 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -28,9 +28,9 @@ struct hash_table {\n \tstruct hash_table_entry *array;\n };\n \n-extern void *lookup_hash(unsigned int hash, struct hash_table *table);\n+extern void *lookup_hash(unsigned int hash, const struct hash_table *table);\n extern void **insert_hash(unsigned int hash, void *ptr, struct hash_table *table);\n-extern int for_each_hash(struct hash_table *table, int (*fn)(void *));\n+extern int for_each_hash(const struct hash_table *table, int (*fn)(void *));\n extern void free_hash(struct hash_table *table);\n \n static inline void init_hash(struct hash_table *table)\ndiff --git a/read-cache.c b/read-cache.c\nindex bf649a3..a92b25b 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -255,13 +255,13 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \treturn changed;\n }\n \n-static int is_racy_timestamp(struct index_state *istate, struct cache_entry *ce)\n+static int is_racy_timestamp(const struct index_state *istate, struct cache_entry *ce)\n {\n \treturn (istate->timestamp &&\n \t\t((unsigned int)istate->timestamp) <= ce->ce_mtime);\n }\n \n-int ie_match_stat(struct index_state *istate,\n+int ie_match_stat(const struct index_state *istate,\n \t\t  struct cache_entry *ce, struct stat *st,\n \t\t  unsigned int options)\n {\n@@ -304,7 +304,7 @@ int ie_match_stat(struct index_state *istate,\n \treturn changed;\n }\n \n-int ie_modified(struct index_state *istate,\n+int ie_modified(const struct index_state *istate,\n \t\tstruct cache_entry *ce, struct stat *st, unsigned int options)\n {\n \tint changed, changed_fs;\n@@ -412,7 +412,7 @@ int cache_name_compare(const char *name1, int flags1, const char *name2, int fla\n \treturn 0;\n }\n \n-int index_name_pos(struct index_state *istate, const char *name, int namelen)\n+int index_name_pos(const struct index_state *istate, const char *name, int namelen)\n {\n \tint first, last;\n \n@@ -1201,7 +1201,7 @@ int discard_index(struct index_state *istate)\n \treturn 0;\n }\n \n-int unmerged_index(struct index_state *istate)\n+int unmerged_index(const struct index_state *istate)\n {\n \tint i;\n \tfor (i = 0; i < istate->cache_nr; i++) {\n@@ -1346,7 +1346,7 @@ static int ce_write_entry(SHA_CTX *c, int fd, struct cache_entry *ce)\n \treturn ce_write(c, fd, ondisk, size);\n }\n \n-int write_index(struct index_state *istate, int newfd)\n+int write_index(const struct index_state *istate, int newfd)\n {\n \tSHA_CTX c;\n \tstruct cache_header hdr;\n-- \n1.5.4.3.452.g67136\n\n\n"},{"id":"71311","messageId":"76edcf443909a95e2c391e23390a963f9a417dee.1204856187.git.torvalds@linux-foundation.org","threadId":"12554","inReplyTo":"cover.1204856187.git.torvalds@linux-foundation.org","subject":"[PATCH 1/4] Fix tree-walking compare_entry() in the presense of --prefix","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-06T23:44:48Z","receivedAt":"2008-03-06T23:44:48Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"When we make the \"root\" tree-walk info entry have a pathname in it, we\nneed to have a ->prev pointer so that compare_entry will actually notice\nand traverse into the root.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n tree-walk.c |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 842cb6a..02e2aed 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -107,6 +107,7 @@ int tree_entry(struct tree_desc *desc, struct name_entry *entry)\n void setup_traverse_info(struct traverse_info *info, const char *base)\n {\n \tint pathlen = strlen(base);\n+\tstatic struct traverse_info dummy;\n \n \tmemset(info, 0, sizeof(*info));\n \tif (pathlen && base[pathlen-1] == '/')\n@@ -114,6 +115,8 @@ void setup_traverse_info(struct traverse_info *info, const char *base)\n \tinfo->pathlen = pathlen ? pathlen + 1 : 0;\n \tinfo->name.path = base;\n \tinfo->name.sha1 = (void *)(base + pathlen + 1);\n+\tif (pathlen)\n+\t\tinfo->prev = &dummy;\n }\n \n char *make_traverse_path(char *path, const struct traverse_info *info, const struct name_entry *n)\n-- \n1.5.4.3.452.g67136\n\n\n"},{"id":"71313","messageId":"8676ea8b0313abfc2e0946f45f636643e28aade8.1204856187.git.torvalds@linux-foundation.org","threadId":"12554","inReplyTo":"cover.1204856187.git.torvalds@linux-foundation.org","subject":"[PATCH 4/4] Make 'unpack_trees()' have a separate source and destination index","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-07T02:12:28Z","receivedAt":"2008-03-07T02:12:28Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"We will always unpack into our own internal index, but we will take the\nsource from wherever specified, and we will optionally write the result\nto a specified index (optionally, because not everybody even _wants_ any\nresult: the index diffing really wants to just walk the tree and index\nin parallel).\n\nThis ends up removing a fair number more lines than it adds, for the\nsimple reason that we can now skip all the crud that tried to be\noh-so-careful about maintaining our position in the index as we were\ntraversing and modifying it.  Since we don't actually modify the source\nindex any more, we can just update the 'o->pos' pointer without worrying\nabout whether an index entry got removed or replaced or added to.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n builtin-checkout.c        |    9 ++-\n builtin-commit.c          |    3 +-\n builtin-merge-recursive.c |    3 +-\n builtin-read-tree.c       |   24 +-------\n diff-lib.c                |   49 +++------------\n unpack-trees.c            |  149 ++++++++++++++++++++++-----------------------\n unpack-trees.h            |   16 +++--\n 7 files changed, 101 insertions(+), 152 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 9bdb623..7deb504 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -160,7 +160,8 @@ static int reset_to_new(struct tree *tree, int quiet)\n \topts.merge = 1;\n \topts.fn = oneway_merge;\n \topts.verbose_update = !quiet;\n-\topts.index = &the_index;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n \tparse_tree(tree);\n \tinit_tree_desc(&tree_desc, tree->buffer, tree->size);\n \tif (unpack_trees(1, &tree_desc, &opts))\n@@ -180,7 +181,8 @@ static void reset_clean_to_new(struct tree *tree, int quiet)\n \topts.merge = 1;\n \topts.fn = oneway_merge;\n \topts.verbose_update = !quiet;\n-\topts.index = &the_index;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n \tparse_tree(tree);\n \tinit_tree_desc(&tree_desc, tree->buffer, tree->size);\n \tif (unpack_trees(1, &tree_desc, &opts))\n@@ -231,7 +233,8 @@ static int merge_working_tree(struct checkout_opts *opts,\n \n \t\tmemset(&topts, 0, sizeof(topts));\n \t\ttopts.head_idx = -1;\n-\t\ttopts.index = &the_index;\n+\t\ttopts.src_index = &the_index;\n+\t\ttopts.dst_index = &the_index;\n \n \t\trefresh_cache(REFRESH_QUIET);\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 38a5422..660a345 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -198,7 +198,8 @@ static void create_base_index(void)\n \topts.head_idx = 1;\n \topts.index_only = 1;\n \topts.merge = 1;\n-\topts.index = &the_index;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n \n \topts.fn = oneway_merge;\n \ttree = parse_tree_indirect(head_sha1);\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 50b3896..fa02bb5 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -213,7 +213,8 @@ static int git_merge_trees(int index_only,\n \topts.merge = 1;\n \topts.head_idx = 2;\n \topts.fn = threeway_merge;\n-\topts.index = &the_index;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n \n \tinit_tree_desc_from_tree(t+0, common);\n \tinit_tree_desc_from_tree(t+1, head);\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex d004e90..160456d 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -102,7 +102,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \n \tmemset(&opts, 0, sizeof(opts));\n \topts.head_idx = -1;\n-\topts.index = &the_index;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n \n \tgit_config(git_default_config);\n \n@@ -221,27 +222,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \tif ((opts.dir && !opts.update))\n \t\tdie(\"--exclude-per-directory is meaningless unless -u\");\n \n-\tif (opts.prefix) {\n-\t\tint pfxlen = strlen(opts.prefix);\n-\t\tint pos;\n-\t\tif (opts.prefix[pfxlen-1] != '/')\n-\t\t\tdie(\"prefix must end with /\");\n-\t\tif (stage != 2)\n-\t\t\tdie(\"binding merge takes only one tree\");\n-\t\tpos = cache_name_pos(opts.prefix, pfxlen);\n-\t\tif (0 <= pos)\n-\t\t\tdie(\"corrupt index file\");\n-\t\tpos = -pos-1;\n-\t\tif (pos < active_nr &&\n-\t\t    !strncmp(active_cache[pos]->name, opts.prefix, pfxlen))\n-\t\t\tdie(\"subdirectory '%s' already exists.\", opts.prefix);\n-\t\tpos = cache_name_pos(opts.prefix, pfxlen-1);\n-\t\tif (0 <= pos)\n-\t\t\tdie(\"file '%.*s' already exists.\",\n-\t\t\t\t\tpfxlen-1, opts.prefix);\n-\t\topts.pos = -1 - pos;\n-\t}\n-\n \tif (opts.merge) {\n \t\tif (stage < 2)\n \t\t\tdie(\"just how do you expect me to merge %d trees?\", stage-1);\ndiff --git a/diff-lib.c b/diff-lib.c\nindex e359058..9520773 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -600,8 +600,7 @@ static void mark_merge_entries(void)\n  */\n static void do_oneway_diff(struct unpack_trees_options *o,\n \tstruct cache_entry *idx,\n-\tstruct cache_entry *tree,\n-\tint idx_pos, int idx_nr)\n+\tstruct cache_entry *tree)\n {\n \tstruct rev_info *revs = o->unpack_data;\n \tint match_missing, cached;\n@@ -643,34 +642,6 @@ static void do_oneway_diff(struct unpack_trees_options *o,\n }\n \n /*\n- * Count how many index entries go with the first one\n- */\n-static inline int count_skip(const struct cache_entry *src, int pos)\n-{\n-\tint skip = 1;\n-\n-\t/* We can only have multiple entries if the first one is not stage-0 */\n-\tif (ce_stage(src)) {\n-\t\tstruct cache_entry **p = active_cache + pos;\n-\t\tint namelen = ce_namelen(src);\n-\n-\t\tfor (;;) {\n-\t\t\tconst struct cache_entry *ce;\n-\t\t\tpos++;\n-\t\t\tif (pos >= active_nr)\n-\t\t\t\tbreak;\n-\t\t\tce = *++p;\n-\t\t\tif (ce_namelen(ce) != namelen)\n-\t\t\t\tbreak;\n-\t\t\tif (memcmp(ce->name, src->name, namelen))\n-\t\t\t\tbreak;\n-\t\t\tskip++;\n-\t\t}\n-\t}\n-\treturn skip;\n-}\n-\n-/*\n  * The unpack_trees() interface is designed for merging, so\n  * the different source entries are designed primarily for\n  * the source trees, with the old index being really mainly\n@@ -685,18 +656,12 @@ static inline int count_skip(const struct cache_entry *src, int pos)\n  * the fairly complex unpack_trees() semantic requirements, including\n  * the skipping, the path matching, the type conflict cases etc.\n  */\n-static int oneway_diff(struct cache_entry **src,\n-\tstruct unpack_trees_options *o,\n-\tint index_pos)\n+static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)\n {\n-\tint skip = 0;\n \tstruct cache_entry *idx = src[0];\n \tstruct cache_entry *tree = src[1];\n \tstruct rev_info *revs = o->unpack_data;\n \n-\tif (index_pos >= 0)\n-\t\tskip = count_skip(idx, index_pos);\n-\n \t/*\n \t * Unpack-trees generates a DF/conflict entry if\n \t * there was a directory in the index and a tree\n@@ -707,9 +672,9 @@ static int oneway_diff(struct cache_entry **src,\n \t\ttree = NULL;\n \n \tif (ce_path_match(idx ? idx : tree, revs->prune_data))\n-\t\tdo_oneway_diff(o, idx, tree, index_pos, skip);\n+\t\tdo_oneway_diff(o, idx, tree);\n \n-\treturn skip;\n+\treturn 0;\n }\n \n int run_diff_index(struct rev_info *revs, int cached)\n@@ -734,7 +699,8 @@ int run_diff_index(struct rev_info *revs, int cached)\n \topts.merge = 1;\n \topts.fn = oneway_diff;\n \topts.unpack_data = revs;\n-\topts.index = &the_index;\n+\topts.src_index = &the_index;\n+\topts.dst_index = NULL;\n \n \tinit_tree_desc(&t, tree->buffer, tree->size);\n \tif (unpack_trees(1, &t, &opts))\n@@ -788,7 +754,8 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \topts.merge = 1;\n \topts.fn = oneway_diff;\n \topts.unpack_data = &revs;\n-\topts.index = &the_index;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n \n \tinit_tree_desc(&t, tree->buffer, tree->size);\n \tif (unpack_trees(1, &t, &opts))\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex cb8f847..0cdf198 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -8,10 +8,18 @@\n #include \"progress.h\"\n #include \"refs.h\"\n \n-static inline void remove_entry(int remove, struct unpack_trees_options *o)\n+static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,\n+\tunsigned int set, unsigned int clear)\n {\n-\tif (remove >= 0)\n-\t\tremove_index_entry_at(o->index, remove);\n+\tunsigned int size = ce_size(ce);\n+\tstruct cache_entry *new = xmalloc(size);\n+\n+\tclear |= CE_HASHED | CE_UNHASHED;\n+\n+\tmemcpy(new, ce, size);\n+\tnew->next = NULL;\n+\tnew->ce_flags = (new->ce_flags & ~clear) | set;\n+\tadd_index_entry(&o->result, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|ADD_CACHE_SKIP_DFCHECK);\n }\n \n /* Unlink the last component and attempt to remove leading\n@@ -51,11 +59,12 @@ static void check_updates(struct unpack_trees_options *o)\n \tunsigned cnt = 0, total = 0;\n \tstruct progress *progress = NULL;\n \tchar last_symlink[PATH_MAX];\n+\tstruct index_state *index = &o->result;\n \tint i;\n \n \tif (o->update && o->verbose_update) {\n-\t\tfor (total = cnt = 0; cnt < o->index->cache_nr; cnt++) {\n-\t\t\tstruct cache_entry *ce = o->index->cache[cnt];\n+\t\tfor (total = cnt = 0; cnt < index->cache_nr; cnt++) {\n+\t\t\tstruct cache_entry *ce = index->cache[cnt];\n \t\t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\t\ttotal++;\n \t\t}\n@@ -66,15 +75,15 @@ static void check_updates(struct unpack_trees_options *o)\n \t}\n \n \t*last_symlink = '\\0';\n-\tfor (i = 0; i < o->index->cache_nr; i++) {\n-\t\tstruct cache_entry *ce = o->index->cache[i];\n+\tfor (i = 0; i < index->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = index->cache[i];\n \n \t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\tif (ce->ce_flags & CE_REMOVE) {\n \t\t\tif (o->update)\n \t\t\t\tunlink_entry(ce->name, last_symlink);\n-\t\t\tremove_index_entry_at(o->index, i);\n+\t\t\tremove_index_entry_at(&o->result, i);\n \t\t\ti--;\n \t\t\tcontinue;\n \t\t}\n@@ -89,28 +98,27 @@ static void check_updates(struct unpack_trees_options *o)\n \tstop_progress(&progress);\n }\n \n-static inline int call_unpack_fn(struct cache_entry **src, struct unpack_trees_options *o, int remove)\n+static inline int call_unpack_fn(struct cache_entry **src, struct unpack_trees_options *o)\n {\n-\tint ret = o->fn(src, o, remove);\n-\tif (ret > 0) {\n-\t\to->pos += ret;\n+\tint ret = o->fn(src, o);\n+\tif (ret > 0)\n \t\tret = 0;\n-\t}\n \treturn ret;\n }\n \n static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_options *o)\n {\n \tstruct cache_entry *src[5] = { ce, };\n+\n+\to->pos++;\n \tif (ce_stage(ce)) {\n \t\tif (o->skip_unmerged) {\n-\t\t\to->pos++;\n-\t\t} else {\n-\t\t\tremove_entry(o->pos, o);\n+\t\t\tadd_entry(o, ce, 0, 0);\n+\t\t\treturn 0;\n \t\t}\n \t\treturn 0;\n \t}\n-\treturn call_unpack_fn(src, o, o->pos);\n+\treturn call_unpack_fn(src, o);\n }\n \n int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long df_conflicts, struct name_entry *names, struct traverse_info *info)\n@@ -200,7 +208,7 @@ static struct cache_entry *create_ce_entry(const struct traverse_info *info, con\n }\n \n static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmask, struct cache_entry *src[5],\n-\tconst struct name_entry *names, const struct traverse_info *info, int remove)\n+\tconst struct name_entry *names, const struct traverse_info *info)\n {\n \tint i;\n \tstruct unpack_trees_options *o = info->data;\n@@ -240,12 +248,11 @@ static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmas\n \t}\n \n \tif (o->merge)\n-\t\treturn call_unpack_fn(src, o, remove);\n+\t\treturn call_unpack_fn(src, o);\n \n \tn += o->merge;\n-\tremove_entry(remove, o);\n \tfor (i = 0; i < n; i++)\n-\t\tadd_index_entry(o->index, src[i], ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);\n+\t\tadd_entry(o, src[i], 0, 0);\n \treturn 0;\n }\n \n@@ -253,7 +260,6 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n {\n \tstruct cache_entry *src[5] = { NULL, };\n \tstruct unpack_trees_options *o = info->data;\n-\tint remove = -1;\n \tconst struct name_entry *p = names;\n \n \t/* Find first entry with a real name (we could use \"mask\" too) */\n@@ -262,8 +268,8 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \n \t/* Are we supposed to look at the index too? */\n \tif (o->merge) {\n-\t\twhile (o->pos < o->index->cache_nr) {\n-\t\t\tstruct cache_entry *ce = o->index->cache[o->pos];\n+\t\twhile (o->pos < o->src_index->cache_nr) {\n+\t\t\tstruct cache_entry *ce = o->src_index->cache[o->pos];\n \t\t\tint cmp = compare_entry(ce, info, p);\n \t\t\tif (cmp < 0) {\n \t\t\t\tif (unpack_index_entry(ce, o) < 0)\n@@ -271,24 +277,25 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!cmp) {\n+\t\t\t\to->pos++;\n \t\t\t\tif (ce_stage(ce)) {\n \t\t\t\t\t/*\n \t\t\t\t\t * If we skip unmerged index entries, we'll skip this\n \t\t\t\t\t * entry *and* the tree entries associated with it!\n \t\t\t\t\t */\n-\t\t\t\t\tif (o->skip_unmerged)\n+\t\t\t\t\tif (o->skip_unmerged) {\n+\t\t\t\t\t\tadd_entry(o, ce, 0, 0);\n \t\t\t\t\t\treturn mask;\n-\t\t\t\t\tremove_entry(o->pos, o);\n+\t\t\t\t\t}\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \t\t\t\tsrc[0] = ce;\n-\t\t\t\tremove = o->pos;\n \t\t\t}\n \t\t\tbreak;\n \t\t}\n \t}\n \n-\tif (unpack_nondirectories(n, mask, dirmask, src, names, info, remove) < 0)\n+\tif (unpack_nondirectories(n, mask, dirmask, src, names, info) < 0)\n \t\treturn -1;\n \n \t/* Now handle any directories.. */\n@@ -313,8 +320,6 @@ static int unpack_failed(struct unpack_trees_options *o, const char *message)\n \t\t\treturn error(message);\n \t\treturn -1;\n \t}\n-\tdiscard_index(o->index);\n-\tread_index(o->index);\n \treturn -1;\n }\n \n@@ -330,6 +335,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tstate.quiet = 1;\n \tstate.refresh_cache = 1;\n \n+\tmemset(&o->result, 0, sizeof(o->result));\n \to->merge_size = len;\n \n \tif (!dfc)\n@@ -350,8 +356,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \n \t/* Any left-over entries in the index? */\n \tif (o->merge) {\n-\t\twhile (o->pos < o->index->cache_nr) {\n-\t\t\tstruct cache_entry *ce = o->index->cache[o->pos];\n+\t\twhile (o->pos < o->src_index->cache_nr) {\n+\t\t\tstruct cache_entry *ce = o->src_index->cache[o->pos];\n \t\t\tif (unpack_index_entry(ce, o) < 0)\n \t\t\t\treturn unpack_failed(o, NULL);\n \t\t}\n@@ -360,7 +366,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tif (o->trivial_merges_only && o->nontrivial_merge)\n \t\treturn unpack_failed(o, \"Merge requires file-level merging\");\n \n+\to->src_index = NULL;\n \tcheck_updates(o);\n+\tif (o->dst_index)\n+\t\t*o->dst_index = o->result;\n \treturn 0;\n }\n \n@@ -396,7 +405,7 @@ static int verify_uptodate(struct cache_entry *ce,\n \t\treturn 0;\n \n \tif (!lstat(ce->name, &st)) {\n-\t\tunsigned changed = ie_match_stat(o->index, ce, &st, CE_MATCH_IGNORE_VALID);\n+\t\tunsigned changed = ie_match_stat(o->src_index, ce, &st, CE_MATCH_IGNORE_VALID);\n \t\tif (!changed)\n \t\t\treturn 0;\n \t\t/*\n@@ -419,7 +428,7 @@ static int verify_uptodate(struct cache_entry *ce,\n static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o)\n {\n \tif (ce)\n-\t\tcache_tree_invalidate_path(o->index->cache_tree, ce->name);\n+\t\tcache_tree_invalidate_path(o->src_index->cache_tree, ce->name);\n }\n \n /*\n@@ -464,12 +473,12 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \t * in that directory.\n \t */\n \tnamelen = strlen(ce->name);\n-\tpos = index_name_pos(o->index, ce->name, namelen);\n+\tpos = index_name_pos(o->src_index, ce->name, namelen);\n \tif (0 <= pos)\n \t\treturn cnt; /* we have it as nondirectory */\n \tpos = -pos - 1;\n-\tfor (i = pos; i < o->index->cache_nr; i++) {\n-\t\tstruct cache_entry *ce = o->index->cache[i];\n+\tfor (i = pos; i < o->src_index->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = o->src_index->cache[i];\n \t\tint len = ce_namelen(ce);\n \t\tif (len < namelen ||\n \t\t    strncmp(ce->name, ce->name, namelen) ||\n@@ -481,7 +490,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \t\tif (!ce_stage(ce)) {\n \t\t\tif (verify_uptodate(ce, o))\n \t\t\t\treturn -1;\n-\t\t\tce->ce_flags |= CE_REMOVE;\n+\t\t\tadd_entry(o, ce, CE_REMOVE, 0);\n \t\t}\n \t\tcnt++;\n \t}\n@@ -567,9 +576,9 @@ static int verify_absent(struct cache_entry *ce, const char *action,\n \t\t * delete this path, which is in a subdirectory that\n \t\t * is being replaced with a blob.\n \t\t */\n-\t\tcnt = index_name_pos(o->index, ce->name, strlen(ce->name));\n+\t\tcnt = index_name_pos(&o->result, ce->name, strlen(ce->name));\n \t\tif (0 <= cnt) {\n-\t\t\tstruct cache_entry *ce = o->index->cache[cnt];\n+\t\t\tstruct cache_entry *ce = o->result.cache[cnt];\n \t\t\tif (ce->ce_flags & CE_REMOVE)\n \t\t\t\treturn 0;\n \t\t}\n@@ -584,7 +593,6 @@ static int verify_absent(struct cache_entry *ce, const char *action,\n static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\tstruct unpack_trees_options *o)\n {\n-\tmerge->ce_flags |= CE_UPDATE;\n \tif (old) {\n \t\t/*\n \t\t * See if we can re-use the old CE directly?\n@@ -607,29 +615,29 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\tinvalidate_ce_path(merge, o);\n \t}\n \n-\tmerge->ce_flags &= ~CE_STAGEMASK;\n-\tadd_index_entry(o->index, merge, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n+\tadd_entry(o, merge, CE_UPDATE, CE_STAGEMASK);\n \treturn 1;\n }\n \n static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,\n \t\tstruct unpack_trees_options *o)\n {\n-\tif (old) {\n-\t\tif (verify_uptodate(old, o))\n-\t\t\treturn -1;\n-\t} else\n+\t/* Did it exist in the index? */\n+\tif (!old) {\n \t\tif (verify_absent(ce, \"removed\", o))\n \t\t\treturn -1;\n-\tce->ce_flags |= CE_REMOVE;\n-\tadd_index_entry(o->index, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n+\t\treturn 0;\n+\t}\n+\tif (verify_uptodate(old, o))\n+\t\treturn -1;\n+\tadd_entry(o, ce, CE_REMOVE, 0);\n \tinvalidate_ce_path(ce, o);\n \treturn 1;\n }\n \n static int keep_entry(struct cache_entry *ce, struct unpack_trees_options *o)\n {\n-\tadd_index_entry(o->index, ce, ADD_CACHE_OK_TO_ADD);\n+\tadd_entry(o, ce, 0, 0);\n \treturn 1;\n }\n \n@@ -649,9 +657,7 @@ static void show_stage_entry(FILE *o,\n }\n #endif\n \n-int threeway_merge(struct cache_entry **stages,\n-\t\tstruct unpack_trees_options *o,\n-\t\tint remove)\n+int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)\n {\n \tstruct cache_entry *index;\n \tstruct cache_entry *head;\n@@ -728,10 +734,8 @@ int threeway_merge(struct cache_entry **stages,\n \t}\n \n \t/* #1 */\n-\tif (!head && !remote && any_anc_missing) {\n-\t\tremove_entry(remove, o);\n+\tif (!head && !remote && any_anc_missing)\n \t\treturn 0;\n-\t}\n \n \t/* Under the new \"aggressive\" rule, we resolve mostly trivial\n \t * cases that we historically had git-merge-one-file resolve.\n@@ -763,10 +767,9 @@ int threeway_merge(struct cache_entry **stages,\n \t\tif ((head_deleted && remote_deleted) ||\n \t\t    (head_deleted && remote && remote_match) ||\n \t\t    (remote_deleted && head && head_match)) {\n-\t\t\tremove_entry(remove, o);\n \t\t\tif (index)\n \t\t\t\treturn deleted_entry(index, index, o);\n-\t\t\telse if (ce && !head_deleted) {\n+\t\t\tif (ce && !head_deleted) {\n \t\t\t\tif (verify_absent(ce, \"removed\", o))\n \t\t\t\t\treturn -1;\n \t\t\t}\n@@ -789,7 +792,6 @@ int threeway_merge(struct cache_entry **stages,\n \t\t\treturn -1;\n \t}\n \n-\tremove_entry(remove, o);\n \to->nontrivial_merge = 1;\n \n \t/* #2, #3, #4, #6, #7, #9, #10, #11. */\n@@ -824,9 +826,7 @@ int threeway_merge(struct cache_entry **stages,\n  * \"carry forward\" rule, please see <Documentation/git-read-tree.txt>.\n  *\n  */\n-int twoway_merge(struct cache_entry **src,\n-\t\tstruct unpack_trees_options *o,\n-\t\tint remove)\n+int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n {\n \tstruct cache_entry *current = src[0];\n \tstruct cache_entry *oldtree = src[1];\n@@ -854,7 +854,6 @@ int twoway_merge(struct cache_entry **src,\n \t\t}\n \t\telse if (oldtree && !newtree && same(current, oldtree)) {\n \t\t\t/* 10 or 11 */\n-\t\t\tremove_entry(remove, o);\n \t\t\treturn deleted_entry(oldtree, current, o);\n \t\t}\n \t\telse if (oldtree && newtree &&\n@@ -864,7 +863,6 @@ int twoway_merge(struct cache_entry **src,\n \t\t}\n \t\telse {\n \t\t\t/* all other failures */\n-\t\t\tremove_entry(remove, o);\n \t\t\tif (oldtree)\n \t\t\t\treturn o->gently ? -1 : reject_merge(oldtree);\n \t\t\tif (current)\n@@ -876,7 +874,6 @@ int twoway_merge(struct cache_entry **src,\n \t}\n \telse if (newtree)\n \t\treturn merged_entry(newtree, current, o);\n-\tremove_entry(remove, o);\n \treturn deleted_entry(oldtree, current, o);\n }\n \n@@ -887,8 +884,7 @@ int twoway_merge(struct cache_entry **src,\n  * stage0 does not have anything there.\n  */\n int bind_merge(struct cache_entry **src,\n-\t\tstruct unpack_trees_options *o,\n-\t\tint remove)\n+\t\tstruct unpack_trees_options *o)\n {\n \tstruct cache_entry *old = src[0];\n \tstruct cache_entry *a = src[1];\n@@ -898,7 +894,7 @@ int bind_merge(struct cache_entry **src,\n \t\t\t     o->merge_size);\n \tif (a && old)\n \t\treturn o->gently ? -1 :\n-\t\t\terror(\"Entry '%s' overlaps.  Cannot bind.\", a->name);\n+\t\t\terror(\"Entry '%s' overlaps with '%s'.  Cannot bind.\", a->name, old->name);\n \tif (!a)\n \t\treturn keep_entry(old, o);\n \telse\n@@ -911,9 +907,7 @@ int bind_merge(struct cache_entry **src,\n  * The rule is:\n  * - take the stat information from stage0, take the data from stage1\n  */\n-int oneway_merge(struct cache_entry **src,\n-\t\tstruct unpack_trees_options *o,\n-\t\tint remove)\n+int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n {\n \tstruct cache_entry *old = src[0];\n \tstruct cache_entry *a = src[1];\n@@ -922,18 +916,19 @@ int oneway_merge(struct cache_entry **src,\n \t\treturn error(\"Cannot do a oneway merge of %d trees\",\n \t\t\t     o->merge_size);\n \n-\tif (!a) {\n-\t\tremove_entry(remove, o);\n+\tif (!a)\n \t\treturn deleted_entry(old, old, o);\n-\t}\n+\n \tif (old && same(old, a)) {\n+\t\tint update = 0;\n \t\tif (o->reset) {\n \t\t\tstruct stat st;\n \t\t\tif (lstat(old->name, &st) ||\n-\t\t\t    ie_match_stat(o->index, old, &st, CE_MATCH_IGNORE_VALID))\n-\t\t\t\told->ce_flags |= CE_UPDATE;\n+\t\t\t    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID))\n+\t\t\t\tupdate |= CE_UPDATE;\n \t\t}\n-\t\treturn keep_entry(old, o);\n+\t\tadd_entry(o, old, update, 0);\n+\t\treturn 0;\n \t}\n \treturn merged_entry(a, old, o);\n }\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 65add16..e8abbcd 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -4,8 +4,7 @@\n struct unpack_trees_options;\n \n typedef int (*merge_fn_t)(struct cache_entry **src,\n-\t\tstruct unpack_trees_options *options,\n-\t\tint remove);\n+\t\tstruct unpack_trees_options *options);\n \n struct unpack_trees_options {\n \tint reset;\n@@ -28,15 +27,18 @@ struct unpack_trees_options {\n \n \tstruct cache_entry *df_conflict_entry;\n \tvoid *unpack_data;\n-\tstruct index_state *index;\n+\n+\tstruct index_state *dst_index;\n+\tconst struct index_state *src_index;\n+\tstruct index_state result;\n };\n \n extern int unpack_trees(unsigned n, struct tree_desc *t,\n \t\tstruct unpack_trees_options *options);\n \n-int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o, int);\n-int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o, int);\n-int bind_merge(struct cache_entry **src, struct unpack_trees_options *o, int);\n-int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o, int);\n+int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o);\n+int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o);\n+int bind_merge(struct cache_entry **src, struct unpack_trees_options *o);\n+int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o);\n \n #endif\n-- \n1.5.4.3.452.g67136\n\n"},{"id":"71309","messageId":"cover.1204856187.git.torvalds@linux-foundation.org","threadId":"12554","inReplyTo":null,"subject":"[PATCH 0/4] Make unpack_trees() do separate source and destination indexes","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-07T02:16:27Z","receivedAt":"2008-03-07T02:16:27Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"Ok, this goes on top of the five-commit series yesterday.  The first two\npatches are pure fixes and cleanups, the third one does a fairly\nmechanical thing to get rid of implied usage of \"the_index\", and the\nlast one actually splits up the source and destination index handling.\n\nLinus Torvalds (4):\n  Fix tree-walking compare_entry() in the presense of --prefix\n  Add 'const' where appropriate to index handling functions\n  Make 'unpack_trees()' take the index to work on as an argument\n  Make 'unpack_trees()' have a separate source and destination index\n\nThe nice thing to see is how this clearly removes more lines than it\nadds, and the code is a bit more readable too, I think.\n\n builtin-checkout.c        |    9 +++\n builtin-commit.c          |    2 +\n builtin-merge-recursive.c |    2 +\n builtin-read-tree.c       |   23 +------\n cache.h                   |   10 ++--\n diff-lib.c                |   47 ++-----------\n hash.c                    |    6 +-\n hash.h                    |    4 +-\n read-cache.c              |   12 ++--\n tree-walk.c               |    3 +\n unpack-trees.c            |  158 ++++++++++++++++++++++-----------------------\n unpack-trees.h            |   15 +++--\n 12 files changed, 128 insertions(+), 163 deletions(-)\n\n"},{"id":"71318","messageId":"7vwsof3x5n.fsf@gitster.siamese.dyndns.org","threadId":"12554","inReplyTo":"cover.1204856187.git.torvalds@linux-foundation.org","subject":"Re: [PATCH 0/4] Make unpack_trees() do separate source and destination indexes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-07T05:45:08Z","receivedAt":"2008-03-07T05:45:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> Ok, this goes on top of the five-commit series yesterday.  The first two\n> patches are pure fixes and cleanups, the third one does a fairly\n> mechanical thing to get rid of implied usage of \"the_index\", and the\n> last one actually splits up the source and destination index handling.\n\nWhat I have been wondering about this series was if we can re-enable\ncache-tree for more of the unpack_trees() users.\n\nCurrently, all unpack_trees() users, other than a single-tree read-tree,\ninvalidates the whole cache-tree information, as Daniel's \"pop one, decide\nwhat to put back, all in the original index\" had too many manual index\nmanipulations and sprinkling cache_tree_invalidate() call everywhere was\ntoo much cluttering of already hard-to-follow codepath.\n\nI'd want to see at least two-way read-tree preserve as much cache-tree\ninformation as possible.  People often work on one branch to completion\n(i.e. make a commit, which involves a write-tree, which leaves a fully\nvalid cache-tree in the index), switch branches (i.e. two-way read-tree,\nwhich unfortunately decimates cache-tree in the current implementation) to\ncommit a small fix.  If major parts of the cache-tree were left intact\nduring branch switching, these unchanged subdirectories do not have to be\nrehashed in write-tree when making this final commit.\n\n\n\n\n"},{"id":"71350","messageId":"alpine.LSU.1.00.0803071507510.19395@racer.site","threadId":"12554","inReplyTo":"8676ea8b0313abfc2e0946f45f636643e28aade8.1204856187.git.torvalds@linux-foundation.org","subject":"Re: [PATCH 4/4] Make 'unpack_trees()' have a separate source and destination index","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-07T14:13:17Z","receivedAt":"2008-03-07T14:13:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 6 Mar 2008, Linus Torvalds wrote:\n\n> We will always unpack into our own internal index, but we will take the\n> source from wherever specified, and we will optionally write the result\n> to a specified index (optionally, because not everybody even _wants_ any\n> result: the index diffing really wants to just walk the tree and index\n> in parallel).\n> \n> This ends up removing a fair number more lines than it adds, for the \n> simple reason that we can now skip all the crud that tried to be \n> oh-so-careful about maintaining our position in the index as we were \n> traversing and modifying it.  Since we don't actually modify the source \n> index any more, we can just update the 'o->pos' pointer without worrying \n> about whether an index entry got removed or replaced or added to.\n\nNice.  Although you could have done better on the account of remove/add \nratio if you did not add empty lines ;-)  But then, you removed a few \ncurly brackets around single lines, too ;-)\n\n> @@ -221,27 +222,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n>  \tif ((opts.dir && !opts.update))\n>  \t\tdie(\"--exclude-per-directory is meaningless unless -u\");\n>  \n> -\tif (opts.prefix) {\n> -\t\tint pfxlen = strlen(opts.prefix);\n> -\t\tint pos;\n> -\t\tif (opts.prefix[pfxlen-1] != '/')\n> -\t\t\tdie(\"prefix must end with /\");\n> -\t\tif (stage != 2)\n> -\t\t\tdie(\"binding merge takes only one tree\");\n> -\t\tpos = cache_name_pos(opts.prefix, pfxlen);\n> -\t\tif (0 <= pos)\n> -\t\t\tdie(\"corrupt index file\");\n> -\t\tpos = -pos-1;\n> -\t\tif (pos < active_nr &&\n> -\t\t    !strncmp(active_cache[pos]->name, opts.prefix, pfxlen))\n> -\t\t\tdie(\"subdirectory '%s' already exists.\", opts.prefix);\n> -\t\tpos = cache_name_pos(opts.prefix, pfxlen-1);\n> -\t\tif (0 <= pos)\n> -\t\t\tdie(\"file '%.*s' already exists.\",\n> -\t\t\t\t\tpfxlen-1, opts.prefix);\n> -\t\topts.pos = -1 - pos;\n> -\t}\n> -\n\nWas the wholesale removal intentional?  I think there are a few sanity \nchecks, and I did not see the checks moved to somewhere else.  But then, \nthere could be redundant checks somewhere else that I missed.\n\n> @@ -360,7 +366,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>  \tif (o->trivial_merges_only && o->nontrivial_merge)\n>  \t\treturn unpack_failed(o, \"Merge requires file-level merging\");\n>  \n> +\to->src_index = NULL;\n>  \tcheck_updates(o);\n> +\tif (o->dst_index)\n> +\t\t*o->dst_index = o->result;\n>  \treturn 0;\n>  }\n>  \n\nI wonder if you should discard_index(o->dst_index) if o->src_index == \no->dst_index (before you set it to NULL, of course).\n\nI cannot really comment on the rest, since the whole unpack_trees() logic \nwas too complicated for me.  Maybe I have a chance after this patch.\n\nCiao,\nDscho\n\n"},{"id":"71359","messageId":"alpine.LFD.1.00.0803070903080.12253@woody.linux-foundation.org","threadId":"12554","inReplyTo":"alpine.LSU.1.00.0803071507510.19395@racer.site","subject":"Re: [PATCH 4/4] Make 'unpack_trees()' have a separate source and destination index","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-07T17:32:07Z","receivedAt":"2008-03-07T17:32:07Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 7 Mar 2008, Johannes Schindelin wrote:\n> \n> > @@ -221,27 +222,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n> >  \tif ((opts.dir && !opts.update))\n> >  \t\tdie(\"--exclude-per-directory is meaningless unless -u\");\n> >  \n> > -\tif (opts.prefix) {\n> > -\t\tint pfxlen = strlen(opts.prefix);\n> > -\t\tint pos;\n> > -\t\tif (opts.prefix[pfxlen-1] != '/')\n> > -\t\t\tdie(\"prefix must end with /\");\n> > -\t\tif (stage != 2)\n> > -\t\t\tdie(\"binding merge takes only one tree\");\n> > -\t\tpos = cache_name_pos(opts.prefix, pfxlen);\n> > -\t\tif (0 <= pos)\n> > -\t\t\tdie(\"corrupt index file\");\n> > -\t\tpos = -pos-1;\n> > -\t\tif (pos < active_nr &&\n> > -\t\t    !strncmp(active_cache[pos]->name, opts.prefix, pfxlen))\n> > -\t\t\tdie(\"subdirectory '%s' already exists.\", opts.prefix);\n> > -\t\tpos = cache_name_pos(opts.prefix, pfxlen-1);\n> > -\t\tif (0 <= pos)\n> > -\t\t\tdie(\"file '%.*s' already exists.\",\n> > -\t\t\t\t\tpfxlen-1, opts.prefix);\n> > -\t\topts.pos = -1 - pos;\n> > -\t}\n> > -\n> \n> Was the wholesale removal intentional?  I think there are a few sanity \n> checks, and I did not see the checks moved to somewhere else.  But then, \n> there could be redundant checks somewhere else that I missed.\n\nIt was intentional. The \"bind_merge()\" function already checks for overlap \nin the individual entries, so those sanity checks don't actually buy you \nanything. And those games with \"opt.pos\" actualyl made the new model not \nwork, because it would mean that we'd skip the old entries before that \n\"pos\" entirely (instead of moving them over to the new index).\n\nHowever, you're right to point it out. The old checks are somewhat \ndifferent from the new ones - \"bind_merge()\" will check for overlap in \nindividual entries, but it doesn't disallow merging non-overlapping \nsubdirectories. I thought that extending the functionality would be a \nfeature, but maybe somebody really wants those checks to be there on \npurpose.\n\nSo maybe we could have left a few of those checks. I just thought it was \npretty interesting how we disallowed doing\n\n\t- index:\n\t\tsubdirectory/Makefile\n\n\t- tree:\n\t\t.. anything *but* a 'Makefile' entry ..\n\n\t- git-read-tree --prefix subdirectory/ <tree>\n\nand without the checks it actually works fine (it creates a \"union\" of the \nindex and the incoming tree).\n\nIf somebody doesn't want that, I think they should check up-front (easy \nenough: just test if \"git ls-files subdirectory/\" returns anything), so I \nactually think the sanity checks just remove capabilities rather than add \nanything.\n\n> > @@ -360,7 +366,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n> >  \tif (o->trivial_merges_only && o->nontrivial_merge)\n> >  \t\treturn unpack_failed(o, \"Merge requires file-level merging\");\n> >  \n> > +\to->src_index = NULL;\n> >  \tcheck_updates(o);\n> > +\tif (o->dst_index)\n> > +\t\t*o->dst_index = o->result;\n> >  \treturn 0;\n> >  }\n> >  \n> \n> I wonder if you should discard_index(o->dst_index) if o->src_index == \n> o->dst_index (before you set it to NULL, of course).\n\nNot quite yet. I _really_ wanted to, but we can't do it - there's a few \nusers that want to use \"the_index\" that I was not ready to work out what \nthe right thing to do was.\n\nIn particular, when we do write_entry() of the finished index, that one \ndoes:\n\n    check_updates\n      checkout_entry()\n        write_entry\n          convert_to_working_tree\n            git_checkattr\n              bootstrap_attr_stack\n                read_attr\n                  read_index_data\n                    index_name_pos(&the_index)\n\nand the thing is, if we discard the old \"the_index()\" early (like we \n*should* do), that callchain breaks because the attribute code will use \n\"the_index\" rather than the index we are actually using!\n\nNow, the fact is, I think that's a bug in the attribute code (well, \n\"historical artifact\" from the fact that we always used to use just \n\"the_index\" for everything). It should no longer (in my opinion) just \nblindly use \"the_index\" when we are writing out something else than \n\"the_index\", but hey, that's what it does.\n\nIf you want to play with it, here's a patch that I did *not* post on top \nof the whole series that makes the issue more obvious. That \"memset()\" is \njust to poison any users of the index that isn't finalized yet, to make \nyou get a nice SIGSEGV rather than just inexplicable failures.\n\nAnyway, to fix that thing, we'd have to pass the correct index around all \nthe way, which is definitely worth doing regardless, but it was kind of \nbeyond the aim of _this_ particular patch series.\n\nHere's the patch to get you started :)\n\n\t\tLinus\n\n---\n unpack-trees.c |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 0cdf198..afa9c9d 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -366,6 +366,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tif (o->trivial_merges_only && o->nontrivial_merge)\n \t\treturn unpack_failed(o, \"Merge requires file-level merging\");\n \n+\tif (o->dst_index) {\n+\t\tdiscard_index(o->dst_index);\n+\t\tmemset(o->dst_index, 0x55, sizeof(*o->dst_index));\n+\t}\n \to->src_index = NULL;\n \tcheck_updates(o);\n \tif (o->dst_index)\n"},{"id":"71361","messageId":"alpine.LFD.1.00.0803070932260.12253@woody.linux-foundation.org","threadId":"12554","inReplyTo":"7vwsof3x5n.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/4] Make unpack_trees() do separate source and destination indexes","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-07T17:46:16Z","receivedAt":"2008-03-07T17:46:16Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 6 Mar 2008, Junio C Hamano wrote:\n> \n> What I have been wondering about this series was if we can re-enable\n> cache-tree for more of the unpack_trees() users.\n> \n> Currently, all unpack_trees() users, other than a single-tree read-tree,\n> invalidates the whole cache-tree information, as Daniel's \"pop one, decide\n> what to put back, all in the original index\" had too many manual index\n> manipulations and sprinkling cache_tree_invalidate() call everywhere was\n> too much cluttering of already hard-to-follow codepath.\n\nIt should work as-is, but not optimally.\n\nThe reason I say that is that the result-tree won't have any cache tree \n*at*all* with this series, which is a possible peformance poblem (but \nthere are performance upsides to the series too, so I'm not sure it's all \nthat noticeable). So the \"source\" index will keep its cache-tree \nunmodified, because it simply never got modified in the first place.\n\nOf course, all _current_ users will have src_index == dst_index (except \nfor the \"diff_oneway\" thing that doesn't have a destination at all, \nbecause it doesn't actually want any index manipulation), so for all \ncurrent users this is all pure downside: we throw away the good cache_tree \nfor the incoming index, and we aren't even able to use it for the result.\n\nBut because I envisioned that the people who really *care* will be the \nones that want to have different incoming and outgoing indexes, I didn't \nactually worry about it. Ie all the \"git status\" and \"git commit\" stuff \nwould presumably end up using different indexes, and then they can re-use \nthe (unmodified) source, and that would generally be the one that they \nactually want.\n\nBut I dunno. Maybe we'd actually want to try to re-use the cache_tree from \nthe source some way, if we normally actually want it in the destination.\n\nSo this series really isn't a final thing, but I do think that in the form \nI posted it, it's a step forward in (a) readability/maintainability and \n(b) flexibility. And I think all the really *core* stuff got converted, \nbut now there's a lot of small details hanging around that just don't take \nadvantage of the new capabilities.\n\nI basically tried to keep things bug-for-bug compatible. I also didn't try \nto optimize some things that I know I'll want to eventually.\n\nFor example, since we now create a new \"index_state\" entirely, that means \nthat we may need to re-hash the cache entries in the name hash, and that \nmeans that we now allocate a new one *and*copy*it* rather than just re-use \ncache entries directly.\n\nSo that's the reason for the new add_entry() function that actually \nallocates a new cache_entry. And it actually has room for various very \nobvious optimizations that I did *not* do:\n\n - if the cache_entry comes from a tree (common), just re-use it, since it \n   wasn't hashed in the first place\n - if \"o->dst_index\" is NULL, just don't do anything, because we'll not \n   use the result anyway.\n - if \"o->dst_index == o->src_index\" and the cache_entry doesn't have \n   CE_HASHED set, just re-use it directly.\n\nadn things like that. So there were a lot of small micro-optimizations \nthat I didn't do, because quite frankly, this conversion was really \npainful in the first place (the set of patches I sent out may have \n_looked_ fairly clean, but that's only because I didn't show you the pain \nit was to create them ;)\n\n\t\tLinus\n"},{"id":"71382","messageId":"alpine.LNX.1.00.0803071603170.19665@iabervon.org","threadId":"12554","inReplyTo":"8676ea8b0313abfc2e0946f45f636643e28aade8.1204856187.git.torvalds@linux-foundation.org","subject":"Re: [PATCH 4/4] Make 'unpack_trees()' have a separate source and destination index","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-07T21:11:09Z","receivedAt":"2008-03-07T21:11:09Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 6 Mar 2008, Linus Torvalds wrote:\n\n> We will always unpack into our own internal index, but we will take the\n> source from wherever specified, and we will optionally write the result\n> to a specified index (optionally, because not everybody even _wants_ any\n> result: the index diffing really wants to just walk the tree and index\n> in parallel).\n> \n> This ends up removing a fair number more lines than it adds, for the\n> simple reason that we can now skip all the crud that tried to be\n> oh-so-careful about maintaining our position in the index as we were\n> traversing and modifying it.  Since we don't actually modify the source\n> index any more, we can just update the 'o->pos' pointer without worrying\n> about whether an index entry got removed or replaced or added to.\n\nIt looks to me like it's leaking stuff stored in the index it creates if \nit ends up failing. I'm not entirely sure of the index lifecycle stuff, \nbut it seems like it would be necessary. Aside from that, I think it \nshould be right, although I haven't really gone over the resulting code in \ndetail.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"71384","messageId":"alpine.LFD.1.00.0803071339150.3013@woody.linux-foundation.org","threadId":"12554","inReplyTo":"alpine.LNX.1.00.0803071603170.19665@iabervon.org","subject":"Re: [PATCH 4/4] Make 'unpack_trees()' have a separate source and destination index","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-07T21:48:40Z","receivedAt":"2008-03-07T21:48:40Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 7 Mar 2008, Daniel Barkalow wrote:\n>\n> It looks to me like it's leaking stuff stored in the index it creates if \n> it ends up failing.\n\nYes, adding a \"discard_index(&o->result)\" to the failure path sounds like \na good idea. Patch appended.\n\n> I'm not entirely sure of the index lifecycle stuff, \n> but it seems like it would be necessary.\n\nWell, \"necessary\" may not be the right word, because it's a purely \ninternal index thing, so all that happens is a memory leak. But it's \ncertainly easy to do and correct.\n\nThat said, the *big* memory leak comes from the fact that we leak the \ncache_entry things left and right. That's a very traditional leak: because \nwe used to build up the cache entries by just mapping them directly in \nfrom the index file (and we emulate that in modern times by allocating \nthem from one big array), we can't actually free them one-by-one.\n\nSo doing the \"discard_index()\" will free the hash tables etc, which is \ngood, and it will free the \"istate->alloc\" but that is never set on the \nresult because we don't get the result from the index read. So we don't \nactually free the individual cache entries themselves that got created \nfrom the trees.\n\nThat's not something new, btw. We never did. But some day we should just \nadd a flag to the cache_entry() that it's a \"free one by one\" kind, and \nthen we could/should do it. In the meantime, this one-liner will fix \n*some* of the memory leaks, but not that old traditional one.\n\n\t\tLinus\n\n---\n unpack-trees.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 0cdf198..da68557 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -315,6 +315,7 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \n static int unpack_failed(struct unpack_trees_options *o, const char *message)\n {\n+\tdiscard_index(&o->result);\n \tif (!o->gently) {\n \t\tif (message)\n \t\t\treturn error(message);\n"}]}