Rev 2408: (John Arbash Meinel) Fix bug #103870 by teaching show_diff_trees about modified files in renamed directories in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Apr 12 04:20:21 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2408
revision-id: pqm at pqm.ubuntu.com-20070412032019-axeg1wmcju0odfdr
parent: pqm at pqm.ubuntu.com-20070412005953-vow66znisuxhrauk
parent: john at arbash-meinel.com-20070411220839-0eyz5nrq4vv1chcb
committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2007-04-12 04:20:19 +0100
message:
  (John Arbash Meinel) Fix bug #103870 by teaching show_diff_trees about modified files in renamed directories
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/diff.py                 diff.py-20050309040759-26944fbbf2ebbf36
  bzrlib/tests/test_diff.py      testdiff.py-20050727164403-d1a3496ebb12e339
    ------------------------------------------------------------
    revno: 2405.1.4
    merged: john at arbash-meinel.com-20070411220839-0eyz5nrq4vv1chcb
    parent: john at arbash-meinel.com-20070411220516-bz4941rayeilvjac
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: diff_renamed_103870
    timestamp: Wed 2007-04-11 17:08:39 -0500
    message:
      NEWS for fixing bug #103870
    ------------------------------------------------------------
    revno: 2405.1.3
    merged: john at arbash-meinel.com-20070411220516-bz4941rayeilvjac
    parent: john at arbash-meinel.com-20070411220215-o07i1rcajpghkpom
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: diff_renamed_103870
    timestamp: Wed 2007-04-11 17:05:16 -0500
    message:
      Do a better fix, which recognizes that we should pass the correct old path.
    ------------------------------------------------------------
    revno: 2405.1.2
    merged: john at arbash-meinel.com-20070411220215-o07i1rcajpghkpom
    parent: john at arbash-meinel.com-20070411215736-rii53nkqb42hezup
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: diff_renamed_103870
    timestamp: Wed 2007-04-11 17:02:15 -0500
    message:
      Fix bug #103870 by passing None instead of a (sometimes wrong) path
    ------------------------------------------------------------
    revno: 2405.1.1
    merged: john at arbash-meinel.com-20070411215736-rii53nkqb42hezup
    parent: pqm at pqm.ubuntu.com-20070411044855-b83c4dc6fd093648
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: diff_renamed_103870
    timestamp: Wed 2007-04-11 16:57:36 -0500
    message:
      Add a bunch of direct tests for 'show_diff_trees'
=== modified file 'NEWS'
--- a/NEWS	2007-04-11 01:29:20 +0000
+++ b/NEWS	2007-04-11 22:08:39 +0000
@@ -61,6 +61,11 @@
       (with an ancestry of 16k revisions, ``bzr merge ../already-merged``
       changes from 2m10s to 13s).  (John Arbash Meinel, #103757)
 
+    * Fix ``show_diff_trees`` to handle the case when a file is modified,
+      and the containing directory is renamed. (The file path is different
+      in this versus base, but it isn't marked as a rename).
+      (John Arbash Meinel, #103870)
+
   TESTING:
 
     * Added ``bzrlib.strace.strace`` which will strace a single callable and

=== modified file 'bzrlib/diff.py'
--- a/bzrlib/diff.py	2007-03-13 02:16:17 +0000
+++ b/bzrlib/diff.py	2007-04-11 22:05:16 +0000
@@ -471,8 +471,11 @@
         has_changes = 1
         prop_str = get_prop_change(meta_modified)
         print >>to_file, '=== modified %s %r%s' % (kind, path.encode('utf8'), prop_str)
-        old_name = '%s%s\t%s' % (old_label, path,
-                                 _patch_header_date(old_tree, file_id, path))
+        # The file may be in a different location in the old tree (because
+        # the containing dir was renamed, but the file itself was not)
+        old_path = old_tree.id2path(file_id)
+        old_name = '%s%s\t%s' % (old_label, old_path,
+                                 _patch_header_date(old_tree, file_id, old_path))
         new_name = '%s%s\t%s' % (new_label, path,
                                  _patch_header_date(new_tree, file_id, path))
         if text_modified:
@@ -485,7 +488,11 @@
 
 def _patch_header_date(tree, file_id, path):
     """Returns a timestamp suitable for use in a patch header."""
-    return timestamp.format_patch_date(tree.get_file_mtime(file_id, path))
+    mtime = tree.get_file_mtime(file_id, path)
+    assert mtime is not None, \
+        "got an mtime of None for file-id %s, path %s in tree %s" % (
+                file_id, path, tree)
+    return timestamp.format_patch_date(mtime)
 
 
 def _raise_if_nonexistent(paths, old_tree, new_tree):

=== modified file 'bzrlib/tests/test_diff.py'
--- a/bzrlib/tests/test_diff.py	2007-03-11 19:39:37 +0000
+++ b/bzrlib/tests/test_diff.py	2007-04-11 22:05:16 +0000
@@ -214,7 +214,22 @@
         self.assertEqual(out.splitlines(True) + ['\n'], lines)
 
 
-class TestDiffDates(TestCaseWithTransport):
+class TestShowDiffTreesHelper(TestCaseWithTransport):
+    """Has a helper for running show_diff_trees"""
+
+    def get_diff(self, tree1, tree2, specific_files=None, working_tree=None):
+        output = StringIO()
+        if working_tree is not None:
+            extra_trees = (working_tree,)
+        else:
+            extra_trees = ()
+        show_diff_trees(tree1, tree2, output, specific_files=specific_files,
+                        extra_trees=extra_trees, old_label='old/',
+                        new_label='new/')
+        return output.getvalue()
+
+
+class TestDiffDates(TestShowDiffTreesHelper):
 
     def setUp(self):
         super(TestDiffDates, self).setUp()
@@ -254,17 +269,6 @@
         # set the date stamps for files in the working tree to known values
         os.utime('file1', (1144195200, 1144195200)) # 2006-04-05 00:00:00 UTC
 
-    def get_diff(self, tree1, tree2, specific_files=None, working_tree=None):
-        output = StringIO()
-        if working_tree is not None:
-            extra_trees = (working_tree,)
-        else:
-            extra_trees = ()
-        show_diff_trees(tree1, tree2, output, specific_files=specific_files,
-                        extra_trees=extra_trees, old_label='old/', 
-                        new_label='new/')
-        return output.getvalue()
-
     def test_diff_rev_tree_working_tree(self):
         output = self.get_diff(self.wt.basis_tree(), self.wt)
         # note that the date for old/file1 is from rev 2 rather than from
@@ -354,6 +358,90 @@
         self.assertNotContainsRe(out, 'file1\t')
 
 
+
+class TestShowDiffTrees(TestShowDiffTreesHelper):
+    """Direct tests for show_diff_trees"""
+
+    def test_modified_file(self):
+        """Test when a file is modified."""
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree_contents([('tree/file', 'contents\n')])
+        tree.add(['file'], ['file-id'])
+        tree.commit('one', rev_id='rev-1')
+
+        self.build_tree_contents([('tree/file', 'new contents\n')])
+        diff = self.get_diff(tree.basis_tree(), tree)
+        self.assertContainsRe(diff, "=== modified file 'file'\n")
+        self.assertContainsRe(diff, '--- old/file\t')
+        self.assertContainsRe(diff, '\\+\\+\\+ new/file\t')
+        self.assertContainsRe(diff, '-contents\n'
+                                    '\\+new contents\n')
+
+    def test_modified_file_in_renamed_dir(self):
+        """Test when a file is modified in a renamed directory."""
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree(['tree/dir/'])
+        self.build_tree_contents([('tree/dir/file', 'contents\n')])
+        tree.add(['dir', 'dir/file'], ['dir-id', 'file-id'])
+        tree.commit('one', rev_id='rev-1')
+
+        tree.rename_one('dir', 'other')
+        self.build_tree_contents([('tree/other/file', 'new contents\n')])
+        diff = self.get_diff(tree.basis_tree(), tree)
+        self.assertContainsRe(diff, "=== renamed directory 'dir' => 'other'\n")
+        self.assertContainsRe(diff, "=== modified file 'other/file'\n")
+        # XXX: This is technically incorrect, because it used to be at another
+        # location. What to do?
+        self.assertContainsRe(diff, '--- old/dir/file\t')
+        self.assertContainsRe(diff, '\\+\\+\\+ new/other/file\t')
+        self.assertContainsRe(diff, '-contents\n'
+                                    '\\+new contents\n')
+
+    def test_renamed_directory(self):
+        """Test when only a directory is only renamed."""
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree(['tree/dir/'])
+        self.build_tree_contents([('tree/dir/file', 'contents\n')])
+        tree.add(['dir', 'dir/file'], ['dir-id', 'file-id'])
+        tree.commit('one', rev_id='rev-1')
+
+        tree.rename_one('dir', 'newdir')
+        diff = self.get_diff(tree.basis_tree(), tree)
+        # Renaming a directory should be a single "you renamed this dir" even
+        # when there are files inside.
+        self.assertEqual("=== renamed directory 'dir' => 'newdir'\n", diff)
+
+    def test_renamed_file(self):
+        """Test when a file is only renamed."""
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree_contents([('tree/file', 'contents\n')])
+        tree.add(['file'], ['file-id'])
+        tree.commit('one', rev_id='rev-1')
+
+        tree.rename_one('file', 'newname')
+        diff = self.get_diff(tree.basis_tree(), tree)
+        self.assertContainsRe(diff, "=== renamed file 'file' => 'newname'\n")
+        # We shouldn't have a --- or +++ line, because there is no content
+        # change
+        self.assertNotContainsRe(diff, '---')
+
+    def test_renamed_and_modified_file(self):
+        """Test when a file is only renamed."""
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree_contents([('tree/file', 'contents\n')])
+        tree.add(['file'], ['file-id'])
+        tree.commit('one', rev_id='rev-1')
+
+        tree.rename_one('file', 'newname')
+        self.build_tree_contents([('tree/newname', 'new contents\n')])
+        diff = self.get_diff(tree.basis_tree(), tree)
+        self.assertContainsRe(diff, "=== renamed file 'file' => 'newname'\n")
+        self.assertContainsRe(diff, '--- old/file\t')
+        self.assertContainsRe(diff, '\\+\\+\\+ new/newname\t')
+        self.assertContainsRe(diff, '-contents\n'
+                                    '\\+new contents\n')
+
+
 class TestPatienceDiffLib(TestCase):
 
     def test_unique_lcs(self):




More information about the bazaar-commits mailing list