git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] Test for apparent regression of merging renamed files
@ 2011-05-02 19:04 Ciaran
  2011-05-03 19:15 ` Ciaran
  0 siblings, 1 reply; 3+ messages in thread
From: Ciaran @ 2011-05-02 19:04 UTC (permalink / raw)
  To: git, newren, Junio C Hamano

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

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH RFC] Test for apparent regression of merging renamed files
  2011-05-02 19:04 [PATCH RFC] Test for apparent regression of merging renamed files Ciaran
@ 2011-05-03 19:15 ` Ciaran
  2011-05-04  2:18   ` Elijah Newren
  0 siblings, 1 reply; 3+ messages in thread
From: Ciaran @ 2011-05-03 19:15 UTC (permalink / raw)
  To: git, newren, Junio C Hamano

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.

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.



> ---
> 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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RFC] Test for apparent regression of merging renamed files
  2011-05-03 19:15 ` Ciaran
@ 2011-05-04  2:18   ` Elijah Newren
  0 siblings, 0 replies; 3+ messages in thread
From: Elijah Newren @ 2011-05-04  2:18 UTC (permalink / raw)
  To: Ciaran; +Cc: git, Junio C Hamano

Hi,

On Tue, May 3, 2011 at 1:15 PM, Ciaran <ciaranj@gmail.com> wrote:
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-05-04  2:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-02 19:04 [PATCH RFC] Test for apparent regression of merging renamed files Ciaran
2011-05-03 19:15 ` Ciaran
2011-05-04  2:18   ` Elijah Newren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).