launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:diff-detect-renames into turnip:master.

Commit message:
Detect renames when generating diffs

LP: #1712754

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1712754 in turnip: "git diffs do not track renames"
  https://bugs.launchpad.net/turnip/+bug/1712754

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/330036
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:diff-detect-renames into turnip:master.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 1291c13..5767958 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -362,16 +362,17 @@ def get_merge_diff(repo_store, repo_name, sha1_base,
             from_tree = repo[sha1_base].tree
         merged_index = repo.merge_commits(sha1_base, sha1_head)
         conflicts = _add_conflicted_files(repo, merged_index)
-        patch = merged_index.diff_to_tree(
-            from_tree, context_lines=context_lines).patch
+        diff = merged_index.diff_to_tree(
+            from_tree, context_lines=context_lines)
+        diff.find_similar()
+        patch = diff.patch
         if patch is None:
             patch = u''
         shas = [sha1_base, sha1_head]
         commits = [get_commit(repo_store, repo_name, sha, repo)
                    for sha in shas]
-        diff = {'commits': commits, 'patch': patch,
+        return {'commits': commits, 'patch': patch,
                 'conflicts': sorted(conflicts)}
-        return diff
 
 
 def get_diff(repo_store, repo_name, sha1_from, sha1_to, context_lines=3):
@@ -385,16 +386,17 @@ def get_diff(repo_store, repo_name, sha1_from, sha1_to, context_lines=3):
         shas = [sha1_from, sha1_to]
         commits = [get_commit(repo_store, repo_name, sha, repo)
                    for sha in shas]
-        patch = repo.diff(
+        diff = repo.diff(
             commits[0]['sha1'], commits[1]['sha1'], False, 0,
-            context_lines).patch
+            context_lines)
+        diff.find_similar()
+        patch = diff.patch
         if patch is None:
             patch = u''
-        diff = {
+        return {
             'commits': commits,
             'patch': patch,
         }
-        return diff
 
 
 def get_log(repo_store, repo_name, start=None, limit=None, stop=None):
diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
index 065b067..4e3c808 100644
--- a/turnip/api/tests/test_api.py
+++ b/turnip/api/tests/test_api.py
@@ -361,6 +361,19 @@ class ApiTestCase(TestCase):
         self.assertIn(b'-foo', resp.body)
         self.assertIn(b'+bar', resp.body)
 
+    def test_repo_diff_detects_renames(self):
+        """get_diff finds renamed files."""
+        repo = RepoFactory(self.repo_store)
+        c1 = repo.add_commit('foo\n', 'foo.txt')
+        repo.repo.index.remove('foo.txt')
+        c2 = repo.add_commit('foo\n', 'bar.txt', parents=[c1])
+
+        path = '/repo/{}/compare/{}..{}'.format(
+            self.repo_path, quote('{}^'.format(c2)), c2)
+        resp = self.app.get(path)
+        self.assertIn(
+            b'diff --git a/foo.txt b/bar.txt\n', resp.json_body['patch'])
+
     def test_repo_diff_merge(self):
         """Ensure expected changes exist in diff patch."""
         repo = RepoFactory(self.repo_store)
@@ -438,6 +451,18 @@ class ApiTestCase(TestCase):
             self.repo_path, repo.nonexistent_oid(), c1), expect_errors=True)
         self.assertEqual(404, resp.status_code)
 
+    def test_repo_diff_merge_detects_renames(self):
+        """get_merge_diff finds renamed files."""
+        repo = RepoFactory(self.repo_store)
+        c1 = repo.add_commit('foo\n', 'foo.txt')
+        repo.repo.index.remove('foo.txt')
+        c2 = repo.add_commit('foo\n', 'bar.txt', parents=[c1])
+
+        resp = self.app.get('/repo/{}/compare-merge/{}:{}'.format(
+            self.repo_path, c1, c2))
+        self.assertIn(
+            b'diff --git a/foo.txt b/bar.txt\n', resp.json_body['patch'])
+
     def test_repo_get_commit(self):
         factory = RepoFactory(self.repo_store)
         message = 'Computers make me angry.'