[PATCH] Make sure diff-helper can tell rename/copy in the new diff-raw format.

Subsystems: the rest

DORMANTno replies

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] Make sure diff-helper can tell rename/copy in the new diff-raw format.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:41:58 

This adds tests to make sure that diff-helper can tell renames
from copies using the same "everything but the last one are
copies and the last one is either rename or stay" logic.

Signed-off-by: Junio C Hamano <redacted>
---

t/t4003-diff-rename-1.sh |    6 +--
t/t4005-diff-rename-2.sh |   93 +++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
--- a/t/t4003-diff-rename-1.sh
+++ b/t/t4003-diff-rename-1.sh
@@ -58,7 +58,7 @@ rename new COPYING.#
 EOF
 
 test_expect_success \
-    'validate output from rename/copy detection' \
+    'validate output from rename/copy detection (#1)' \
     'diff -u current expected'
 
 test_expect_success \
@@ -98,7 +98,7 @@ diff --git a/COPYING b/COPYING
 EOF
 
 test_expect_success \
-    'validate output from rename/copy detection' \
+    'validate output from rename/copy detection (#2)' \
     'diff -u current expected'
 
 test_expect_success \
@@ -127,7 +127,7 @@ copy to COPYING.#
 EOF
 
 test_expect_success \
-    'validate output from rename/copy detection' \
+    'validate output from rename/copy detection (#3)' \
     'diff -u current expected'
 
 test_done
diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
--- a/t/t4005-diff-rename-2.sh
+++ b/t/t4005-diff-rename-2.sh
@@ -36,7 +36,42 @@ cat >expected <<\EOF
 EOF
 
 test_expect_success \
-    'validate output from rename/copy detection' \
+    'validate output from rename/copy detection (#1)' \
+    'diff -u current expected'
+
+# make sure diff-helper groks it correctly.
+mv expected raw-output
+GIT_DIFF_OPTS=--unified=0 git-diff-helper <raw-output |
+sed -e 's/\([0-9][0-9]*\)/#/g' >current &&
+cat >expected <<\EOF
+diff --git a/COPYING b/COPYING.#
+similarity index #%
+copy from COPYING
+copy to COPYING.#
+--- a/COPYING
++++ b/COPYING.#
+@@ -# +# @@
+- HOWEVER, in order to allow a migration to GPLv# if that seems like
++ However, in order to allow a migration to GPLv# if that seems like
+diff --git a/COPYING b/COPYING.#
+similarity index #%
+rename old COPYING
+rename new COPYING.#
+--- a/COPYING
++++ b/COPYING.#
+@@ -# +# @@
+- Note that the only valid version of the GPL as far as this project
++ Note that the only valid version of the G.P.L as far as this project
+@@ -# +# @@
+- HOWEVER, in order to allow a migration to GPLv# if that seems like
++ HOWEVER, in order to allow a migration to G.P.Lv# if that seems like
+@@ -# +# @@
+-	This file is licensed under the GPL v#, or a later version
++	This file is licensed under the G.P.L v#, or a later version
+EOF
+
+test_expect_success \
+    'validate output from diff-helper (#1)' \
     'diff -u current expected'
 
 test_expect_success \
@@ -56,7 +91,39 @@ cat >expected <<\EOF
 EOF
 
 test_expect_success \
-    'validate output from rename/copy detection' \
+    'validate output from rename/copy detection (#2)' \
+    'diff -u current expected'
+
+# make sure diff-helper groks it correctly.
+mv expected raw-output
+GIT_DIFF_OPTS=--unified=0 git-diff-helper <raw-output |
+sed -e 's/\([0-9][0-9]*\)/#/g' >current
+cat >expected <<\EOF
+diff --git a/COPYING b/COPYING.#
+similarity index #%
+copy from COPYING
+copy to COPYING.#
+--- a/COPYING
++++ b/COPYING.#
+@@ -# +# @@
+- HOWEVER, in order to allow a migration to GPLv# if that seems like
++ However, in order to allow a migration to GPLv# if that seems like
+diff --git a/COPYING b/COPYING
+--- a/COPYING
++++ b/COPYING
+@@ -# +# @@
+- Note that the only valid version of the GPL as far as this project
++ Note that the only valid version of the G.P.L as far as this project
+@@ -# +# @@
+- HOWEVER, in order to allow a migration to GPLv# if that seems like
++ HOWEVER, in order to allow a migration to G.P.Lv# if that seems like
+@@ -# +# @@
+-	This file is licensed under the GPL v#, or a later version
++	This file is licensed under the G.P.L v#, or a later version
+EOF
+
+test_expect_success \
+    'validate output from diff-helper (#2)' \
     'diff -u current expected'
 
 test_expect_success \
@@ -76,7 +143,27 @@ cat >expected <<\EOF
 EOF
 
 test_expect_success \
-    'validate output from rename/copy detection' \
+    'validate output from rename/copy detection (#3)' \
+    'diff -u current expected'
+
+# make sure diff-helper groks it correctly.
+mv expected raw-output
+GIT_DIFF_OPTS=--unified=0 git-diff-helper <raw-output |
+sed -e 's/\([0-9][0-9]*\)/#/g' >current
+cat >expected <<\EOF
+diff --git a/COPYING b/COPYING.#
+similarity index #%
+copy from COPYING
+copy to COPYING.#
+--- a/COPYING
++++ b/COPYING.#
+@@ -# +# @@
+- HOWEVER, in order to allow a migration to GPLv# if that seems like
++ However, in order to allow a migration to GPLv# if that seems like
+EOF
+
+test_expect_success \
+    'validate output from diff-helper (#3)' \
     'diff -u current expected'
 
 test_done
------------------------------------------------

Re: [PATCH] Make sure diff-helper can tell rename/copy in the new diff-raw format.

From: Linus Torvalds <torvalds@osdl.org>
Date: 2016-06-15 22:41:58 


On Mon, 23 May 2005, Junio C Hamano wrote:
This adds tests to make sure that diff-helper can tell renames
from copies using the same "everything but the last one are
copies and the last one is either rename or stay" logic.
Btw, I still disagree with this notion that the order of the use of the 
names makes a difference.

I think that when we generate a diff, we should always have the _option_
to make sure that we generate it in a format where you can apply it
"incrementally". But I disagree with the notion that it's something
fundamental, and in fact, I even think that it's not necessarily a good
default.

For example, let's say that you have modified "fileA" _and_ you have 
created a "fileB" that is a copy of the original "fileA" with some _other_ 
slight modifications. We'll call the SHA1's involved "sha_A", "sha_A'" and 
"sha_B"

I think it's perfectly valid to say

	:100644 100644 <sha_A> <sha_A'> M	fileA	fileA
	:100644 100644 <sha_A> <sha_B> C89	fileA	fileB

which says "fileA" was modified from orig-A to new-A, and "fileB" is a 
copy based on orig-A.

(I've used a new syntax just to confuse the issue, with the extra field 
for "what happened", aka "M" for "modify", "C89" for "copy a 89% similar 
file").

Now, when the above is turned into a "diff", that diff is no longer
something you can apply "incrementally" - you have to apply it as if
you're applying all differences to the "original tree". But the thing is,
that's actually what I _want_, because I was planning on writing a tool
that applies patches that applies them all-or-nothing.

Also, it turns out that this kind of "non-incremental" diff is the kind
that I personally want to see as a _human_, because quite frankly, my
brain-capacity is that of a demented ocelot, and I can't _remember_ what
happened in other parts of the diff. I much prefer the stateless "oh, this
file X is in that relation Y to the previous version of file Z".

I do that partly because I actually routinely edit patches. If you have 
the incremental format, that's practically impossible, while the stateless 
version is fine.

See?

So I think all the clever "don't re-use files we have modified" etc is 
actually wrong. If you want to make a traditional diff that can be applied 
with normal "patch", you just don't use the -M or -C flags.

		Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help