From 84a5750bc5e5211bd06857e4edb08e332d0e7adf Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 28 May 2008 14:54:02 -0700 Subject: [PATCH 1/4] checkout: make reset_clean_to_new() not die by itself Instead, have its error percolate up through the callchain and let it be the exit status of the main command. No semantic changes yet. Signed-off-by: Junio C Hamano --- builtin-checkout.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/builtin-checkout.c b/builtin-checkout.c index 00dc8cafd8..cc97724c87 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -172,7 +172,7 @@ static int reset_to_new(struct tree *tree, int quiet) return 0; } -static void reset_clean_to_new(struct tree *tree, int quiet) +static int reset_clean_to_new(struct tree *tree, int quiet) { struct unpack_trees_options opts; struct tree_desc tree_desc; @@ -189,7 +189,8 @@ static void reset_clean_to_new(struct tree *tree, int quiet) parse_tree(tree); init_tree_desc(&tree_desc, tree->buffer, tree->size); if (unpack_trees(1, &tree_desc, &opts)) - exit(128); + return 128; + return 0; } struct checkout_opts { @@ -295,7 +296,9 @@ static int merge_working_tree(struct checkout_opts *opts, return ret; merge_trees(new->commit->tree, work, old->commit->tree, new->name, "local", &result); - reset_clean_to_new(new->commit->tree, opts->quiet); + ret = reset_clean_to_new(new->commit->tree, opts->quiet); + if (ret) + return ret; } } From 6286a08db3d1e718f4be6959d0f380215026800b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 28 May 2008 14:59:40 -0700 Subject: [PATCH 2/4] checkout: consolidate reset_{to_new,clean_to_new}() These two were very similar functions with only tiny bit of difference. Signed-off-by: Junio C Hamano --- builtin-checkout.c | 70 +++++++++++++++++----------------------------- 1 file changed, 25 insertions(+), 45 deletions(-) diff --git a/builtin-checkout.c b/builtin-checkout.c index cc97724c87..9af5197b60 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -151,48 +151,6 @@ static void describe_detached_head(char *msg, struct commit *commit) strbuf_release(&sb); } -static int reset_to_new(struct tree *tree, int quiet) -{ - struct unpack_trees_options opts; - struct tree_desc tree_desc; - - memset(&opts, 0, sizeof(opts)); - opts.head_idx = -1; - opts.update = 1; - opts.reset = 1; - opts.merge = 1; - opts.fn = oneway_merge; - opts.verbose_update = !quiet; - opts.src_index = &the_index; - opts.dst_index = &the_index; - parse_tree(tree); - init_tree_desc(&tree_desc, tree->buffer, tree->size); - if (unpack_trees(1, &tree_desc, &opts)) - return 128; - return 0; -} - -static int reset_clean_to_new(struct tree *tree, int quiet) -{ - struct unpack_trees_options opts; - struct tree_desc tree_desc; - - memset(&opts, 0, sizeof(opts)); - opts.head_idx = -1; - opts.skip_unmerged = 1; - opts.reset = 1; - opts.merge = 1; - opts.fn = oneway_merge; - opts.verbose_update = !quiet; - opts.src_index = &the_index; - opts.dst_index = &the_index; - parse_tree(tree); - init_tree_desc(&tree_desc, tree->buffer, tree->size); - if (unpack_trees(1, &tree_desc, &opts)) - return 128; - return 0; -} - struct checkout_opts { int quiet; int merge; @@ -203,6 +161,28 @@ struct checkout_opts { enum branch_track track; }; +static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree) +{ + struct unpack_trees_options opts; + struct tree_desc tree_desc; + + memset(&opts, 0, sizeof(opts)); + opts.head_idx = -1; + opts.update = worktree; + opts.skip_unmerged = !worktree; + opts.reset = 1; + opts.merge = 1; + opts.fn = oneway_merge; + opts.verbose_update = !o->quiet; + opts.src_index = &the_index; + opts.dst_index = &the_index; + parse_tree(tree); + init_tree_desc(&tree_desc, tree->buffer, tree->size); + if (unpack_trees(1, &tree_desc, &opts)) + return 128; + return 0; +} + struct branch_info { const char *name; /* The short name used */ const char *path; /* The full name of a real branch */ @@ -227,7 +207,7 @@ static int merge_working_tree(struct checkout_opts *opts, read_cache(); if (opts->force) { - ret = reset_to_new(new->commit->tree, opts->quiet); + ret = reset_tree(new->commit->tree, opts, 1); if (ret) return ret; } else { @@ -291,12 +271,12 @@ static int merge_working_tree(struct checkout_opts *opts, add_files_to_cache(NULL, NULL, 0); work = write_tree_from_memory(); - ret = reset_to_new(new->commit->tree, opts->quiet); + ret = reset_tree(new->commit->tree, opts, 1); if (ret) return ret; merge_trees(new->commit->tree, work, old->commit->tree, new->name, "local", &result); - ret = reset_clean_to_new(new->commit->tree, opts->quiet); + ret = reset_tree(new->commit->tree, opts, 0); if (ret) return ret; } From 2e2b887d1c2c2385825160e587d711ecb5935ef5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 28 May 2008 15:12:30 -0700 Subject: [PATCH 3/4] unpack_trees(): allow callers to differentiate worktree errors from merge errors Instead of uniformly returning -1 on any error, this teaches unpack_trees() to return -2 when the merge itself is Ok but worktree refuses to get updated. Signed-off-by: Junio C Hamano --- unpack-trees.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 0de5a31c0b..cba0aca062 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -358,8 +358,13 @@ static int unpack_failed(struct unpack_trees_options *o, const char *message) return -1; } +/* + * N-way merge "len" trees. Returns 0 on success, -1 on failure to manipulate the + * resulting index, -2 on failure to reflect the changes to the work tree. + */ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o) { + int ret; static struct cache_entry *dfc; if (len > MAX_UNPACK_TREES) @@ -404,11 +409,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options return unpack_failed(o, "Merge requires file-level merging"); o->src_index = NULL; - if (check_updates(o)) - return -1; + ret = check_updates(o) ? (-2) : 0; if (o->dst_index) *o->dst_index = o->result; - return 0; + return ret; } /* Here come the merge functions */ From 291d823e364cb51cab67f0786b809fe038b92aa8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 28 May 2008 15:26:59 -0700 Subject: [PATCH 4/4] checkout: "best effort" checkout When unpack_trees() returned an error while switching branches, we used to stop right there, exiting without writing the index out or switching HEAD. This is Ok when unpack_trees() returned an error because it detected untracked files or locally modified paths that could be overwritten by branch switching, because that error return is done before we start to modify the work tree. But it is undesirable if unpack_trees() already started to update the work tree and a failure is returned because some but not all paths are updated in the work tree, perhaps because a directory that some files need to go in was read-only by mistake, or a file that will be overwritten by branch switching had a mandatory lock on it and we failed to unlink it. This changes the behaviour upon such an error to complete the branch switching; the files updated in the work tree will hopefully be much more consistent with the index and HEAD derived from the switched-to branch. We still issue error messages, and exit the command with non-zero status, so scripted callers need to notice it. Signed-off-by: Junio C Hamano --- builtin-checkout.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/builtin-checkout.c b/builtin-checkout.c index 9af5197b60..93ea69bfaa 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -155,6 +155,7 @@ struct checkout_opts { int quiet; int merge; int force; + int writeout_error; char *new_branch; int new_branch_log; @@ -178,9 +179,20 @@ static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree) opts.dst_index = &the_index; parse_tree(tree); init_tree_desc(&tree_desc, tree->buffer, tree->size); - if (unpack_trees(1, &tree_desc, &opts)) + switch (unpack_trees(1, &tree_desc, &opts)) { + case -2: + o->writeout_error = 1; + /* + * We return 0 nevertheless, as the index is all right + * and more importantly we have made best efforts to + * update paths in the work tree, and we cannot revert + * them. + */ + case 0: + return 0; + default: return 128; - return 0; + } } struct branch_info { @@ -243,7 +255,8 @@ static int merge_working_tree(struct checkout_opts *opts, tree = parse_tree_indirect(new->commit->object.sha1); init_tree_desc(&trees[1], tree->buffer, tree->size); - if (unpack_trees(2, trees, &topts)) { + ret = unpack_trees(2, trees, &topts); + if (ret == -1) { /* * Unpack couldn't do a trivial merge; either * give up or do a real merge, depending on @@ -478,7 +491,8 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new) update_refs_for_switch(opts, &old, new); - return post_checkout_hook(old.commit, new->commit, 1); + ret = post_checkout_hook(old.commit, new->commit, 1); + return ret || opts->writeout_error; } int cmd_checkout(int argc, const char **argv, const char *prefix)