[RFC] fix for 335033 and 300001

[Parth Malwankar]

Hello,

I had decided to try and fix two somewhat easy issues
335033 and 300001 [1,2].

The fix[3] is incomplete right now.

The basic idea is that instead of just creating a backup.bzr
file, a backup.bzr.~N~ file is created. This file (*~) is already
ignored by bzr and the N ensures that there is no conflict
with and existing directory.

Two things that still remain to be done are, adding tests
and replacing  os.path.exists below with something that
works with different transports.

Do bzr already have an API that can be used in case of
os.path.exists? If not, just adding a '~' would still fix
335033.
Whats the best place to add tests for this fix.

Is there anything else that I am missing?

Regards,
Parth

[1] https://bugs.launchpad.net/bzr/+bug/335033
[2] https://bugs.launchpad.net/bzr/+bug/335033
[3] https://code.launchpad.net/~parthm/bzr/335033


[bzr.dev]% bzr diff --old trunk --new 335033
=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2010-02-10 17:52:08 +0000
+++ bzrlib/bzrdir.py	2010-02-15 07:29:02 +0000
@@ -575,6 +575,15 @@

         :return: Tuple with old path name and new path name
         """
+        def name_gen(base='backup.bzr'):
+            counter = 1
+            name = "%s.~%d~" % (base, counter)
+            while os.path.exists(name):
+                counter += 1
+                name = "%s.~%d~" % (base, counter)
+            return name
+
+        BACKUP_DIR=name_gen()
         pb = ui.ui_factory.nested_progress_bar()
         try:
             # FIXME: bug 300001 -- the backup fails if the backup directory
@@ -584,9 +593,9 @@
             # FIXME: bug 262450 -- the backup directory should have the same
             # permissions as the .bzr directory (probably a bug in copy_tree)
             old_path = self.root_transport.abspath('.bzr')
-            new_path = self.root_transport.abspath('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,))
-            self.root_transport.copy_tree('.bzr', 'backup.bzr')
+            self.root_transport.copy_tree('.bzr', BACKUP_DIR)
             return (old_path, new_path)
         finally:
             pb.finished()