From 0ecd180a27eab7c576feddc6f2be0626a1e0630b Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 12 Aug 2014 16:59:31 -0700 Subject: unpack-trees: simplify 'all other failures' case In the 'if (current)' block of twoway_merge, we handle the boring errors by checking if the entry from the old tree, current index, and new tree are present, to get a pathname for the error message from one of them: if (oldtree) return o->gently ? -1 : reject_merge(oldtree, o); if (current) return o->gently ? -1 : reject_merge(current, o); if (newtree) return o->gently ? -1 : reject_merge(newtree, o); return -1; Since this is guarded by 'if (current)', the second test is guaranteed to succeed. Moreover, any of the three entries, if present, would have the same path because there is no rename detection in this code path. Even if some day in the future the entries' paths differ, the 'current' path used in the index and worktree would presumably be the most recognizable for the end user. Simplify by just using 'current'. Noticed by coverity, Id:290002 Signed-off-by: Stefan Beller Improved-by: Junio C Hamano Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- unpack-trees.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index ad3e9a04fe..f4a9aa97a4 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1791,16 +1791,8 @@ int twoway_merge(const struct cache_entry * const *src, /* 20 or 21 */ return merged_entry(newtree, current, o); } - else { - /* all other failures */ - if (oldtree) - return o->gently ? -1 : reject_merge(oldtree, o); - if (current) - return o->gently ? -1 : reject_merge(current, o); - if (newtree) - return o->gently ? -1 : reject_merge(newtree, o); - return -1; - } + else + return o->gently ? -1 : reject_merge(current, o); } else if (newtree) { if (oldtree && !o->initial_checkout) { -- cgit v1.2.3 From 6c1db1b38886f70165cb9f5822b1a2e99a2c331b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 12 Aug 2014 17:00:45 -0700 Subject: unpack-trees: use 'cuddled' style for if-else cascade Match the predominant style in git by following K&R style for if/else cascades. Documentation/CodingStyle from linux.git explains: Note that the closing brace is empty on a line of its own, _except_ in the cases where it is followed by a continuation of the same statement, ie a "while" in a do-statement or an "else" in an if-statement, like this: if (x == y) { .. } else if (x > y) { ... } else { .... } Rationale: K&R. Also, note that this brace-placement also minimizes the number of empty (or almost empty) lines, without any loss of readability. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- unpack-trees.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index f4a9aa97a4..187b15b56f 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1771,8 +1771,7 @@ int twoway_merge(const struct cache_entry * const *src, return merged_entry(newtree, current, o); } return o->gently ? -1 : reject_merge(current, o); - } - else if ((!oldtree && !newtree) || /* 4 and 5 */ + } else if ((!oldtree && !newtree) || /* 4 and 5 */ (!oldtree && newtree && same(current, newtree)) || /* 6 and 7 */ (oldtree && newtree && @@ -1781,17 +1780,14 @@ int twoway_merge(const struct cache_entry * const *src, !same(oldtree, newtree) && /* 18 and 19 */ same(current, newtree))) { return keep_entry(current, o); - } - else if (oldtree && !newtree && same(current, oldtree)) { + } else if (oldtree && !newtree && same(current, oldtree)) { /* 10 or 11 */ return deleted_entry(oldtree, current, o); - } - else if (oldtree && newtree && + } else if (oldtree && newtree && same(current, oldtree) && !same(current, newtree)) { /* 20 or 21 */ return merged_entry(newtree, current, o); - } - else + } else return o->gently ? -1 : reject_merge(current, o); } else if (newtree) { -- cgit v1.2.3 From 6a143aa2b23b97fa8363e2f4fd16f23b4c5b104d Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 12 Aug 2014 17:03:18 -0700 Subject: checkout -m: attempt merge when deletion of path was staged twoway_merge() is missing an o->gently check in the case where a file that needs to be modified is missing from the index but present in the old and new trees. As a result, in this case 'git checkout -m' errors out instead of trying to perform a merge. Fix it by checking o->gently. While at it, inline the o->gently check into reject_merge to prevent future call sites from making the same mistake. Noticed by code inspection. The test for the motivating case was added by JC. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t7201-co.sh | 17 +++++++++++++++++ unpack-trees.c | 11 ++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/t/t7201-co.sh b/t/t7201-co.sh index 0c9ec0ad44..eae9e5a937 100755 --- a/t/t7201-co.sh +++ b/t/t7201-co.sh @@ -223,6 +223,23 @@ test_expect_success 'checkout --merge --conflict=diff3 ' ' test_cmp two expect ' +test_expect_success 'switch to another branch while carrying a deletion' ' + + git checkout -f master && git reset --hard && git clean -f && + git rm two && + + test_must_fail git checkout simple 2>errs && + test_i18ngrep overwritten errs && + + git checkout --merge simple 2>errs && + test_i18ngrep ! overwritten errs && + git ls-files -u && + test_must_fail git cat-file -t :0:two && + test "$(git cat-file -t :1:two)" = blob && + test "$(git cat-file -t :2:two)" = blob && + test_must_fail git cat-file -t :3:two +' + test_expect_success 'checkout to detach HEAD (with advice declined)' ' git config advice.detachedHead false && diff --git a/unpack-trees.c b/unpack-trees.c index 187b15b56f..6c45af7248 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1178,7 +1178,8 @@ return_failed: static int reject_merge(const struct cache_entry *ce, struct unpack_trees_options *o) { - return add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce->name); + return o->gently ? -1 : + add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce->name); } static int same(const struct cache_entry *a, const struct cache_entry *b) @@ -1633,7 +1634,7 @@ int threeway_merge(const struct cache_entry * const *stages, /* #14, #14ALT, #2ALT */ if (remote && !df_conflict_head && head_match && !remote_match) { if (index && !same(index, remote) && !same(index, head)) - return o->gently ? -1 : reject_merge(index, o); + return reject_merge(index, o); return merged_entry(remote, index, o); } /* @@ -1641,7 +1642,7 @@ int threeway_merge(const struct cache_entry * const *stages, * make sure that it matches head. */ if (index && !same(index, head)) - return o->gently ? -1 : reject_merge(index, o); + return reject_merge(index, o); if (head) { /* #5ALT, #15 */ @@ -1770,7 +1771,7 @@ int twoway_merge(const struct cache_entry * const *src, else return merged_entry(newtree, current, o); } - return o->gently ? -1 : reject_merge(current, o); + return reject_merge(current, o); } else if ((!oldtree && !newtree) || /* 4 and 5 */ (!oldtree && newtree && same(current, newtree)) || /* 6 and 7 */ @@ -1788,7 +1789,7 @@ int twoway_merge(const struct cache_entry * const *src, /* 20 or 21 */ return merged_entry(newtree, current, o); } else - return o->gently ? -1 : reject_merge(current, o); + return reject_merge(current, o); } else if (newtree) { if (oldtree && !o->initial_checkout) { -- cgit v1.2.3