Comment 5 for bug 214825

Revision history for this message
Martin Pool (mbp) wrote : Re: [Bug 214825] Re: Doesn't support uploading symlinks

On 17 February 2010 01:24, Neil Santos <email address hidden> wrote:
> Out of curiousity, what would be needed for bzr-upload to support
> symlinks?  I've created a hack that handles symlinks locally, but it's
> ugly (even for a hack), merely a way to shoehorn support in because I
> really need it.
>
> >From what I've learned in implementing my hack, it seems Bzr's
> Transports would need to implement it on a as-supported basis.  Or is
> this overkill, and would it be enough to localize the work inside bzr-
> upload?
>
> ** Patch added: "Quick and dirty hack to support for dealing with symlinks using LocalTransport on *nix systems"
>   http://launchpadlibrarian.net/39268272/bzr-upload_unix-symlink-support.patch

That patch looks basically reasonable. You should put up a branch and
propose a merge.

A few comments:

+ def upload_symlink_robustly(self, relpath, symlink_target):
+ if not isinstance(self.to_transport, transport.local.LocalTransport):
+ raise NotImplementedError

I don't see why you can't do it on non-local transports as long as
they support symlinks? Similarly there's no reason why it should
matter whether the local os supports them or not. Instead just try it
and check for TransportNotPossible.

+ 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
+
+ if osutils.has_symlinks():
+ # Target might not be there at this time; dummy file should be
+ # overwritten at some point, possibly by another upload.
+ target_path = osutils.normpath(osutils.pathjoin(
+ osutils.dirname(relpath),
+ symlink_target))

It seems like this should be done by url manipulation not os paths, or
it will break on windows.

+ self.make_remote_dir_robustly(osutils.dirname(target_path))
+ self._up_put_bytes(target_path, "", None)
+
+ os.symlink(symlink_target,
+ self.to_transport.local_abspath(relpath))
+ else:
+ raise NotImplementedError
+
     def make_remote_dir(self, relpath, mode=None):
         if mode is None:

A test for this would be nice, if bzr-upload has enough test framework
to support it.

Thanks!
--
Martin <http://launchpad.net/~mbp/>