threads / patch / 43415

patchRe: [PATCH] Remove uncontested renamed files during merge.

Subject: Re: [PATCH] Remove uncontested renamed files during merge.

## tl;dr

4 messages between Dec 13, 2006 and Dec 13, 2006. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Shawn O. Pearce· Dec 13, 2006, 09:55 UTC · lore

[PATCH] Remove uncontested renamed files during merge.

Prior to 65ac6e9c3f47807cb603af07a6a9e1a43bc119ae we deleted a file from the working directory during a merge if the file existed in the working directory before the merge started but was renamed by the branch which is being merged in. This broke in 65ac63 as git-merge-recursive did not actually update the working directory on this uncontested rename case.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 The same repository that spawned my 'Bug in merge-recursive in
 virtual commit corner case' thread discovered this feature of
 git-merge-recursive today.
 I'm not sure this is the right way to fix the issue, but it does
 appear to fix the problem and passes both the existing tests and
 this new one.
 merge-recursive.c        |    2 +-
 t/t6024-merge-rename2.sh |   97 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 1 deletions(-)
Show changes to 2 files +98 −1

merge-recursive.c, t/t6024-merge-rename2.sh

diff --git a/merge-recursive.c b/merge-recursive.c
index 9d53bcd..741d17f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -873,7 +873,7 @@ static int process_renames(struct path_list *a_renames,
 			struct diff_filespec src_other, dst_other;
 			int try_merge, stage = a_renames == renames1 ? 3: 2;
 
-			remove_file(1, ren1_src, 1);
+			remove_file(1, ren1_src, 0);
 
 			hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
 			src_other.mode = ren1->src_entry->stages[stage].mode;
diff --git a/t/t6024-merge-rename2.sh b/t/t6024-merge-rename2.sh
new file mode 100755
index 0000000..69c66cf
--- /dev/null
+++ b/t/t6024-merge-rename2.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+
+test_description='Merge-recursive merging renames'
+. ./test-lib.sh
+
+test_expect_success setup \
+'
+cat >A <<\EOF &&
+a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+c cccccccccccccccccccccccccccccccccccccccccccccccc
+d dddddddddddddddddddddddddddddddddddddddddddddddd
+e eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+f ffffffffffffffffffffffffffffffffffffffffffffffff
+g gggggggggggggggggggggggggggggggggggggggggggggggg
+h hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
+i iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii
+j jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj
+k kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk
+l llllllllllllllllllllllllllllllllllllllllllllllll
+m mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm
+n nnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn
+o oooooooooooooooooooooooooooooooooooooooooooooooo
+EOF
+
+cat >M <<\EOF &&
+A AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+B BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
+C CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC
+D DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD
+E EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE
+F FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
+G GGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG
+H HHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH
+I IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII
+J JJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJ
+K KKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKK
+L LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL
+M MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM
+N NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN
+O OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO
+EOF
+
+git add A M &&
+git commit -m "initial has A and M" &&
+git branch white &&
+git branch red &&
+
+git checkout white &&
+sed -e "/^g /s/.*/g : white changes a line/" <A >B &&
+sed -e "/^G /s/.*/G : colored branch changes a line/" <M >N &&
+rm -f A M &&
+git update-index --add --remove A B M N &&
+git commit -m "white renames A->B, M->N" &&
+
+git checkout red &&
+echo created by red >R &&
+git update-index --add R &&
+git commit -m "red creates R" &&
+
+git checkout master'
+
+# This test broke in 65ac6e9c3f47807cb603af07a6a9e1a43bc119ae
+test_expect_success 'merge white into red (A->B,M->N)' \
+'
+	git checkout -b red-white red &&
+	git merge white &&
+	git write-tree >/dev/null || {
+		echo "BAD: merge did not complete"
+		return 1
+	}
+
+	test -f B || {
+		echo "BAD: B does not exist in working directory"
+		return 1
+	}
+	test -f N || {
+		echo "BAD: N does not exist in working directory"
+		return 1
+	}
+	test -f R || {
+		echo "BAD: R does not exist in working directory"
+		return 1
+	}
+
+	test -f A && {
+		echo "BAD: A still exists in working directory"
+		return 1
+	}
+	test -f M && {
+		echo "BAD: M still exists in working directory"
+		return 1
+	}
+	return 0
+'
+
+test_done
Johannes Schindelin· Dec 13, 2006, 10:31 UTC · re: Shawn O. Pearce · lore
Hi,
On Wed, 13 Dec 2006, Shawn O. Pearce wrote:
Show 19 quoted lines
> Prior to 65ac6e9c3f47807cb603af07a6a9e1a43bc119ae we deleted a
> file from the working directory during a merge if the file existed
> in the working directory before the merge started but was renamed
> by the branch which is being merged in.  This broke in 65ac63 as
> git-merge-recursive did not actually update the working directory
> on this uncontested rename case.
> 
> [...]
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 9d53bcd..741d17f 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -873,7 +873,7 @@ static int process_renames(struct path_list *a_renames,
>  			struct diff_filespec src_other, dst_other;
>  			int try_merge, stage = a_renames == renames1 ? 3: 2;
>  
> -			remove_file(1, ren1_src, 1);
> +			remove_file(1, ren1_src, 0);

I _think_ that the "0" should be "!index_only". After all, these functions are not only called on the virtual merges, but also on the final merge, which indeed should update the working directory. And since it is a rename, the old file has to go.

Ciao,
Shawn Pearce· Dec 13, 2006, 10:42 UTC · re: Johannes Schindelin · lore
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 12 quoted lines
> > diff --git a/merge-recursive.c b/merge-recursive.c
> > index 9d53bcd..741d17f 100644
> > --- a/merge-recursive.c
> > +++ b/merge-recursive.c
> > @@ -873,7 +873,7 @@ static int process_renames(struct path_list *a_renames,
> >  			struct diff_filespec src_other, dst_other;
> >  			int try_merge, stage = a_renames == renames1 ? 3: 2;
> >  
> > -			remove_file(1, ren1_src, 1);
> > +			remove_file(1, ren1_src, 0);
> 
> I _think_ that the "0" should be "!index_only".

Err, actually "index_only". Since that's no_wd. But yea, you are right, I didn't consider the index_only mode of operation here.

-->-- corrected patch follows --<-- Remove uncontested renamed files during merge.

Prior to 65ac6e9c3f47807cb603af07a6a9e1a43bc119ae we deleted a file from the working directory during a merge if the file existed before the merge started but was renamed by the branch being merged in. This broke in 65ac63 as git-merge-recursive did not actually update the working directory on an uncontested rename.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 merge-recursive.c        |    2 +-
 t/t6024-merge-rename2.sh |   97 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 1 deletions(-)
Show changes to 2 files +98 −1

merge-recursive.c, t/t6024-merge-rename2.sh

diff --git a/merge-recursive.c b/merge-recursive.c
index 9d53bcd..13e96eb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -873,7 +873,7 @@ static int process_renames(struct path_list *a_renames,
 			struct diff_filespec src_other, dst_other;
 			int try_merge, stage = a_renames == renames1 ? 3: 2;
 
-			remove_file(1, ren1_src, 1);
+			remove_file(1, ren1_src, index_only);
 
 			hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
 			src_other.mode = ren1->src_entry->stages[stage].mode;
diff --git a/t/t6024-merge-rename2.sh b/t/t6024-merge-rename2.sh
new file mode 100755
index 0000000..69c66cf
--- /dev/null
+++ b/t/t6024-merge-rename2.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+
+test_description='Merge-recursive merging renames'
+. ./test-lib.sh
+
+test_expect_success setup \
+'
+cat >A <<\EOF &&
+a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+c cccccccccccccccccccccccccccccccccccccccccccccccc
+d dddddddddddddddddddddddddddddddddddddddddddddddd
+e eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+f ffffffffffffffffffffffffffffffffffffffffffffffff
+g gggggggggggggggggggggggggggggggggggggggggggggggg
+h hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
+i iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii
+j jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj
+k kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk
+l llllllllllllllllllllllllllllllllllllllllllllllll
+m mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm
+n nnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn
+o oooooooooooooooooooooooooooooooooooooooooooooooo
+EOF
+
+cat >M <<\EOF &&
+A AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+B BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
+C CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC
+D DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD
+E EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE
+F FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
+G GGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG
+H HHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH
+I IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII
+J JJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJ
+K KKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKK
+L LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL
+M MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM
+N NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN
+O OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO
+EOF
+
+git add A M &&
+git commit -m "initial has A and M" &&
+git branch white &&
+git branch red &&
+
+git checkout white &&
+sed -e "/^g /s/.*/g : white changes a line/" <A >B &&
+sed -e "/^G /s/.*/G : colored branch changes a line/" <M >N &&
+rm -f A M &&
+git update-index --add --remove A B M N &&
+git commit -m "white renames A->B, M->N" &&
+
+git checkout red &&
+echo created by red >R &&
+git update-index --add R &&
+git commit -m "red creates R" &&
+
+git checkout master'
+
+# This test broke in 65ac6e9c3f47807cb603af07a6a9e1a43bc119ae
+test_expect_success 'merge white into red (A->B,M->N)' \
+'
+	git checkout -b red-white red &&
+	git merge white &&
+	git write-tree >/dev/null || {
+		echo "BAD: merge did not complete"
+		return 1
+	}
+
+	test -f B || {
+		echo "BAD: B does not exist in working directory"
+		return 1
+	}
+	test -f N || {
+		echo "BAD: N does not exist in working directory"
+		return 1
+	}
+	test -f R || {
+		echo "BAD: R does not exist in working directory"
+		return 1
+	}
+
+	test -f A && {
+		echo "BAD: A still exists in working directory"
+		return 1
+	}
+	test -f M && {
+		echo "BAD: M still exists in working directory"
+		return 1
+	}
+	return 0
+'
+
+test_done
-- 
1.4.4.2.g8662
Johannes Schindelin· Dec 13, 2006, 10:45 UTC · re: Shawn Pearce · lore
Hi,
On Wed, 13 Dec 2006, Shawn Pearce wrote:
Show 15 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > diff --git a/merge-recursive.c b/merge-recursive.c
> > > index 9d53bcd..741d17f 100644
> > > --- a/merge-recursive.c
> > > +++ b/merge-recursive.c
> > > @@ -873,7 +873,7 @@ static int process_renames(struct path_list *a_renames,
> > >  			struct diff_filespec src_other, dst_other;
> > >  			int try_merge, stage = a_renames == renames1 ? 3: 2;
> > >  
> > > -			remove_file(1, ren1_src, 1);
> > > +			remove_file(1, ren1_src, 0);
> > 
> > I _think_ that the "0" should be "!index_only".
> 
> Err, actually "index_only".  Since that's no_wd.
Oops. Correct. My memory starts failing me.
> -->-- corrected patch follows --<--
> Remove uncontested renamed files during merge.
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Ciao, Dscho

← back to recent threads