[merge] cleanups in upgrade

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Tim and I did some work towards doing upgrade on the server side.
> Here is a preliminary patch:
> 
>  * move some code to make backups from upgrade.py in to bzrdir, where
> it can be either customized per implementation or run more cleanly on
> the server
> 
>  * needs_format_conversion(format=None) used to imply upgrading to the
> default format; now it's deprecated.  This is only called from inside
> upgrade (or maybe plugins) so I think the need for convenience is
> questionable, and being explicit is better.
> 
> 

I don't really see the point of adding a progress bar in order to call
"pb.note()". I would guess that you were trying to add progress to
'copy_tree' could you clarify?

- -        self.root_transport.copy_tree('.bzr', 'backup.bzr')
- -        return (self.root_transport.abspath('.bzr'),
- -                self.root_transport.abspath('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.
+            #
+            # 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')
+            pb.note('making backup of %s' % (old_path,))
+            pb.note('  to %s' % (new_path,))
+            self.root_transport.copy_tree('.bzr', 'backup.bzr')
+            return (old_path, new_path)
+        finally:
+            pb.finished()


^- Also, there is a "bug" that pb.note() doesn't log to ~/.bzr.log while
"trace.note()" does, and "trace.note()" knows to clear the progress bars
anyway.

I also would tend to do this as

note("making backup of %s\n to %s", old_path, new_path)

These are pretty minor bits, though.


Ah, I see:
- -    def _backup_control_dir(self):
- -        self.pb.note('making backup of tree history')
- -        old_path, new_path = self.bzrdir.backup_bzrdir()
- -        self.pb.note('%s has been backed up to %s', old_path, new_path)
- -        self.pb.note('if conversion fails, you can move this directory
back to .bzr')
- -        self.pb.note('if it succeeds, you can remove this directory if
you wish')


You did it because the old code used a "pb" rather than trace.note().

Anyway, I think I like the change, though I don't think you really want
to use a progress bar here.

John
=:->

BB:approve

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAklw0K8ACgkQJdeBCYSNAAME0wCfS8rh7RQe49NDTlWu2Rx526V9
MoUAmgNueICbT5MAvBaurJzRnAn5OOMy
=F/IK
-----END PGP SIGNATURE-----