{"thread":{"id":"11738","subject":"[PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","startedAt":"2008-01-25T23:24:52Z","lastAt":"2008-01-26T00:54:18Z","messageCount":3,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"66605","messageId":"alpine.LNX.1.00.0801251820330.13593@iabervon.org","threadId":"11738","inReplyTo":null,"subject":"[PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-01-25T23:24:52Z","receivedAt":"2008-01-25T23:24:52Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"Way back in read-tree.c, we used a mode 0 cache entry to indicate that\nan entry had been deleted, so that the update code would remove the\nworking tree file, and we would just skip it when writing out the\nindex file afterward.\n\nThese days, unpack_trees is a library function, and it is still\nleaving these entries in the active cache. Furthermore, unpack_trees\ndoesn't correctly ignore those entries, and who knows what other code\nwouldn't expect them to be there, but just isn't yet called after a\ncall to unpack_trees. To avoid having other code trip over these\nentries, have check_updates() remove them after it removes the working\ntree files.\n\nWhile we're at it, make the loop more obvious and skip passing in\nglobals to a static function with only one caller.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\nIt's likely that this should get merged in some way with Linus's series to \nhandle the in-core index differently in general. But this is a simple \nsolution to the problem that's sufficient for this particular series.\n\n unpack-trees.c |   16 +++++++++-------\n 1 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex d6bae1b..ac919c7 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -288,17 +288,17 @@ static void unlink_entry(char *name, char *last_symlink)\n }\n \n static struct checkout state;\n-static void check_updates(struct cache_entry **src, int nr,\n-\t\t\tstruct unpack_trees_options *o)\n+static void check_updates(struct unpack_trees_options *o)\n {\n \tunsigned short mask = htons(CE_UPDATE);\n \tunsigned cnt = 0, total = 0;\n \tstruct progress *progress = NULL;\n \tchar last_symlink[PATH_MAX];\n+\tint i;\n \n \tif (o->update && o->verbose_update) {\n-\t\tfor (total = cnt = 0; cnt < nr; cnt++) {\n-\t\t\tstruct cache_entry *ce = src[cnt];\n+\t\tfor (total = cnt = 0; cnt < active_nr; cnt++) {\n+\t\t\tstruct cache_entry *ce = active_cache[cnt];\n \t\t\tif (!ce->ce_mode || ce->ce_flags & mask)\n \t\t\t\ttotal++;\n \t\t}\n@@ -309,14 +309,16 @@ static void check_updates(struct cache_entry **src, int nr,\n \t}\n \n \t*last_symlink = '\\0';\n-\twhile (nr--) {\n-\t\tstruct cache_entry *ce = *src++;\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n \n \t\tif (!ce->ce_mode || ce->ce_flags & mask)\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\tif (!ce->ce_mode) {\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\ti--;\n \t\t\tcontinue;\n \t\t}\n \t\tif (ce->ce_flags & mask) {\n@@ -375,7 +377,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t\terror(\"Merge requires file-level merging\");\n \t}\n \n-\tcheck_updates(active_cache, active_nr, o);\n+\tcheck_updates(o);\n \treturn 0;\n }\n \n-- \n1.5.4.rc3.4.g16335\n"},{"id":"66614","messageId":"7v63xh5uo0.fsf@gitster.siamese.dyndns.org","threadId":"11738","inReplyTo":"alpine.LNX.1.00.0801251820330.13593@iabervon.org","subject":"Re: [PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-25T23:53:35Z","receivedAt":"2008-01-25T23:53:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> It's likely that this should get merged in some way with Linus's series to \n> handle the in-core index differently in general. But this is a simple \n> solution to the problem that's sufficient for this particular series.\n\nJust to let you know, the early parts of lt/in-core-index series\nare already in 'next', and it will be one of the first topics to\ngraduate to 'master' post 1.5.4.\n\nI think we both understand that this round is sent purely for\ndiscussion and I won't be picking them up right now nor later\nbefore the \"for possible inclusion\" resend you will be making\npost 1.5.4, so it should not matter too much which exact version\nthis round is based on.\n\nIt however may be worthwhile for you to plan rebasing your\nfuture rounds on top of 077c48df8a72b046a2f562fedffa1c3d3a73a4e2\n(read-cache.c: fix a couple more CE_REMOVE conversion).\n\nThanks.\n"},{"id":"66618","messageId":"alpine.LNX.1.00.0801251939570.13593@iabervon.org","threadId":"11738","inReplyTo":"7v63xh5uo0.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-01-26T00:54:18Z","receivedAt":"2008-01-26T00:54:18Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 25 Jan 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > It's likely that this should get merged in some way with Linus's series to \n> > handle the in-core index differently in general. But this is a simple \n> > solution to the problem that's sufficient for this particular series.\n> \n> Just to let you know, the early parts of lt/in-core-index series\n> are already in 'next', and it will be one of the first topics to\n> graduate to 'master' post 1.5.4.\n> \n> I think we both understand that this round is sent purely for\n> discussion and I won't be picking them up right now nor later\n> before the \"for possible inclusion\" resend you will be making\n> post 1.5.4, so it should not matter too much which exact version\n> this round is based on.\n> \n> It however may be worthwhile for you to plan rebasing your\n> future rounds on top of 077c48df8a72b046a2f562fedffa1c3d3a73a4e2\n> (read-cache.c: fix a couple more CE_REMOVE conversion).\n\nI've now got a version rebased on that. The conflicts are pretty \nstraightforward, and the tests all pass (once I fix the unrelated bug in \n9/9 wrt the lockfile API change).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}