Rev 5415: Deprecate BzrDir.generate_backup_name and use osutils.available_backup_name. in file:///home/vila/src/bzr/bugs/323111-orphans/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/bugs/323111-orphans/

------------------------------------------------------------
revno: 5415
revision-id: v.ladeuil+lp at free.fr-20100909130255-ppyxbb3q137skxbf
parent: v.ladeuil+lp at free.fr-20100909092733-yn70gnd11vfwyp30
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: backup-names
timestamp: Thu 2010-09-09 15:02:55 +0200
message:
  Deprecate BzrDir.generate_backup_name and use osutils.available_backup_name.
  
  * bzrlib/tests/test_bzrdir.py:
  (TestGenerateBackupName.test_deprecated_generate_backup_name): Added.
  
  * bzrlib/workingtree.py:
  (WorkingTree.remove.backup): Fix line too long in a call site
  
  * bzrlib/bzrdir.py:
  (BzrDir.generate_backup_name): Deperecated.
  (BzrDir._available_backup_name): Renamed to better hint about race
  conditions and implementation delegated to
  osutils.available_backup_name.
  (BzrDir.backup_bzrdir): Cleaned up.
-------------- next part --------------
=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py	2010-08-25 02:16:31 +0000
+++ b/bzrlib/bzrdir.py	2010-09-09 13:02:55 +0000
@@ -86,6 +86,10 @@
     registry,
     symbol_versioning,
     )
+from bzrlib.symbol_versioning import (
+    deprecated_in,
+    deprecated_method,
+    )
 
 
 class BzrDir(controldir.ControlDir):
@@ -515,14 +519,13 @@
                                                format=format).bzrdir
         return bzrdir.create_workingtree()
 
+    @deprecated_method(deprecated_in((2, 3, 0)))
     def generate_backup_name(self, base):
-        """Generate a non-existing backup file name based on base."""
-        counter = 1
-        name = "%s.~%d~" % (base, counter)
-        while self.root_transport.has(name):
-            counter += 1
-            name = "%s.~%d~" % (base, counter)
-        return name
+        return self._available_backup_name(base)
+
+    def _available_backup_name(self, base):
+        """Find a non-existing backup file name based on base."""
+        return osutils.available_backup_name(base, self.root_transport.has)
 
     def backup_bzrdir(self):
         """Backup this bzr control directory.
@@ -530,16 +533,13 @@
         :return: Tuple with old path name and new path name
         """
 
-        backup_dir=self.generate_backup_name('backup.bzr')
         pb = ui.ui_factory.nested_progress_bar()
         try:
-            # FIXME: bug 300001 -- the backup fails if the backup directory
-            # already exists, but it should instead either remove it or make
-            # a new backup directory.
-            #
             old_path = self.root_transport.abspath('.bzr')
+            backup_dir = self._available_backup_name('backup.bzr')
             new_path = self.root_transport.abspath(backup_dir)
-            ui.ui_factory.note('making backup of %s\n  to %s' % (old_path, new_path,))
+            ui.ui_factory.note('making backup of %s\n  to %s'
+                               % (old_path, new_path,))
             self.root_transport.copy_tree('.bzr', backup_dir)
             return (old_path, new_path)
         finally:

=== modified file 'bzrlib/tests/test_bzrdir.py'
--- a/bzrlib/tests/test_bzrdir.py	2010-08-20 19:07:17 +0000
+++ b/bzrlib/tests/test_bzrdir.py	2010-09-09 13:02:55 +0000
@@ -32,6 +32,7 @@
     repository,
     osutils,
     remote,
+    symbol_versioning,
     urlutils,
     win32utils,
     workingtree,
@@ -1417,6 +1418,9 @@
 
 
 class TestGenerateBackupName(TestCaseWithMemoryTransport):
+    # FIXME: This may need to be unified with test_osutils.TestBackupNames or
+    # moved to per_bzrdir or per_transport for better coverage ?
+    # -- vila 20100909
 
     def setUp(self):
         super(TestGenerateBackupName, self).setUp()
@@ -1425,9 +1429,14 @@
             possible_transports=[self._transport])
         self._bzrdir = bzrdir.BzrDir.open_from_transport(self._transport)
 
+    def test_deprecated_generate_backup_name(self):
+        res = self.applyDeprecated(
+                symbol_versioning.deprecated_in((2, 3, 0)),
+                self._bzrdir.generate_backup_name, 'whatever')
+
     def test_new(self):
-        self.assertEqual("a.~1~", self._bzrdir.generate_backup_name("a"))
+        self.assertEqual("a.~1~", self._bzrdir._available_backup_name("a"))
 
     def test_exiting(self):
         self._transport.put_bytes("a.~1~", "some content")
-        self.assertEqual("a.~2~", self._bzrdir.generate_backup_name("a"))
+        self.assertEqual("a.~2~", self._bzrdir._available_backup_name("a"))

=== modified file 'bzrlib/workingtree.py'
--- a/bzrlib/workingtree.py	2010-08-20 19:07:17 +0000
+++ b/bzrlib/workingtree.py	2010-09-09 13:02:55 +0000
@@ -2078,9 +2078,10 @@
                     files_to_backup.append(path[1])
 
         def backup(file_to_backup):
-            backup_name = self.bzrdir.generate_backup_name(file_to_backup)
+            backup_name = self.bzrdir._available_backup_name(file_to_backup)
             osutils.rename(abs_path, self.abspath(backup_name))
-            return "removed %s (but kept a copy: %s)" % (file_to_backup, backup_name)
+            return "removed %s (but kept a copy: %s)" % (file_to_backup,
+                                                         backup_name)
 
         # Build inv_delta and delete files where applicable,
         # do this before any modifications to inventory.