[PATCH 0/4] Support threshold in copy/rename detection

[PATCH 0/4] Support threshold in copy/rename detection

[Vitor Antunes]

Add possibility of configuring a threshold in the copy and rename detections.
Copy and rename detection options were not documented and no test cases existed
for it. This is corrected in this set of patches.

Vitor Antunes (4):
  git-p4: Allow setting rename/copy detection threshold.
  git-p4: Add description of rename/copy detection options.
  git-p4: Add test case for rename detection.
  git-p4: Add test case for copy detection.

 contrib/fast-import/git-p4     |   18 ++++--
 contrib/fast-import/git-p4.txt |   25 ++++++++
 t/t9800-git-p4.sh              |  121 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+), 5 deletions(-)

-- 
1.7.5.4

[PATCH 1/4] git-p4: Allow setting rename/copy detection threshold.

[Vitor Antunes]

Copy and rename detection arguments (-C and -M) allow setting a threshold value
for the similarity ratio. If the similarity is below this threshold the rename
or copy is ignored and the file is added as new.
This patch allows setting git-p4.detectRenames and git-p4.detectCopies options
to an integer value to set the respective threshold.

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4 |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 6b9de9e..cf719be 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -774,15 +774,23 @@ class P4Submit(Command, P4UserMap):
 
         if not self.detectRenames:
             # If not explicitly set check the config variable
-            self.detectRenames = gitConfig("git-p4.detectRenames").lower() == "true"
+            self.detectRenames = gitConfig("git-p4.detectRenames")
 
-        if self.detectRenames:
+        diffOpts = ""
+        if self.detectRenames.lower() == "true":
             diffOpts = "-M"
-        else:
-            diffOpts = ""
+        elif self.detectRenames != "":
+            self.detectRenames = int(self.detectRenames)
+            if self.detectRenames >= 0 and self.detectRenames <= 100:
+                diffOpts = "-M%d" % self.detectRenames
 
-        if gitConfig("git-p4.detectCopies").lower() == "true":
+        detectCopies = gitConfig("git-p4.detectCopies")
+        if detectCopies.lower() == "true":
             diffOpts += " -C"
+        elif detectCopies != "":
+            detectCopies = int(detectCopies)
+            if detectCopies >= 0 and detectCopies <= 100:
+                diffOpts += " -C%d" % detectCopies
 
         if gitConfig("git-p4.detectCopiesHarder").lower() == "true":
             diffOpts += " --find-copies-harder"
-- 
1.7.5.4

[PATCH 2/4] git-p4: Add description of rename/copy detection options.

[Vitor Antunes]

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4.txt |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index caa4bb3..2ffbccc 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -232,6 +232,31 @@ git-p4.skipUserNameCheck
 When submitting, git-p4 checks that the git commits are authored by the current
 p4 user, and warns if they are not. This disables the check.
 
+git-p4.detectRenames
+
+Detect renames when submitting changes to Perforce server. Will enable -M git
+argument. Can be optionally set to a number representing the threshold
+percentage value of the rename detection.
+
+  git config [--global] git-p4.detectRenames true
+  git config [--global] git-p4.detectRenames 50
+
+git-p4.detectCopies
+
+Detect copies when submitting changes to Perforce server. Will enable -C git
+argument. Can be optionally set to a number representing the threshold
+percentage value of the copy detection.
+
+  git config [--global] git-p4.detectCopies true
+  git config [--global] git-p4.detectCopies 80
+
+git-p4.detectCopiesHarder
+
+Detect copies even between files that did not change when submitting changes to
+Perforce server. Will enable --find-copies-harder git argument.
+
+  git config [--global] git-p4.detectCopies true
+
 Implementation Details...
 =========================
 
-- 
1.7.5.4

[PATCH 3/4] git-p4: Add test case for rename detection.

[Vitor Antunes]

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 t/t9800-git-p4.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 97ec975..d01a1cb 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -269,6 +269,56 @@ test_expect_success 'initial import time from top change time' '
 	test $p4time = $gittime
 '
 
+# Rename a file and confirm that rename is not detected in P4.
+# Rename the new file again with detectRenames option enabled and confirm that
+# this is detected in P4.
+# Rename the new file again adding an extra blank line, configure a big
+# threshold in detectRenames and confirm that rename is not detected in P4.
+# Rename the new file again adding another extra blank line, configure a small
+# threshold in detectRenames and confirm that rename is not detected in P4.
+test_expect_success 'detect renames' '
+	git init "$git" &&
+	cd "$git" &&
+	cd "$TRASH_DIRECTORY" &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	p4 files //depot/* &&
+	cd "$git" &&
+	git mv file1 file4 &&
+	git commit -a -m "Rename file1 to file4" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.skipSubmitEditCheck true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file4 &&
+	! p4 filelog //depot/file4 | grep -q "branch from //depot/file1" &&
+	git mv file4 file5 &&
+	git commit -a -m "Rename file4 to file5" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.detectRenames true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file5 &&
+	p4 filelog //depot/file5 | grep -q "branch from //depot/file4" &&
+	git mv file5 file6 &&
+	echo update >> file6 &&
+	git add file6 &&
+	git commit -a -m "Rename file5 to file6 with changes" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.detectRenames 95 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file6 &&
+	! p4 filelog //depot/file6 | grep -q "branch from //depot/file5" &&
+	git mv file6 file7 &&
+	echo update >> file7 &&
+	git add file7 &&
+	git commit -a -m "Rename file6 to file7 with changes" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.detectRenames 80 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file7 &&
+	p4 filelog //depot/file7 | grep -q "branch from //depot/file6" &&
+	cd "$TRASH_DIRECTORY" &&
+	rm -rf "$git" && mkdir "$git"
+'
+
 test_expect_success 'shutdown' '
 	pid=`pgrep -f p4d` &&
 	test -n "$pid" &&
-- 
1.7.5.4

[PATCH 4/4] git-p4: Add test case for copy detection.

[Vitor Antunes]

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 t/t9800-git-p4.sh |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index d01a1cb..a4f3d66 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -319,6 +319,77 @@ test_expect_success 'detect renames' '
 	rm -rf "$git" && mkdir "$git"
 '
 
+# Copy a file and confirm that copy is not detected in P4.
+# Copy a file with detectCopies option enabled and confirm that copy is not
+# detected in P4.
+# Modify and copy a file with detectCopies option enabled and confirm that copy
+# is detected in P4.
+# Copy a file with detectCopies and detectCopiesHarder options enabled and
+# confirm that copy is detected in P4.
+# Modify and copy a file, configure a big threshold in detectCopies and confirm
+# that copy is not detected in P4.
+# Modify and copy a file, configure a small threshold in detectCopies and
+# confirm that copy is detected in P4.
+test_expect_success 'detect copies' '
+	git init "$git" &&
+	cd "$git" &&
+	cd "$TRASH_DIRECTORY" &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	cd "$git" &&
+	cp file2 file8 &&
+	git add file8 &&
+	git commit -a -m "Copy file2 to file8" &&
+	git diff-tree -r -C HEAD
+	git config git-p4.skipSubmitEditCheck true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file8 &&
+	! p4 filelog //depot/file8 | grep -q "branch from //depot/file" &&
+	cp file2 file9 &&
+	git add file9 &&
+	git commit -a -m "Copy file2 to file9" &&
+	git diff-tree -r -C HEAD
+	git config git-p4.detectCopies true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file9 &&
+	! p4 filelog //depot/file9 | grep -q "branch from //depot/file" &&
+	echo file2 >> file2 &&
+	cp file2 file10 &&
+	git add file2 file10 &&
+	git commit -a -m "Modify and copy file2 to file10" &&
+	git diff-tree -r -C HEAD
+	"$GITP4" submit &&
+	p4 filelog //depot/file10 &&
+	p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
+	cp file2 file11 &&
+	git add file11 &&
+	git commit -a -m "Copy file2 to file11" &&
+	git diff-tree -r -C --find-copies-harder HEAD
+	git config git-p4.detectCopiesHarder true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file11 &&
+	p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
+	cp file2 file12 &&
+	echo >> file12 &&
+	git add file12 &&
+	git commit -a -m "Copy file2 to file12 with changes" &&
+	git diff-tree -r -C --find-copies-harder HEAD
+	git config git-p4.detectCopies 98 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file12 &&
+	! p4 filelog //depot/file12 | grep -q "branch from //depot/file" &&
+	cp file2 file13 &&
+	echo >> file13 &&
+	git add file13 &&
+	git commit -a -m "Copy file2 to file13 with changes" &&
+	git diff-tree -r -C --find-copies-harder HEAD
+	git config git-p4.detectCopies 80 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file13 &&
+	p4 filelog //depot/file13 | grep -q "branch from //depot/file" &&
+	cd "$TRASH_DIRECTORY" &&
+	rm -rf "$git" && mkdir "$git"
+'
+
 test_expect_success 'shutdown' '
 	pid=`pgrep -f p4d` &&
 	test -n "$pid" &&
-- 
1.7.5.4

Re: [PATCH 1/4] git-p4: Allow setting rename/copy detection threshold.

[Pete Wyckoff]

vitor.hda@gmail.com wrote on Thu, 18 Aug 2011 02:04 +0100:
> Copy and rename detection arguments (-C and -M) allow setting a threshold value
> for the similarity ratio. If the similarity is below this threshold the rename
> or copy is ignored and the file is added as new.
> This patch allows setting git-p4.detectRenames and git-p4.detectCopies options
> to an integer value to set the respective threshold.
[..]
> -        if self.detectRenames:
> +        diffOpts = ""
> +        if self.detectRenames.lower() == "true":
>              diffOpts = "-M"
> -        else:
> -            diffOpts = ""
> +        elif self.detectRenames != "":
> +            self.detectRenames = int(self.detectRenames)
> +            if self.detectRenames >= 0 and self.detectRenames <= 100:
> +                diffOpts = "-M%d" % self.detectRenames
>  

This is a good idea.  But I notice that -M, -C, -B can take
a diverse set of option forms, including floating point and
optional "%".  "-B" can take two args, separated by a "/".

See diff_scoreopt_parse().  It would be weird for git-p4 to limit
that set to integers like you do here.

Would you be willing to change it to pass the string blindly?
Just filter out the special values, and pass everything else
unchanged.

    "true" -> "-M"
    "false" -> ""
    "" -> ""
    ".*" -> "\1"

Dunno if you care about --break-rewrites.  Just pointing to it
as a similar option to -M and -C in case you missed it.

		-- Pete

Re: [PATCH 1/4] git-p4: Allow setting rename/copy detection threshold.

[Vitor Antunes]

On Thu, Aug 18, 2011 at 4:18 PM, Pete Wyckoff <pw@padd.com> wrote:
> This is a good idea.  But I notice that -M, -C, -B can take
> a diverse set of option forms, including floating point and
> optional "%".  "-B" can take two args, separated by a "/".
>
> See diff_scoreopt_parse().  It would be weird for git-p4 to limit
> that set to integers like you do here.
>
> Would you be willing to change it to pass the string blindly?
> Just filter out the special values, and pass everything else
> unchanged.
>
>    "true" -> "-M"
>    "false" -> ""
>    "" -> ""
>    ".*" -> "\1"

This makes sense. I did notice that -M and -C supported, at least, a
'%' char at the end. But I decided to ignore it. I think your is idea
is not only good, but also a simpler solution. I'll send a new set of
patches shortly with this implemented.

> Dunno if you care about --break-rewrites.  Just pointing to it
> as a similar option to -M and -C in case you missed it.

I was completely unaware of this option, so I don't feel comfortable
in adding it as a configuration option for now. Will need to
understand it first ;)

-- 
Vitor Antunes