threads / rfc / 27239

RFC patchTest for apparent regression of merging renamed files

Subject: [PATCH RFC] Test for apparent regression of merging renamed files

## tl;dr

3 messages between May 2, 2011 and May 4, 2011. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Ciaran· May 2, 2011, 19:04 UTC · lore

Provides a test to cover a case that appears to have been regressed out by:

 b2c8c0a762745768e8fb249949801c1aed8f7c1d

At this point no tags contain the above commit, but 'master' does. I'm unsure what is causing the issue, but can see discussion about this fix here:

http://git.661346.n2.nabble.com/BUG-merge-recursive-triggered-quot-BUG-quot-td6179353.html

Providing a patch to demonstrate the issue and spark discussion. --- Whilst working up a patch for my 'whitespace ignorant rename detection' ( http://git.661346.n2.nabble.com/PATCH-RFC-Rename-detection-and-whitespace-td6288524.html ) I was unable to get my tests working, turns out that something that used to work for me, no longer does in master's HEAD.

I don't really understand the fault so I'm throwing it to the list in the hope that someone brighter than me can take it on (I've CC'd the signoff-s to that end! :) Sorry!

 t/t9801-merge-rename.sh |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100755 t/t9801-merge-rename.sh

Show changes to t/t9801-merge-rename.sh +41 −0

diff --git a/t/t9801-merge-rename.sh b/t/t9801-merge-rename.sh
new file mode 100755
index 0000000..363911f
--- /dev/null
+++ b/t/t9801-merge-rename.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Test for rename merge regression'
+
+. ./test-lib.sh
+
+write_out_files() {
+cat > noformatting.txt << EOF
+if(true){
+// Meaningless
+}
+EOF
+
+cat > formatting.txt << EOF
+	if ( true ) {
+	  // Meaningless
+	}
+EOF
+}
+
+# Test the case where a rename happens in one branch and
+# a whitespace change occurs in a different branch.  The rename
+# change should apply to the whitespace modified file
+test_expect_success setup '
+	write_out_files &&
+	git add formatting.txt &&
+
+	test_tick &&
+	git commit -m Initial &&
+	git checkout -b rename_branch &&
+	git mv formatting.txt formatting_renamed.txt &&
+	git commit -m Rename &&
+	git checkout master &&
+	rm formatting.txt &&
+	mv noformatting.txt formatting.txt &&
+	git commit -a -m Reformat
+'
+test_expect_success 'merge' '
+	git merge rename_branch
+'
+test_done

-- 
1.7.4.1

Ciaran· May 3, 2011, 19:15 UTC · re: Ciaran · lore

Re: [PATCH RFC] Test for apparent regression of merging renamed files

On Mon, May 2, 2011 at 8:04 PM, Ciaran <ciaranj@gmail.com> wrote:

Show 10 quoted lines

> Provides a test to cover a case that appears to have been regressed out by:
>
>  b2c8c0a762745768e8fb249949801c1aed8f7c1d
>
> At this point no tags contain the above commit, but 'master' does.  I'm unsure
> what is causing the issue, but can see discussion about this fix here:
>
> http://git.661346.n2.nabble.com/BUG-merge-recursive-triggered-quot-BUG-quot-td6179353.html
>
> Providing a patch to demonstrate the issue and spark discussion.

Hmm I think with hindsight I should've been clearer ;) The current master branch appears to contain a break in it, seemingly introduced by commit b2c8c0a . The rename & merge situation given in the associated test results in output from git status such as :

$ git status # On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # deleted: formatting.txt # # Unmerged paths: # (use "git reset HEAD <file>..." to unstage) # (use "git add/rm <file>..." as appropriate to mark resolution) # # both modified: formatting_renamed.txt #

Whereas prior to b2c8c0a762745768e8fb249949801c1aed8f7c1d (for example @ d98a509ec3e9ff22bb642f778900691d0c715ba9 ) the status was:

# On branch master nothing to commit (working directory clean)

And git log --oneline -1 says :

e03ad75 Merge branch 'rename_branch'

The original behaviour (successful merge) seems 'better' than the current behaviour 'failed merge, 'erroneous' both modified flag' ... am I mis-reading this, is this a new non-supported edge case ?

Cheers!
 -cJ.

Show 64 quoted lines

> ---
> Whilst working up a patch for my 'whitespace ignorant rename
> detection' ( http://git.661346.n2.nabble.com/PATCH-RFC-Rename-detection-and-whitespace-td6288524.html
> ) I was unable to get my tests working, turns out that something that
> used to work for me, no longer does in master's HEAD.
>
> I don't really understand the fault so I'm throwing it to the list in
> the hope that someone brighter than me can take it on (I've
> CC'd the signoff-s to that end! :) Sorry!
>
>  t/t9801-merge-rename.sh |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
>  create mode 100755 t/t9801-merge-rename.sh
>
> diff --git a/t/t9801-merge-rename.sh b/t/t9801-merge-rename.sh
> new file mode 100755
> index 0000000..363911f
> --- /dev/null
> +++ b/t/t9801-merge-rename.sh
> @@ -0,0 +1,41 @@
> +#!/bin/sh
> +
> +test_description='Test for rename merge regression'
> +
> +. ./test-lib.sh
> +
> +write_out_files() {
> +cat > noformatting.txt << EOF
> +if(true){
> +// Meaningless
> +}
> +EOF
> +
> +cat > formatting.txt << EOF
> +       if ( true ) {
> +         // Meaningless
> +       }
> +EOF
> +}
> +
> +# Test the case where a rename happens in one branch and
> +# a whitespace change occurs in a different branch.  The rename
> +# change should apply to the whitespace modified file
> +test_expect_success setup '
> +       write_out_files &&
> +       git add formatting.txt &&
> +
> +       test_tick &&
> +       git commit -m Initial &&
> +       git checkout -b rename_branch &&
> +       git mv formatting.txt formatting_renamed.txt &&
> +       git commit -m Rename &&
> +       git checkout master &&
> +       rm formatting.txt &&
> +       mv noformatting.txt formatting.txt &&
> +       git commit -a -m Reformat
> +'
> +test_expect_success 'merge' '
> +       git merge rename_branch
> +'
> +test_done
> --
> 1.7.4.1
>

Elijah Newren· May 4, 2011, 02:18 UTC · re: Ciaran · lore

Re: [PATCH RFC] Test for apparent regression of merging renamed files

Hi,

On Tue, May 3, 2011 at 1:15 PM, Ciaran <ciaranj@gmail.com> wrote:

Show 11 quoted lines

> On Mon, May 2, 2011 at 8:04 PM, Ciaran <ciaranj@gmail.com> wrote:
>> Provides a test to cover a case that appears to have been regressed out by:
>>
>>  b2c8c0a762745768e8fb249949801c1aed8f7c1d
>>
>> At this point no tags contain the above commit, but 'master' does.  I'm unsure
>> what is causing the issue, but can see discussion about this fix here:
>>
>> http://git.661346.n2.nabble.com/BUG-merge-recursive-triggered-quot-BUG-quot-td6179353.html
>>
>> Providing a patch to demonstrate the issue and spark discussion.

....

> The original behaviour (successful merge) seems 'better' than the
> current behaviour 'failed merge, 'erroneous' both modified flag' ...
> am I mis-reading this, is this a new non-supported edge case ?

Thanks for the testcase. This is clearly a bug; and a rather easy one to trigger at that. I'll have to take a look, though I suspect it's time to fix the deeper issues Junio pointed out previously...

Elijah