{"thread":{"id":"11742","subject":"[PATCH 1/9] Allow callers of unpack_trees() to handle failure","startedAt":"2008-01-25T23:24:37Z","lastAt":"2008-01-25T23:24:37Z","messageCount":1,"participants":["Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"66609","messageId":"alpine.LNX.1.00.0801251819091.13593@iabervon.org","threadId":"11742","inReplyTo":null,"subject":"[PATCH 1/9] Allow callers of unpack_trees() to handle failure","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-01-25T23:24:37Z","receivedAt":"2008-01-25T23:24:37Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"Return an error from unpack_trees() instead of calling die(), and exit\nwith an error in read-tree. The merge function can return negative to\nabort.\n\nThis will be used in builtin-checkout -m.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n builtin-read-tree.c |    3 +-\n unpack-trees.c      |   85 ++++++++++++++++++++++++++++----------------------\n 2 files changed, 50 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex c0ea034..e4c6317 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -269,7 +269,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\tparse_tree(tree);\n \t\tinit_tree_desc(t+i, tree->buffer, tree->size);\n \t}\n-\tunpack_trees(nr_trees, t, &opts);\n+\tif (unpack_trees(nr_trees, t, &opts))\n+\t\treturn 128;\n \n \t/*\n \t * When reading only one tree (either the most basic form,\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex aa2513e..a56e1c9 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -219,6 +219,8 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,\n \t\t\t\t}\n #endif\n \t\t\t\tret = o->fn(src, o, remove);\n+\t\t\t\tif (ret < 0)\n+\t\t\t\t\treturn ret;\n \n #if DBRT_DEBUG > 1\n \t\t\t\tprintf(\"Added %d entries\\n\", ret);\n@@ -360,7 +362,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t}\n \n \tif (o->trivial_merges_only && o->nontrivial_merge)\n-\t\tdie(\"Merge requires file-level merging\");\n+\t\treturn error(\"Merge requires file-level merging\");\n \n \tcheck_updates(active_cache, active_nr, o);\n \treturn 0;\n@@ -368,10 +370,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \n /* Here come the merge functions */\n \n-static void reject_merge(struct cache_entry *ce)\n+static int reject_merge(struct cache_entry *ce)\n {\n-\tdie(\"Entry '%s' would be overwritten by merge. Cannot merge.\",\n-\t    ce->name);\n+\treturn error(\"Entry '%s' would be overwritten by merge. Cannot merge.\",\n+\t\t     ce->name);\n }\n \n static int same(struct cache_entry *a, struct cache_entry *b)\n@@ -389,18 +391,18 @@ static int same(struct cache_entry *a, struct cache_entry *b)\n  * When a CE gets turned into an unmerged entry, we\n  * want it to be up-to-date\n  */\n-static void verify_uptodate(struct cache_entry *ce,\n+static int verify_uptodate(struct cache_entry *ce,\n \t\tstruct unpack_trees_options *o)\n {\n \tstruct stat st;\n \n \tif (o->index_only || o->reset)\n-\t\treturn;\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\tif (!changed)\n-\t\t\treturn;\n+\t\t\treturn 0;\n \t\t/*\n \t\t * NEEDSWORK: the current default policy is to allow\n \t\t * submodule to be out of sync wrt the supermodule\n@@ -409,12 +411,12 @@ static void verify_uptodate(struct cache_entry *ce,\n \t\t * checked out.\n \t\t */\n \t\tif (S_ISGITLINK(ntohl(ce->ce_mode)))\n-\t\t\treturn;\n+\t\t\treturn 0;\n \t\terrno = 0;\n \t}\n \tif (errno == ENOENT)\n-\t\treturn;\n-\tdie(\"Entry '%s' not uptodate. Cannot merge.\", ce->name);\n+\t\treturn 0;\n+\treturn error(\"Entry '%s' not uptodate. Cannot merge.\", ce->name);\n }\n \n static void invalidate_ce_path(struct cache_entry *ce)\n@@ -480,7 +482,8 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \t\t * ce->name is an entry in the subdirectory.\n \t\t */\n \t\tif (!ce_stage(ce)) {\n-\t\t\tverify_uptodate(ce, o);\n+\t\t\tif (verify_uptodate(ce, o))\n+\t\t\t\treturn -1;\n \t\t\tce->ce_mode = 0;\n \t\t}\n \t\tcnt++;\n@@ -499,8 +502,8 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \t\td.exclude_per_dir = o->dir->exclude_per_dir;\n \ti = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);\n \tif (i)\n-\t\tdie(\"Updating '%s' would lose untracked files in it\",\n-\t\t    ce->name);\n+\t\treturn error(\"Updating '%s' would lose untracked files in it\",\n+\t\t\t     ce->name);\n \tfree(pathbuf);\n \treturn cnt;\n }\n@@ -509,16 +512,16 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n  * We do not want to remove or overwrite a working tree file that\n  * is not tracked, unless it is ignored.\n  */\n-static void verify_absent(struct cache_entry *ce, const char *action,\n-\t\tstruct unpack_trees_options *o)\n+static int verify_absent(struct cache_entry *ce, const char *action,\n+\t\t\t struct unpack_trees_options *o)\n {\n \tstruct stat st;\n \n \tif (o->index_only || o->reset || !o->update)\n-\t\treturn;\n+\t\treturn 0;\n \n \tif (has_symlink_leading_path(ce->name, NULL))\n-\t\treturn;\n+\t\treturn 0;\n \n \tif (!lstat(ce->name, &st)) {\n \t\tint cnt;\n@@ -528,7 +531,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,\n \t\t\t * ce->name is explicitly excluded, so it is Ok to\n \t\t\t * overwrite it.\n \t\t\t */\n-\t\t\treturn;\n+\t\t\treturn 0;\n \t\tif (S_ISDIR(st.st_mode)) {\n \t\t\t/*\n \t\t\t * We are checking out path \"foo\" and\n@@ -557,7 +560,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,\n \t\t\t * deleted entries here.\n \t\t\t */\n \t\t\to->pos += cnt;\n-\t\t\treturn;\n+\t\t\treturn 0;\n \t\t}\n \n \t\t/*\n@@ -569,12 +572,13 @@ static void verify_absent(struct cache_entry *ce, const char *action,\n \t\tif (0 <= cnt) {\n \t\t\tstruct cache_entry *ce = active_cache[cnt];\n \t\t\tif (!ce_stage(ce) && !ce->ce_mode)\n-\t\t\t\treturn;\n+\t\t\t\treturn 0;\n \t\t}\n \n-\t\tdie(\"Untracked working tree file '%s' \"\n-\t\t    \"would be %s by merge.\", ce->name, action);\n+\t\treturn error(\"Untracked working tree file '%s' \"\n+\t\t\t     \"would be %s by merge.\", ce->name, action);\n \t}\n+\treturn 0;\n }\n \n static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n@@ -592,12 +596,14 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\tif (same(old, merge)) {\n \t\t\tmemcpy(merge, old, offsetof(struct cache_entry, name));\n \t\t} else {\n-\t\t\tverify_uptodate(old, o);\n+\t\t\tif (verify_uptodate(old, o))\n+\t\t\t\treturn -1;\n \t\t\tinvalidate_ce_path(old);\n \t\t}\n \t}\n \telse {\n-\t\tverify_absent(merge, \"overwritten\", o);\n+\t\tif (verify_absent(merge, \"overwritten\", o))\n+\t\t\treturn -1;\n \t\tinvalidate_ce_path(merge);\n \t}\n \n@@ -609,10 +615,12 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\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\tverify_uptodate(old, o);\n-\telse\n-\t\tverify_absent(ce, \"removed\", o);\n+\tif (old) {\n+\t\tif (verify_uptodate(old, o))\n+\t\t\treturn -1;\n+\t} else\n+\t\tif (verify_absent(ce, \"removed\", o))\n+\t\t\treturn -1;\n \tce->ce_mode = 0;\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \tinvalidate_ce_path(ce);\n@@ -700,7 +708,7 @@ int threeway_merge(struct cache_entry **stages,\n \t/* #14, #14ALT, #2ALT */\n \tif (remote && !df_conflict_head && head_match && !remote_match) {\n \t\tif (index && !same(index, remote) && !same(index, head))\n-\t\t\treject_merge(index);\n+\t\t\treturn reject_merge(index);\n \t\treturn merged_entry(remote, index, o);\n \t}\n \t/*\n@@ -708,7 +716,7 @@ int threeway_merge(struct cache_entry **stages,\n \t * make sure that it matches head.\n \t */\n \tif (index && !same(index, head)) {\n-\t\treject_merge(index);\n+\t\treturn reject_merge(index);\n \t}\n \n \tif (head) {\n@@ -759,8 +767,10 @@ int threeway_merge(struct cache_entry **stages,\n \t\t\tremove_entry(remove);\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\t\tverify_absent(ce, \"removed\", o);\n+\t\t\telse if (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 \t\t\treturn 0;\n \t\t}\n \t\t/*\n@@ -776,7 +786,8 @@ int threeway_merge(struct cache_entry **stages,\n \t * conflict resolution files.\n \t */\n \tif (index) {\n-\t\tverify_uptodate(index, o);\n+\t\tif (verify_uptodate(index, o))\n+\t\t\treturn -1;\n \t}\n \n \tremove_entry(remove);\n@@ -856,11 +867,11 @@ int twoway_merge(struct cache_entry **src,\n \t\t\t/* all other failures */\n \t\t\tremove_entry(remove);\n \t\t\tif (oldtree)\n-\t\t\t\treject_merge(oldtree);\n+\t\t\t\treturn reject_merge(oldtree);\n \t\t\tif (current)\n-\t\t\t\treject_merge(current);\n+\t\t\t\treturn reject_merge(current);\n \t\t\tif (newtree)\n-\t\t\t\treject_merge(newtree);\n+\t\t\t\treturn reject_merge(newtree);\n \t\t\treturn -1;\n \t\t}\n \t}\n@@ -887,7 +898,7 @@ int bind_merge(struct cache_entry **src,\n \t\treturn error(\"Cannot do a bind merge of %d trees\\n\",\n \t\t\t     o->merge_size);\n \tif (a && old)\n-\t\tdie(\"Entry '%s' overlaps.  Cannot bind.\", a->name);\n+\t\treturn error(\"Entry '%s' overlaps.  Cannot bind.\", a->name);\n \tif (!a)\n \t\treturn keep_entry(old, o);\n \telse\n-- \n1.5.4.rc3.4.g16335\n"}]}