Merge lp:~jelmer/brz/upload-symlinks into lp:brz

[Jelmer Vernooij]

Add symlink support to 'brz upload'.

Add symlink support to 'brz upload'.

[Vincent Ladeuil (vila)]

Cool, thanks !

review: Approve

=== modified file 'breezy/plugins/upload/cmds.py'
--- breezy/plugins/upload/cmds.py	2018-03-04 15:59:37 +0000
+++ breezy/plugins/upload/cmds.py	2018-09-17 00:27:30 +0000
@@ -24,6 +24,7 @@
     config,
     lazy_import,
     option,
+    osutils,
     )
 lazy_import.lazy_import(globals(), """
 import stat
@@ -184,24 +185,47 @@
             self.outf.write('Uploading %s\n' % old_relpath)
         self._up_put_bytes(old_relpath, self.tree.get_file_text(new_relpath, id), mode)
 
+    def _force_clear(self, relpath):
+        try:
+            st = self._up_stat(relpath)
+            if stat.S_ISDIR(st.st_mode):
+                # A simple rmdir may not be enough
+                if not self.quiet:
+                    self.outf.write('Clearing %s/%s\n' % (
+                            self.to_transport.external_url(), relpath))
+                self._up_delete_tree(relpath)
+            elif stat.S_ISLNK(st.st_mode):
+                if not self.quiet:
+                    self.outf.write('Clearing %s/%s\n' % (
+                        self.to_transport.external_url(), relpath))
+                self._up_delete(relpath)
+        except errors.PathError:
+            pass
+
     def upload_file_robustly(self, relpath, id, mode=None):
         """Upload a file, clearing the way on the remote side.
 
         When doing a full upload, it may happen that a directory exists where
         we want to put our file.
         """
-        try:
-            st = self._up_stat(relpath)
-            if stat.S_ISDIR(st.st_mode):
-                # A simple rmdir may not be enough
-                if not self.quiet:
-                    self.outf.write('Clearing %s/%s\n' % (
-                            self.to_transport.external_url(), relpath))
-                self._up_delete_tree(relpath)
-        except errors.PathError:
-            pass
+        self._force_clear(relpath)
         self.upload_file(relpath, relpath, id, mode)
 
+    def upload_symlink(self, relpath, target):
+        self.to_transport.symlink(target, relpath)
+
+    def upload_symlink_robustly(self, relpath, target):
+        """Handle uploading symlinks.
+        """
+        self._force_clear(relpath)
+        # Target might not be there at this time; dummy file should be
+        # overwritten at some point, possibly by another upload.
+        target = osutils.normpath(osutils.pathjoin(
+            osutils.dirname(relpath),
+            target)
+        )
+        self.upload_symlink(relpath, target)
+
     def make_remote_dir(self, relpath, mode=None):
         if mode is None:
             mode = 0o775
@@ -303,13 +327,16 @@
                     continue
                 if ie.kind == 'file':
                     self.upload_file_robustly(relpath, ie.file_id)
+                elif ie.kind == 'symlink':
+                    try:
+                        self.upload_symlink_robustly(relpath, ie.symlink_target)
+                    except errors.TransportNotPossible:
+                        if not self.quiet:
+                            target = self.tree.path_content_summary(relpath)[3]
+                            self.outf.write('Not uploading symlink %s -> %s\n'
+                                            % (relpath, target))
                 elif ie.kind == 'directory':
                     self.make_remote_dir_robustly(relpath)
-                elif ie.kind == 'symlink':
-                    if not self.quiet:
-                        target = self.tree.path_content_summary(relpath)[3]
-                        self.outf.write('Not uploading symlink %s -> %s\n'
-                                        % (relpath, target))
                 else:
                     raise NotImplementedError
             self.set_uploaded_revid(self.rev_id)
@@ -347,10 +374,7 @@
                 elif kind is  'directory':
                     self.delete_remote_dir_maybe(path)
                 elif kind == 'symlink':
-                    if not self.quiet:
-                        target = self.tree.path_content_summary(path)[3]
-                        self.outf.write('Not deleting remote symlink %s -> %s\n'
-                                        % (path, target))
+                    self.delete_remote_file(path)
                 else:
                     raise NotImplementedError
 
@@ -365,12 +389,7 @@
                     # We update the old_path content because renames and
                     # deletions are differed.
                     self.upload_file(old_path, new_path, id)
-                if kind == 'symlink':
-                    if not self.quiet:
-                        self.outf.write('Not renaming remote symlink %s to %s\n'
-                                        % (old_path, new_path))
-                else:
-                    self.rename_remote(old_path, new_path)
+                self.rename_remote(old_path, new_path)
             self.finish_renames()
             self.finish_deletions()
 
@@ -379,15 +398,18 @@
                     if not self.quiet:
                         self.outf.write('Ignoring %s\n' % path)
                     continue
-                if old_kind == 'file':
+                if old_kind in ('file', 'symlink'):
                     self.delete_remote_file(path)
-                elif old_kind ==  'directory':
+                elif old_kind == 'directory':
                     self.delete_remote_dir(path)
                 else:
                     raise NotImplementedError
 
                 if new_kind == 'file':
                     self.upload_file(path, path, id)
+                elif new_kind == 'symlink':
+                    target = self.tree.get_symlink_target(path)
+                    self.upload_symlink(path, target)
                 elif new_kind is 'directory':
                     self.make_remote_dir(path)
                 else:
@@ -403,10 +425,13 @@
                 elif kind == 'directory':
                     self.make_remote_dir(path)
                 elif kind == 'symlink':
-                    if not self.quiet:
-                        target = self.tree.path_content_summary(path)[3]
-                        self.outf.write('Not uploading symlink %s -> %s\n'
-                                        % (path, target))
+                    target = self.tree.get_symlink_target(path)
+                    try:
+                        self.upload_symlink(path, target)
+                    except errors.TransportNotPossible:
+                        if not self.quiet:
+                            self.outf.write('Not uploading symlink %s -> %s\n'
+                                            % (path, target))
                 else:
                     raise NotImplementedError
 
@@ -419,6 +444,9 @@
                     continue
                 if kind == 'file':
                     self.upload_file(path, path, id)
+                elif kind == 'symlink':
+                    target = self.tree.get_symlink_target(path)
+                    self.upload_symlink(path, target)
                 else:
                     raise NotImplementedError
 
@@ -542,14 +570,11 @@
             locked.unlock()
 
         # We uploaded successfully, remember it
-        branch.lock_write()
-        try:
+        with branch.lock_write():
             upload_location = conf.get('upload_location')
             if upload_location is None or remember:
                 conf.set('upload_location',
                          urlutils.unescape(to_transport.base))
             if auto is not None:
                 conf.set('upload_auto', auto)
-        finally:
-            branch.unlock()
 

=== modified file 'breezy/plugins/upload/tests/test_upload.py'
--- breezy/plugins/upload/tests/test_upload.py	2018-07-02 23:41:13 +0000
+++ breezy/plugins/upload/tests/test_upload.py	2018-09-17 00:27:30 +0000
@@ -47,6 +47,8 @@
     if features.paramiko.available():
         from ....transport import sftp
         usable_classes.add(sftp.SFTPTransport)
+    from ....transport import local
+    usable_classes.add(local.LocalTransport)
     for name, d in basis:
         t_class = d['transport_class']
         if t_class in usable_classes:
@@ -411,19 +413,19 @@
 
         self.do_upload()
 
-        self.assertUpPathDoesNotExist('link')
+        self.assertUpPathExists('link')
 
     def test_rename_symlink(self):
         self.make_branch_and_working_tree()
         old_name, new_name = 'old-link', 'new-link'
         self.add_symlink(old_name, 'target')
         self.do_full_upload()
+
         self.rename_any(old_name, new_name)
 
         self.do_upload()
 
-        self.assertUpPathDoesNotExist(old_name)
-        self.assertUpPathDoesNotExist(new_name)
+        self.assertUpPathExists(new_name)
 
     def get_upload_auto(self):
         # We need a fresh branch to check what has been saved on disk

=== modified file 'breezy/transport/sftp.py'
--- breezy/transport/sftp.py	2018-08-31 05:50:14 +0000
+++ breezy/transport/sftp.py	2018-09-17 00:27:30 +0000
@@ -849,7 +849,7 @@
         """See Transport.readlink."""
         path = self._remote_path(relpath)
         try:
-            return self._get_sftp().readlink(path)
+            return self._get_sftp().readlink(self._remote_path(path))
         except (IOError, paramiko.SSHException) as e:
             self._translate_io_exception(e, path, ': unable to readlink')
 
@@ -857,7 +857,7 @@
         """See Transport.symlink."""
         try:
             conn = self._get_sftp()
-            sftp_retval = conn.symlink(source, link_name)
+            sftp_retval = conn.symlink(source, self._remote_path(link_name))
         except (IOError, paramiko.SSHException) as e:
             self._translate_io_exception(e, link_name,
                                          ': unable to create symlink to %r' % (source))