threads / discuss / 24950

cherry-picking a commit clobbers a file which is a directory in the target commit

Subject: cherry-picking a commit clobbers a file which is a directory in the target commit

## tl;dr

14 messages between Sep 2, 2010 and Oct 21, 2010.

replies: 13people: 5as markdown or json

Nick· Sep 2, 2010, 16:17 UTC · lore
This was seen on git version 1.7.0.4 on Ubuntu Lucid.

Basically, cherry-picking a commit from a branch where a file in the current branch has been replaced by a directory, git clobbers the file and the cherry-pick fails reporting conflicts.

To replicate:
   $ mkdir clobber
   $ cd clobber/
   $ git init
   $ touch sausage
   $ git add sausage
   $ git commit -m "added sausage"
   $ git checkout -b branch1
   $ mv sausage sausage1
   $ mkdir sausage
   $ mv sausage1 sausage/roll
   $ git add sausage/roll
   $ git commit -m "renamed sausage as sausage/roll"
   $ touch falafel
   $ git add falafel
   $ git commit -m "added falafel"
   $ git checkout master
Now if you try to cherry pick the commit just added to branch1 onto master:
    $ git cherry-pick branch1
    Automatic cherry-pick failed.  After resolving the conflicts,
    mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
    and commit the result with:
            git commit -c branch1
    $ git status
    # On branch master
    # Changes to be committed:
    #   (use "git reset HEAD <file>..." to unstage)
    #
    #       renamed:    sausage -> falafel
    #
    # Untracked files:
    #   (use "git add <file>..." to include in what will be committed)
    #
    #       sausage~HEAD

Not what I expected at all. I'd not expect the file 'sausage' to be modified, just the new file 'falafel' added, as I did in the original commit.

I know I can recover from this by moving sausage~HEAD back to sausage, or delete it and check out sausage again, but I suspect it just shouldn't happen at all.

Is this a bug?
N
Nick· Sep 6, 2010, 09:50 UTC · re: Nick · lore

Re: cherry-picking a commit clobbers a file which is a directory in the target commit

I've been Warnocked.  Can anyone point me in the right direction?
Elijah Newren· Sep 6, 2010, 15:20 UTC · re: Nick · lore

Re: cherry-picking a commit clobbers a file which is a directory in the target commit

On Mon, Sep 6, 2010 at 3:50 AM, Nick <oinksocket@letterboxes.org> wrote:
> I've been Warnocked.  Can anyone point me in the right direction?

It's a bug in the recursive merge strategy (i.e. the default one), and affects current master as well as 1.7.0.4. The resolve strategy (which can be used with cherry-pick since 1.7.2) handles this correctly:

$ git cherry-pick --strategy=resolve branch1
Trying simple merge.
Simple merge failed, trying Automatic merge.
[master e95e377] added falafel
 0 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 falafel
I'll investigate.
Elijah
Elijah Newren· Sep 6, 2010, 20:47 UTC · re: Elijah Newren · lore

[PATCH 0/3] Fix resolvable rename + D/F conflict testcases

This fixes an issue reported by Nick, as well as a closely related issue in the handling of rename + directory/file conflicts, particularly where a file on one side of the rename is a directory name on the other side of the merge.

Elijah Newren (3):
  t3509: Add rename + D/F conflict testcases that recursive strategy
    fails
  merge-recursive: Small code cleanup
  merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir
 merge-recursive.c               |   50 ++++++++++++++++-------------
 t/t3509-cherry-pick-merge-df.sh |   66 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 22 deletions(-)
-- 
1.7.3.rc0.170.g5cfb0.dirty
Elijah Newren· Sep 6, 2010, 20:49 UTC · re: Elijah Newren · lore

Re: [PATCH 0/3] Fix resolvable rename + D/F conflict testcases

On Mon, Sep 6, 2010 at 2:47 PM, Elijah Newren <newren@gmail.com> wrote:
> This fixes an issue reported by Nick, as well as a closely related
> issue in the handling of rename + directory/file conflicts,
> particularly where a file on one side of the rename is a directory
> name on the other side of the merge.

I forgot to mention; this patch series is based on next, since it touches some of the code from ks/recursive-rename-add-identical.

Show 9 quoted lines
> Elijah Newren (3):
>  t3509: Add rename + D/F conflict testcases that recursive strategy
>    fails
>  merge-recursive: Small code cleanup
>  merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir
>
>  merge-recursive.c               |   50 ++++++++++++++++-------------
>  t/t3509-cherry-pick-merge-df.sh |   66 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 insertions(+), 22 deletions(-)
Elijah Newren· Sep 6, 2010, 20:47 UTC · re: Elijah Newren · lore

[PATCH 1/3] t3509: Add rename + D/F conflict testcases that recursive strategy fails

When one side of a file rename matches a directory name on the other side, the recursive merge strategy will fail. This is true even if the merge is trivially resolvable.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t3509-cherry-pick-merge-df.sh |   66 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)
diff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh
index a5ccdbf..eb5826f 100755
--- a/t/t3509-cherry-pick-merge-df.sh
+++ b/t/t3509-cherry-pick-merge-df.sh
@@ -32,4 +32,70 @@ test_expect_success SYMLINKS 'Cherry-pick succeeds with rename across D/F confli
 	git cherry-pick branch
 '
 
+test_expect_success 'Setup rename with file on one side matching directory name on other' '
+	git checkout --orphan nick-testcase &&
+	git rm -rf . &&
+
+	>empty &&
+	git add empty &&
+	git commit -m "Empty file" &&
+
+	git checkout -b simple &&
+	mv empty file &&
+	mkdir empty &&
+	mv file empty &&
+	git add empty/file &&
+	git commit -m "Empty file under empty dir" &&
+
+	echo content >newfile &&
+	git add newfile &&
+	git commit -m "New file"
+'
+
+test_expect_success 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (resolve)' '
+	git reset --hard &&
+	git checkout -q nick-testcase^0 &&
+	git cherry-pick --strategy=resolve simple
+'
+
+test_expect_failure 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (recursive)' '
+	git reset --hard &&
+	git checkout -q nick-testcase^0 &&
+	git cherry-pick --strategy=recursive simple
+'
+
+test_expect_success 'Setup rename with file on one side matching different dirname on other' '
+	git reset --hard &&
+	git checkout --orphan mergeme &&
+	git rm -rf . &&
+
+	mkdir sub &&
+	mkdir othersub &&
+	echo content > sub/file &&
+	echo foo > othersub/whatever &&
+	git add -A &&
+	git commit -m "Common commmit" &&
+
+	git rm -rf othersub &&
+	git mv sub/file othersub &&
+	git commit -m "Commit to merge" &&
+
+	git checkout -b newhead mergeme~1 &&
+	>independent-change &&
+	git add independent-change &&
+	git commit -m "Completely unrelated change"
+'
+
+test_expect_success 'Cherry-pick with rename to different D/F conflict succeeds (resolve)' '
+	git reset --hard &&
+	git checkout -q newhead^0 &&
+	git cherry-pick --strategy=resolve mergeme
+'
+
+test_expect_failure 'Cherry-pick with rename to different D/F conflict succeeds (recursive)' '
+	git reset --hard &&
+	git checkout -q newhead^0 &&
+	git cherry-pick --strategy=recursive mergeme
+'
+
 test_done
-- 
1.7.3.rc0.170.g5cfb0.dirty
Elijah Newren· Sep 6, 2010, 20:47 UTC · re: Elijah Newren · lore

[PATCH 2/3] merge-recursive: Small code cleanup

process_renames() had a variable named "stage" and derived variables src_other and dst_other whose purpose was not entirely clear to me. Make the name of stage slightly more descriptive and add a brief comment explaining what is occurring.

Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are identical 2010-09-01), a separate if-block was added to provide a special case for the rename/add conflict case that can be resolved (namely when the contents on the destination side are identical). However, as a separate if block, it's not immediately obvious that its code is related to the subsequent code checking for a rename/add conflict. We can combine and simplify the check slightly.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c574698..5e2886a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -923,15 +923,26 @@ static int process_renames(struct merge_options *o,
 			/* Renamed in 1, maybe changed in 2 */
 			struct string_list_item *item;
 			/* we only use sha1 and mode of these */
-			struct diff_filespec src_other, dst_other;
-			int try_merge, stage = a_renames == renames1 ? 3: 2;
+			struct diff_filespec src_other, dst_other, dst_renamed;
+			int try_merge;
 
-			remove_file(o, 1, ren1_src, o->call_depth || stage == 3);
+			/*
+			 * unpack_trees loads entries from common-commit
+			 * into stage 1, from head-commit into stage 2, and
+			 * from merge-commit into stage 3.  We keep track
+			 * of which side corresponds to the rename.
+			 */
+			int renamed_stage = a_renames == renames1 ? 2 : 3;
+			int other_stage =   a_renames == renames1 ? 3 : 2;
+
+			remove_file(o, 1, ren1_src, o->call_depth || renamed_stage == 2);
 
-			hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
-			src_other.mode = ren1->src_entry->stages[stage].mode;
-			hashcpy(dst_other.sha1, ren1->dst_entry->stages[stage].sha);
-			dst_other.mode = ren1->dst_entry->stages[stage].mode;
+			hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);
+			src_other.mode = ren1->src_entry->stages[other_stage].mode;
+			hashcpy(dst_other.sha1, ren1->dst_entry->stages[other_stage].sha);
+			dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
+			hashcpy(dst_renamed.sha1, ren1->dst_entry->stages[renamed_stage].sha);
+			dst_renamed.mode = ren1->dst_entry->stages[renamed_stage].mode;
 
 			try_merge = 0;
 
@@ -955,13 +966,8 @@ static int process_renames(struct merge_options *o,
 							ren1->pair->two : NULL,
 							branch1 == o->branch1 ?
 							NULL : ren1->pair->two, 1);
-			} else if ((dst_other.mode == ren1->pair->two->mode) &&
-				   sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {
-				/* Added file on the other side
-				   identical to the file being
-				   renamed: clean merge */
-				update_file(o, 1, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
-			} else if (!sha_eq(dst_other.sha1, null_sha1)) {
+			} else if (!sha_eq(dst_other.sha1, null_sha1) &&
+				   !sha_eq(dst_other.sha1, dst_renamed.sha1)) {
 				const char *new_path;
 				clean_merge = 0;
 				try_merge = 1;
-- 
1.7.3.rc0.170.g5cfb0.dirty
Elijah Newren· Sep 6, 2010, 21:25 UTC · re: Elijah Newren · lore

Re: [PATCH 2/3] merge-recursive: Small code cleanup

On Mon, Sep 6, 2010 at 2:47 PM, Elijah Newren <newren@gmail.com> wrote:
Show 15 quoted lines
> process_renames() had a variable named "stage" and derived variables
> src_other and dst_other whose purpose was not entirely clear to me.  Make
> the name of stage slightly more descriptive and add a brief comment
> explaining what is occurring.
>
> Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are
> identical 2010-09-01), a separate if-block was added to provide a special
> case for the rename/add conflict case that can be resolved (namely when
> the contents on the destination side are identical).  However, as a
> separate if block, it's not immediately obvious that its code is related to
> the subsequent code checking for a rename/add conflict.  We can combine and
> simplify the check slightly.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---

Hmmm...should I have split this off from the rest of the series (its only relation is that it cleans up code that made it harder for me to find the real fix)? If I did that, I could rebase the rest of the series on maint...

Junio C Hamano· Sep 6, 2010, 23:49 UTC · re: Elijah Newren · lore

Re: [PATCH 2/3] merge-recursive: Small code cleanup

Elijah Newren <newren@gmail.com> writes:
> Hmmm...should I have split this off from the rest of the series (its
> only relation is that it cleans up code that made it harder for me to
> find the real fix)?  If I did that, I could rebase the rest of the
> series on maint...
Good thinking.

The real polishing of this series will happen after 1.7.3 anyway, so for now a series forking from ks/recursive-rename-add-identical (which I expect to be in 1.7.3) is fine.

Thanks.
Schalk, Ken· Sep 7, 2010, 16:23 UTC · re: Elijah Newren · lore

RE: [PATCH 2/3] merge-recursive: Small code cleanup

Show 7 quoted lines
>Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are
>identical 2010-09-01), a separate if-block was added to provide a special
>case for the rename/add conflict case that can be resolved (namely when
>the contents on the destination side are identical).  However, as a
>separate if block, it's not immediately obvious that its code is related to
>the subsequent code checking for a rename/add conflict.  We can combine and
>simplify the check slightly.
Originally I tried the fix the way you've re-structured it, just adding a test to the if around the rename/add conflict handling.  Unfortunately that didn't completely solve the problem in the case that originally motivated the fix (rename vs. rename+symlink, as in my initial post and my first attempt at adding a test to t/t3030-merge-recursive.sh).  That's why I changed it to a separate if block.
The problem comes down in the code inside the "if(try_merge)" block below.  It merges the source of the rename on the other side with the renamed file, rather than the destination.  In the case with the symlink on the other side, this code merged a symlink with a regular file which resulted in a conflict.  I was trying to eliminate both conflicts in this case by avoiding the final else that sets try_merge=1.
Your re-structuring will therefore only solve half the problem I was trying to solve.
I suppose an alternative solution would have been to change the "if(try_merge)" code to merge with the destination of the rename on the other side, if it exists and is the same type.  However that clearly would have had a much more significant impact on other merge cases, so it didn't seem like a good choice to me.
--Ken
Elijah Newren· Sep 8, 2010, 06:24 UTC · re: Schalk, Ken · lore

Re: [PATCH 2/3] merge-recursive: Small code cleanup

On Tue, Sep 7, 2010 at 10:23 AM, Schalk, Ken <ken.schalk@intel.com> wrote:
Show 15 quoted lines
>>Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are
>>identical 2010-09-01), a separate if-block was added to provide a special
>>case for the rename/add conflict case that can be resolved (namely when
>>the contents on the destination side are identical).  However, as a
>>separate if block, it's not immediately obvious that its code is related to
>>the subsequent code checking for a rename/add conflict.  We can combine and
>>simplify the check slightly.
>
> Originally I tried the fix the way you've re-structured it, just adding a test to the if around the rename/add conflict handling.  Unfortunately that didn't completely solve the problem in the case that originally motivated the fix (rename vs. rename+symlink, as in my initial post and my first attempt at adding a test to t/t3030-merge-recursive.sh).  That's why I changed it to a separate if block.
>
> The problem comes down in the code inside the "if(try_merge)" block below.  It merges the source of the rename on the other side with the renamed file, rather than the destination.  In the case with the symlink on the other side, this code merged a symlink with a regular file which resulted in a conflict.  I was trying to eliminate both conflicts in this case by avoiding the final else that sets try_merge=1.
>
> Your re-structuring will therefore only solve half the problem I was trying to solve.
>
> I suppose an alternative solution would have been to change the "if(try_merge)" code to merge with the destination of the rename on the other side, if it exists and is the same type.  However that clearly would have had a much more significant impact on other merge cases, so it didn't seem like a good choice to me.

Interesting...that means we probably should have stuck with the original testcase you suggested (though marking it with the SYMLINK dependence), since the new one doesn't fail with my modifications but the old one would. The typechange is critical. So I'll drop that portion of my patch.

Perhaps you could submit another patch changing your testcase back to using a symlink to make sure someone like me doesn't break your original testcase in the future?

Thanks, Elijah

Schalk, Ken· Sep 9, 2010, 20:23 UTC · re: Elijah Newren · lore

RE: [PATCH 2/3] merge-recursive: Small code cleanup

>Perhaps you could submit another patch changing your testcase back to
>using a symlink to make sure someone like me doesn't break your
>original testcase in the future?
Here's a patch relative to my last one.  Rather than restoring the previous test, I added it so that platforms with no symlink support can still test copy vs. rename and platforms with symlink support can also test rename vs. rename/symlink.
Signed-off-by: Ken Schalk <ken.schalk@intel.com>
---
 t/t3030-merge-recursive.sh |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index b23bd9f..9514ae2 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -25,6 +25,10 @@ test_expect_success 'setup 1' '
        git branch submod &&
        git branch copy &&
        git branch rename &&
+       if test_have_prereq SYMLINKS
+       then
+               git branch rename-ln
+       fi &&

        echo hello >>a &&
        cp a d/e &&
@@ -256,7 +260,17 @@ test_expect_success 'setup 8' '
        git mv a e &&
        git add e &&
        test_tick &&
-       git commit -m "rename a->e"
+       git commit -m "rename a->e" &&
+       if test_have_prereq SYMLINKS
+       then
+               git checkout rename-ln &&
+               git mv a e &&
+               ln -s e a &&
+               git add a e &&
+               test_tick &&
+               git commit -m "rename a->e, symlink a->e"
+       fi
+
 '

 test_expect_success 'setup 9' '
@@ -618,5 +632,25 @@ test_expect_success 'merge-recursive copy vs. rename' '
        test_cmp expected actual
 '

+if test_have_prereq SYMLINKS
+then
+       test_expect_success 'merge-recursive rename vs. rename/symlink' '
+
+               git checkout -f rename &&
+               git merge rename-ln &&
+               ( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+               (
+                       echo "100644 blob $o0   b"
+                       echo "100644 blob $o0   c"
+                       echo "100644 blob $o0   d/e"
+                       echo "100644 blob $o0   e"
+                       echo "100644 $o0 0      b"
+                       echo "100644 $o0 0      c"
+                       echo "100644 $o0 0      d/e"
+                       echo "100644 $o0 0      e"
+               ) >expected &&
+               test_cmp expected actual
+       '
+fi

 test_done
--
1.7.0
Camille Moncelier· Oct 21, 2010, 19:43 UTC · re: Schalk, Ken · lore

Re: [PATCH 2/3] merge-recursive: Small code cleanup

On Thu, 9 Sep 2010 13:23:26 -0700 "Schalk, Ken" <ken.schalk@intel.com> wrote:

Show 8 quoted lines
> >Perhaps you could submit another patch changing your testcase back to
> >using a symlink to make sure someone like me doesn't break your
> >original testcase in the future?
> 
> Here's a patch relative to my last one.  Rather than restoring the
> previous test, I added it so that platforms with no symlink support
> can still test copy vs. rename and platforms with symlink support can
> also test rename vs. rename/symlink.

Hello, I think I have a test case that seems to be related to this issue.

mkdir -p repo1 pushd repo1 git init .

mkdir dir1 echo file1 > dir1/file1 ln -s dir1 dir2 git add dir1 dir2 git commit -m "Initial status: dir2 -> dir1"

git checkout -b test1 git checkout -b test2

git co test1 git rm dir2 mkdir dir2 touch file2 > dir2/file1 git add dir2/file1 git commit -m "Removing link: dir1/ and dir2/"

message="New file in test1" echo $message > new_file_test1 git add new_file_test1 git commit -m "$message"

git co test2 message="New file in test2" echo $message > new_file_test2 git add new_file_test2 git commit -m "$message"

# Tries to get the last commit (which adds new_file_test1) # into test2 fails. git cherry-pick test1

# Would work with: git cherry-pick --strategy=resolve test1 # (using 1.7.3.1)

Elijah Newren· Sep 6, 2010, 20:47 UTC · re: Elijah Newren · lore

[PATCH 3/3] merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir

In merge-recursive.c, whenever there was a rename where a file name on one
side of the rename matches a directory name on the other side of the merge,
then the very first check that
  string_list_has_string(&o->current_directory_set, ren1_dst)
would trigger forcing it into marking it as a rename/directory conflict.

However, if the path is only renamed on one side and a simple three-way merge between the separate files resolves cleanly, then we don't need to mark it as a rename/directory conflict. So, we can simply move the check for rename/directory conflicts after we've verified that there isn't a rename/rename conflict and that a threeway content merge doesn't work.

This changes the particular error message one gets in the case where the
directory name that a file on one side of the rename matches is not also
part of the rename pair.  For example, with commits containing the files:
  COMMON    -> (HEAD,           MERGE )
  ---------    ---------------  -------
  sub/file1 -> (sub/file1,      newsub)
  <NULL>    -> (newsub/newfile, <NULL>)
then previously when one tried to merge MERGE into HEAD, one would get
  CONFLICT (rename/directory): Rename sub/file1->newsub in HEAD  directory newsub added in merge
  Renaming sub/file1 to newsub~HEAD instead
  Adding newsub/newfile
  Automatic merge failed; fix conflicts and then commit the result.
After this patch, the error message will instead become:
  Removing newsub
  Adding newsub/newfile
  CONFLICT (file/directory): There is a directory with name newsub in merge. Adding newsub as newsub~HEAD
  Automatic merge failed; fix conflicts and then commit the result.
That makes more sense to me, because git can't know that there's a conflict
until after it's tried resolving paths involving newsub/newfile to see if
they are still in the way at the end (and if newsub/newfile is not in the
way at the end, there should be no conflict at all, which did not hold with
git previously).
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c               |   16 ++++++++--------
 t/t3509-cherry-pick-merge-df.sh |    4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 5e2886a..645a79d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -946,14 +946,7 @@ static int process_renames(struct merge_options *o,
 
 			try_merge = 0;
 
-			if (string_list_has_string(&o->current_directory_set, ren1_dst)) {
-				clean_merge = 0;
-				output(o, 1, "CONFLICT (rename/directory): Rename %s->%s in %s "
-				       " directory %s added in %s",
-				       ren1_src, ren1_dst, branch1,
-				       ren1_dst, branch2);
-				conflict_rename_dir(o, ren1, branch1);
-			} else if (sha_eq(src_other.sha1, null_sha1)) {
+			if (sha_eq(src_other.sha1, null_sha1)) {
 				clean_merge = 0;
 				output(o, 1, "CONFLICT (rename/delete): Rename %s->%s in %s "
 				       "and deleted in %s",
@@ -1046,6 +1039,13 @@ static int process_renames(struct merge_options *o,
 					if (!ren1->dst_entry->stages[2].mode !=
 					    !ren1->dst_entry->stages[3].mode)
 						ren1->dst_entry->processed = 0;
+				} else if (string_list_has_string(&o->current_directory_set, ren1_dst)) {
+					clean_merge = 0;
+					output(o, 1, "CONFLICT (rename/directory): Rename %s->%s in %s "
+					       " directory %s added in %s",
+					       ren1_src, ren1_dst, branch1,
+					       ren1_dst, branch2);
+					conflict_rename_dir(o, ren1, branch1);
 				} else {
 					if (mfi.merge || !mfi.clean)
 						output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst);
diff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh
index eb5826f..948ca1b 100755
--- a/t/t3509-cherry-pick-merge-df.sh
+++ b/t/t3509-cherry-pick-merge-df.sh
@@ -58,7 +58,7 @@ test_expect_success 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (reso
 	git cherry-pick --strategy=resolve simple
 '
 
-test_expect_failure 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (recursive)' '
+test_expect_success 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (recursive)' '
 	git reset --hard &&
 	git checkout -q nick-testcase^0 &&
 	git cherry-pick --strategy=recursive simple
@@ -92,7 +92,7 @@ test_expect_success 'Cherry-pick with rename to different D/F conflict succeeds
 	git cherry-pick --strategy=resolve mergeme
 '
 
-test_expect_failure 'Cherry-pick with rename to different D/F conflict succeeds (recursive)' '
+test_expect_success 'Cherry-pick with rename to different D/F conflict succeeds (recursive)' '
 	git reset --hard &&
 	git checkout -q newhead^0 &&
 	git cherry-pick --strategy=recursive mergeme
-- 
1.7.3.rc0.170.g5cfb0.dirty

← back to recent threads