{"thread":{"id":"44828","subject":"[PATCH 0/5] refactor unpack-trees","startedAt":"2017-01-06T21:03:42Z","lastAt":"2017-01-09T19:30:28Z","messageCount":9,"participants":["Stefan Beller","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"308891","messageId":"20170106210330.31761-1-sbeller@google.com","threadId":"44828","inReplyTo":"CAGZ79kaM=Uosm7KAvAP-8w59Tyfc7LZiV3ZOr=PZnBgMCzr2AA@mail.gmail.com","subject":"[PATCH 0/5] refactor unpack-trees","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-06T21:03:25Z","receivedAt":"2017-01-06T21:03:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"unpack-trees is a central file needed for the understanding\nof working tree manipulation. To help with the understanding\nrefactor the code to be more readable.\n\nThe first patch was a standalone patch 8 days ago;\nnow incorporated into this series as a v3,\nreducing the scope of the checkout state.\n\nThe second patch removes a single continue statement;\nit needed some digging to explain, but looks trivial.\n\nThe last 3 patches shorten the check_updates function by adding more\nfunctions. If we ever want to parallelize file IO then these smaller\nfunctions would be the scope to do it, keeping the check_updates as\na high level function guiding through the steps what is happening during\na working tree update.\n\nThanks,\nStefan\n\nStefan Beller (5):\n  unpack-trees: move checkout state into check_updates\n  unpack-trees: remove unneeded continue\n  unpack-trees: factor progress setup out of check_updates\n  unpack-trees: factor file removal out of check_updates\n  unpack-trees: factor working tree update out of check_updates\n\n unpack-trees.c | 96 ++++++++++++++++++++++++++++++++++++++--------------------\n 1 file changed, 64 insertions(+), 32 deletions(-)\n\n-- \n2.11.0.31.g919a8d0.dirty\n\n"},{"id":"308892","messageId":"20170106210330.31761-2-sbeller@google.com","threadId":"44828","inReplyTo":"20170106210330.31761-1-sbeller@google.com","subject":"[PATCHv3 1/5] unpack-trees: move checkout state into check_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-06T21:03:26Z","receivedAt":"2017-01-06T21:03:46Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The checkout state was introduced via 16da134b1f9\n(read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to\nrefactor the checkout state was done in b56aa5b268e (unpack-trees: pass\ncheckout state explicitly to check_updates(), 2016-09-13), but we can\ngo even further.\n\nThe `struct checkout state` is not used in unpack_trees apart from\ninitializing it, so move it into the function that makes use of it,\nwhich is `check_updates`.\n\nReviewed-by: René Scharfe <l.s.r@web.de>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n unpack-trees.c | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex bc56195e27..55c75b4d6a 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -237,77 +237,82 @@ static void display_error_msgs(struct unpack_trees_options *o)\n }\n \n /*\n  * Unlink the last component and schedule the leading directories for\n  * removal, such that empty directories get removed.\n  */\n static void unlink_entry(const struct cache_entry *ce)\n {\n \tif (!check_leading_path(ce->name, ce_namelen(ce)))\n \t\treturn;\n \tif (remove_or_warn(ce->ce_mode, ce->name))\n \t\treturn;\n \tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n }\n \n-static int check_updates(struct unpack_trees_options *o,\n-\t\t\t const struct checkout *state)\n+static int check_updates(struct unpack_trees_options *o)\n {\n \tunsigned cnt = 0, total = 0;\n+\tint i, errs = 0;\n+\n \tstruct progress *progress = NULL;\n \tstruct index_state *index = &o->result;\n-\tint i;\n-\tint errs = 0;\n+\tstruct checkout state = CHECKOUT_INIT;\n+\n+\tstate.force = 1;\n+\tstate.quiet = 1;\n+\tstate.refresh_cache = 1;\n+\tstate.istate = index;\n \n \tif (o->update && o->verbose_update) {\n \t\tfor (total = cnt = 0; cnt < index->cache_nr; cnt++) {\n \t\t\tconst struct cache_entry *ce = index->cache[cnt];\n \t\t\tif (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))\n \t\t\t\ttotal++;\n \t\t}\n \n \t\tprogress = start_progress_delay(_(\"Checking out files\"),\n \t\t\t\t\t\ttotal, 50, 1);\n \t\tcnt = 0;\n \t}\n \n \tif (o->update)\n-\t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT, &o->result);\n+\t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT, index);\n \tfor (i = 0; i < index->cache_nr; i++) {\n \t\tconst struct cache_entry *ce = index->cache[i];\n \n \t\tif (ce->ce_flags & CE_WT_REMOVE) {\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tif (o->update && !o->dry_run)\n \t\t\t\tunlink_entry(ce);\n \t\t\tcontinue;\n \t\t}\n \t}\n-\tremove_marked_cache_entries(&o->result);\n+\tremove_marked_cache_entries(index);\n \tremove_scheduled_dirs();\n \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) {\n \t\t\tif (ce->ce_flags & CE_WT_REMOVE)\n \t\t\t\tdie(\"BUG: both update and delete flags are set on %s\",\n \t\t\t\t    ce->name);\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tce->ce_flags &= ~CE_UPDATE;\n \t\t\tif (o->update && !o->dry_run) {\n-\t\t\t\terrs |= checkout_entry(ce, state, NULL);\n+\t\t\t\terrs |= checkout_entry(ce, &state, NULL);\n \t\t\t}\n \t\t}\n \t}\n \tstop_progress(&progress);\n \tif (o->update)\n \t\tgit_attr_set_direction(GIT_ATTR_CHECKIN, NULL);\n \treturn errs != 0;\n }\n \n static int verify_uptodate_sparse(const struct cache_entry *ce,\n \t\t\t\t  struct unpack_trees_options *o);\n static int verify_absent_sparse(const struct cache_entry *ce,\n \t\t\t\tenum unpack_trees_error_types,\n \t\t\t\tstruct unpack_trees_options *o);\n \n@@ -1113,38 +1118,33 @@ static void mark_new_skip_worktree(struct exclude_list *el,\n \n static int verify_absent(const struct cache_entry *,\n \t\t\t enum unpack_trees_error_types,\n \t\t\t struct unpack_trees_options *);\n /*\n  * N-way merge \"len\" trees.  Returns 0 on success, -1 on failure to manipulate the\n  * resulting index, -2 on failure to reflect the changes to the work tree.\n  *\n  * CE_ADDED, CE_UNPACKED and CE_NEW_SKIP_WORKTREE are used internally\n  */\n int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)\n {\n \tint i, ret;\n \tstatic struct cache_entry *dfc;\n \tstruct exclude_list el;\n-\tstruct checkout state = CHECKOUT_INIT;\n \n \tif (len > MAX_UNPACK_TREES)\n \t\tdie(\"unpack_trees takes at most %d trees\", MAX_UNPACK_TREES);\n-\tstate.force = 1;\n-\tstate.quiet = 1;\n-\tstate.refresh_cache = 1;\n-\tstate.istate = &o->result;\n \n \tmemset(&el, 0, sizeof(el));\n \tif (!core_apply_sparse_checkout || !o->update)\n \t\to->skip_sparse_checkout = 1;\n \tif (!o->skip_sparse_checkout) {\n \t\tchar *sparse = git_pathdup(\"info/sparse-checkout\");\n \t\tif (add_excludes_from_file_to_list(sparse, \"\", 0, &el, 0) < 0)\n \t\t\to->skip_sparse_checkout = 1;\n \t\telse\n \t\t\to->el = &el;\n \t\tfree(sparse);\n \t}\n \n \tmemset(&o->result, 0, sizeof(o->result));\n \to->result.initialized = 1;\n@@ -1257,31 +1257,31 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\tif (ret < 0)\n \t\t\tgoto return_failed;\n \t\t/*\n \t\t * Sparse checkout is meant to narrow down checkout area\n \t\t * but it does not make sense to narrow down to empty working\n \t\t * tree. This is usually a mistake in sparse checkout rules.\n \t\t * Do not allow users to do that.\n \t\t */\n \t\tif (o->result.cache_nr && empty_worktree) {\n \t\t\tret = unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n \t\t\tgoto done;\n \t\t}\n \t}\n \n \to->src_index = NULL;\n-\tret = check_updates(o, &state) ? (-2) : 0;\n+\tret = check_updates(o) ? (-2) : 0;\n \tif (o->dst_index) {\n \t\tif (!ret) {\n \t\t\tif (!o->result.cache_tree)\n \t\t\t\to->result.cache_tree = cache_tree();\n \t\t\tif (!cache_tree_fully_valid(o->result.cache_tree))\n \t\t\t\tcache_tree_update(&o->result,\n \t\t\t\t\t\t  WRITE_TREE_SILENT |\n \t\t\t\t\t\t  WRITE_TREE_REPAIR);\n \t\t}\n \t\tdiscard_index(o->dst_index);\n \t\t*o->dst_index = o->result;\n \t} else {\n \t\tdiscard_index(&o->result);\n \t}\n \n-- \n2.11.0.31.g919a8d0.dirty\n\n"},{"id":"308893","messageId":"CAGZ79kaM=Uosm7KAvAP-8w59Tyfc7LZiV3ZOr=PZnBgMCzr2AA@mail.gmail.com","threadId":"44828","inReplyTo":"xmqq8tqv6051.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2] unpack-trees: move checkout state into check_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-06T21:03:10Z","receivedAt":"2017-01-06T21:04:10Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Dec 31, 2016 at 5:26 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Signed-off-by: Stefan Beller <sbeller@google.com>\n>> ---\n>\n> I'd add René's Reviewed-by: here.\n\ndone\n\n>\n> I think moving heavier and initialized variables earlier and more\n> lightweight and ephemeral ones like \"i\" later does make it easier to\n> follow.  \"errs\" has the significance and the lifetime similar to\n> cnt/total, and logically should be higher, though.  It is not a big\n> enough deal to reroll (but as your futzing of the variable definition\n> order was not a big enough deal to do in this patch, either, so...).\n\nI will send out a series, that is based on this patch shortly;\nas I fuzzed again with the small variables, that series doesn't\napply on this version but the version to be sent out shortly.\n\n>\n> Queued.  Thanks.\n\nplease replace with the following series.\n"},{"id":"308894","messageId":"20170106210330.31761-6-sbeller@google.com","threadId":"44828","inReplyTo":"20170106210330.31761-1-sbeller@google.com","subject":"[PATCH 5/5] unpack-trees: factor working tree update out of check_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-06T21:03:30Z","receivedAt":"2017-01-06T21:04:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This makes check_updates shorter and easier to understand.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n unpack-trees.c | 40 ++++++++++++++++++++++++++--------------\n 1 file changed, 26 insertions(+), 14 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex b954ec1233..b40c069b1b 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -275,67 +275,79 @@ static struct progress *get_progress(struct unpack_trees_options *o)\n \tstruct index_state *index = &o->result;\n \n \tif (!o->update || !o->verbose_update)\n \t\treturn NULL;\n \n \tfor (; cnt < index->cache_nr; cnt++) {\n \t\tconst struct cache_entry *ce = index->cache[cnt];\n \t\tif (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))\n \t\t\ttotal++;\n \t}\n \n \treturn start_progress_delay(_(\"Checking out files\"),\n \t\t\t\t    total, 50, 1);\n }\n \n-static int check_updates(struct unpack_trees_options *o)\n+static int update_working_tree_files(struct unpack_trees_options *o,\n+\t\t\t\t     struct progress *progress,\n+\t\t\t\t     unsigned start_cnt)\n {\n-\tunsigned cnt = 0;\n+\tunsigned cnt = start_cnt;\n \tint i, errs = 0;\n \n-\tstruct progress *progress = NULL;\n \tstruct index_state *index = &o->result;\n \tstruct checkout state = CHECKOUT_INIT;\n \n \tstate.force = 1;\n \tstate.quiet = 1;\n \tstate.refresh_cache = 1;\n \tstate.istate = index;\n \n-\tprogress = get_progress(o);\n-\n-\tif (o->update)\n-\t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT, index);\n-\n-\tcnt = remove_workingtree_files(o, progress);\n-\tremove_marked_cache_entries(index);\n-\tremove_scheduled_dirs();\n-\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) {\n \t\t\tif (ce->ce_flags & CE_WT_REMOVE)\n \t\t\t\tdie(\"BUG: both update and delete flags are set on %s\",\n \t\t\t\t    ce->name);\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tce->ce_flags &= ~CE_UPDATE;\n-\t\t\tif (o->update && !o->dry_run) {\n+\t\t\tif (o->update && !o->dry_run)\n \t\t\t\terrs |= checkout_entry(ce, &state, NULL);\n-\t\t\t}\n \t\t}\n \t}\n+\n+\treturn errs;\n+}\n+\n+static int check_updates(struct unpack_trees_options *o)\n+{\n+\tstruct progress *progress = NULL;\n+\tstruct index_state *index = &o->result;\n+\tint errs;\n+\tunsigned total_removed;\n+\n+\tprogress = get_progress(o);\n+\n+\tif (o->update)\n+\t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT, index);\n+\n+\ttotal_removed = remove_workingtree_files(o, progress);\n+\tremove_marked_cache_entries(index);\n+\tremove_scheduled_dirs();\n+\terrs = update_working_tree_files(o, progress, total_removed);\n+\n \tstop_progress(&progress);\n \tif (o->update)\n \t\tgit_attr_set_direction(GIT_ATTR_CHECKIN, NULL);\n \treturn errs != 0;\n }\n \n static int verify_uptodate_sparse(const struct cache_entry *ce,\n \t\t\t\t  struct unpack_trees_options *o);\n static int verify_absent_sparse(const struct cache_entry *ce,\n \t\t\t\tenum unpack_trees_error_types,\n \t\t\t\tstruct unpack_trees_options *o);\n \n static int apply_sparse_checkout(struct index_state *istate,\n \t\t\t\t struct cache_entry *ce,\n \t\t\t\t struct unpack_trees_options *o)\n-- \n2.11.0.31.g919a8d0.dirty\n\n"},{"id":"308895","messageId":"20170106210330.31761-5-sbeller@google.com","threadId":"44828","inReplyTo":"20170106210330.31761-1-sbeller@google.com","subject":"[PATCH 4/5] unpack-trees: factor file removal out of check_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-06T21:03:29Z","receivedAt":"2017-01-06T21:04:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This makes check_updates shorter and easier to understand.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n unpack-trees.c | 29 +++++++++++++++++++++--------\n 1 file changed, 21 insertions(+), 8 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 971d091fd0..b954ec1233 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -237,30 +237,50 @@ static void display_error_msgs(struct unpack_trees_options *o)\n }\n \n /*\n  * Unlink the last component and schedule the leading directories for\n  * removal, such that empty directories get removed.\n  */\n static void unlink_entry(const struct cache_entry *ce)\n {\n \tif (!check_leading_path(ce->name, ce_namelen(ce)))\n \t\treturn;\n \tif (remove_or_warn(ce->ce_mode, ce->name))\n \t\treturn;\n \tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n }\n \n+static int remove_workingtree_files(struct unpack_trees_options *o,\n+\t\t\t\t    struct progress *progress)\n+{\n+\tint i;\n+\tunsigned cnt = 0;\n+\tstruct index_state *index = &o->result;\n+\n+\tfor (i = 0; i < index->cache_nr; i++) {\n+\t\tconst struct cache_entry *ce = index->cache[i];\n+\n+\t\tif (ce->ce_flags & CE_WT_REMOVE) {\n+\t\t\tdisplay_progress(progress, ++cnt);\n+\t\t\tif (o->update && !o->dry_run)\n+\t\t\t\tunlink_entry(ce);\n+\t\t}\n+\t}\n+\n+\treturn cnt;\n+}\n+\n static struct progress *get_progress(struct unpack_trees_options *o)\n {\n \tunsigned cnt = 0, total = 0;\n \tstruct index_state *index = &o->result;\n \n \tif (!o->update || !o->verbose_update)\n \t\treturn NULL;\n \n \tfor (; cnt < index->cache_nr; cnt++) {\n \t\tconst struct cache_entry *ce = index->cache[cnt];\n \t\tif (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))\n \t\t\ttotal++;\n \t}\n \n \treturn start_progress_delay(_(\"Checking out files\"),\n@@ -273,39 +293,32 @@ static int check_updates(struct unpack_trees_options *o)\n \tint i, errs = 0;\n \n \tstruct progress *progress = NULL;\n \tstruct index_state *index = &o->result;\n \tstruct checkout state = CHECKOUT_INIT;\n \n \tstate.force = 1;\n \tstate.quiet = 1;\n \tstate.refresh_cache = 1;\n \tstate.istate = index;\n \n \tprogress = get_progress(o);\n \n \tif (o->update)\n \t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT, index);\n-\tfor (i = 0; i < index->cache_nr; i++) {\n-\t\tconst struct cache_entry *ce = index->cache[i];\n \n-\t\tif (ce->ce_flags & CE_WT_REMOVE) {\n-\t\t\tdisplay_progress(progress, ++cnt);\n-\t\t\tif (o->update && !o->dry_run)\n-\t\t\t\tunlink_entry(ce);\n-\t\t}\n-\t}\n+\tcnt = remove_workingtree_files(o, progress);\n \tremove_marked_cache_entries(index);\n \tremove_scheduled_dirs();\n \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) {\n \t\t\tif (ce->ce_flags & CE_WT_REMOVE)\n \t\t\t\tdie(\"BUG: both update and delete flags are set on %s\",\n \t\t\t\t    ce->name);\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tce->ce_flags &= ~CE_UPDATE;\n \t\t\tif (o->update && !o->dry_run) {\n \t\t\t\terrs |= checkout_entry(ce, &state, NULL);\n \t\t\t}\n-- \n2.11.0.31.g919a8d0.dirty\n\n"},{"id":"308896","messageId":"20170106210330.31761-4-sbeller@google.com","threadId":"44828","inReplyTo":"20170106210330.31761-1-sbeller@google.com","subject":"[PATCH 3/5] unpack-trees: factor progress setup out of check_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-06T21:03:28Z","receivedAt":"2017-01-06T21:04:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This makes check_updates shorter and easier to understand.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n unpack-trees.c | 32 ++++++++++++++++++++------------\n 1 file changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex f16ef14294..971d091fd0 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -237,55 +237,63 @@ static void display_error_msgs(struct unpack_trees_options *o)\n }\n \n /*\n  * Unlink the last component and schedule the leading directories for\n  * removal, such that empty directories get removed.\n  */\n static void unlink_entry(const struct cache_entry *ce)\n {\n \tif (!check_leading_path(ce->name, ce_namelen(ce)))\n \t\treturn;\n \tif (remove_or_warn(ce->ce_mode, ce->name))\n \t\treturn;\n \tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n }\n \n-static int check_updates(struct unpack_trees_options *o)\n+static struct progress *get_progress(struct unpack_trees_options *o)\n {\n \tunsigned cnt = 0, total = 0;\n+\tstruct index_state *index = &o->result;\n+\n+\tif (!o->update || !o->verbose_update)\n+\t\treturn NULL;\n+\n+\tfor (; cnt < index->cache_nr; cnt++) {\n+\t\tconst struct cache_entry *ce = index->cache[cnt];\n+\t\tif (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))\n+\t\t\ttotal++;\n+\t}\n+\n+\treturn start_progress_delay(_(\"Checking out files\"),\n+\t\t\t\t    total, 50, 1);\n+}\n+\n+static int check_updates(struct unpack_trees_options *o)\n+{\n+\tunsigned cnt = 0;\n \tint i, errs = 0;\n \n \tstruct progress *progress = NULL;\n \tstruct index_state *index = &o->result;\n \tstruct checkout state = CHECKOUT_INIT;\n \n \tstate.force = 1;\n \tstate.quiet = 1;\n \tstate.refresh_cache = 1;\n \tstate.istate = index;\n \n-\tif (o->update && o->verbose_update) {\n-\t\tfor (total = cnt = 0; cnt < index->cache_nr; cnt++) {\n-\t\t\tconst struct cache_entry *ce = index->cache[cnt];\n-\t\t\tif (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))\n-\t\t\t\ttotal++;\n-\t\t}\n-\n-\t\tprogress = start_progress_delay(_(\"Checking out files\"),\n-\t\t\t\t\t\ttotal, 50, 1);\n-\t\tcnt = 0;\n-\t}\n+\tprogress = get_progress(o);\n \n \tif (o->update)\n \t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT, index);\n \tfor (i = 0; i < index->cache_nr; i++) {\n \t\tconst struct cache_entry *ce = index->cache[i];\n \n \t\tif (ce->ce_flags & CE_WT_REMOVE) {\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tif (o->update && !o->dry_run)\n \t\t\t\tunlink_entry(ce);\n \t\t}\n \t}\n \tremove_marked_cache_entries(index);\n \tremove_scheduled_dirs();\n \n-- \n2.11.0.31.g919a8d0.dirty\n\n"},{"id":"308897","messageId":"20170106210330.31761-3-sbeller@google.com","threadId":"44828","inReplyTo":"20170106210330.31761-1-sbeller@google.com","subject":"[PATCH 2/5] unpack-trees: remove unneeded continue","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-06T21:03:27Z","receivedAt":"2017-01-06T21:04:32Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The continue is the last statement in the loop, so not needed.\nThis situation arose in 700e66d66 (2010-07-30, unpack-trees: let\nread-tree -u remove index entries outside sparse area) when statements\nafter the continue were removed.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n unpack-trees.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 55c75b4d6a..f16ef14294 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -272,31 +272,30 @@ static int check_updates(struct unpack_trees_options *o)\n \n \t\tprogress = start_progress_delay(_(\"Checking out files\"),\n \t\t\t\t\t\ttotal, 50, 1);\n \t\tcnt = 0;\n \t}\n \n \tif (o->update)\n \t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT, index);\n \tfor (i = 0; i < index->cache_nr; i++) {\n \t\tconst struct cache_entry *ce = index->cache[i];\n \n \t\tif (ce->ce_flags & CE_WT_REMOVE) {\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tif (o->update && !o->dry_run)\n \t\t\t\tunlink_entry(ce);\n-\t\t\tcontinue;\n \t\t}\n \t}\n \tremove_marked_cache_entries(index);\n \tremove_scheduled_dirs();\n \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) {\n \t\t\tif (ce->ce_flags & CE_WT_REMOVE)\n \t\t\t\tdie(\"BUG: both update and delete flags are set on %s\",\n \t\t\t\t    ce->name);\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tce->ce_flags &= ~CE_UPDATE;\n \t\t\tif (o->update && !o->dry_run) {\n-- \n2.11.0.31.g919a8d0.dirty\n\n"},{"id":"308915","messageId":"20170107013602.act7aouuzgzpwb54@sigill.intra.peff.net","threadId":"44828","inReplyTo":"20170106210330.31761-5-sbeller@google.com","subject":"Re: [PATCH 4/5] unpack-trees: factor file removal out of check_updates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-07T01:36:02Z","receivedAt":"2017-01-07T01:36:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 06, 2017 at 01:03:29PM -0800, Stefan Beller wrote:\n\n> +static int remove_workingtree_files(struct unpack_trees_options *o,\n> +\t\t\t\t    struct progress *progress)\n> +{\n> +\tint i;\n> +\tunsigned cnt = 0;\n> +\tstruct index_state *index = &o->result;\n> +\n> +\tfor (i = 0; i < index->cache_nr; i++) {\n> +\t\tconst struct cache_entry *ce = index->cache[i];\n> +\n> +\t\tif (ce->ce_flags & CE_WT_REMOVE) {\n> +\t\t\tdisplay_progress(progress, ++cnt);\n> +\t\t\tif (o->update && !o->dry_run)\n> +\t\t\t\tunlink_entry(ce);\n> +\t\t}\n> +\t}\n> +\n> +\treturn cnt;\n> +}\n\n\"cnt\" is unsigned here, as it is in the caller. Should the return value\nmatch?\n\n-Peff\n"},{"id":"309056","messageId":"CAGZ79kbZz5s5TdudGcOADDxYShTAm5sTCHRo81SSCc13MoF7WA@mail.gmail.com","threadId":"44828","inReplyTo":"20170107013602.act7aouuzgzpwb54@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] unpack-trees: factor file removal out of check_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-09T19:30:22Z","receivedAt":"2017-01-09T19:30:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jan 6, 2017 at 5:36 PM, Jeff King <peff@peff.net> wrote:\n\n>> +static int remove_workingtree_files(struct unpack_trees_options *o,\n>> +     unsigned cnt = 0;\n\n>\n> \"cnt\" is unsigned here, as it is in the caller. Should the return value\n> match?\n\nYes, obviously. :/\n\nThanks for catching,\nStefan\n"}]}