[PATCH] better handling of symlinks in path

[David Allouche]

On Mon, 2005-10-10 at 18:37 +1000, Robert Collins wrote:
> On Mon, 2005-10-10 at 09:49 +0200, David Allouche wrote:
> > On Mon, 2005-10-10 at 09:57 +1000, Robert Collins wrote:
> > > On Sun, 2005-10-09 at 22:43 +0200, David Allouche wrote:
> > > > +            # check that relpath knows about symlinks
> > > > +            # TODO: SKIP these tests on win32
> > > 
> > > 
> > > To do this, guard the portion of the test, or the whole test case with
> > > 
> > > if osutils.has_symlinks():
> > 
> > Gah... I had fixed it, but did not save the file before making the
> > patch...
> > 
> > Fixed that, and made the _relpath implementation a little bit simpler by
> > calling os.path.realpath directly if it's available instead of using a
> > (pointless IMO) osutils helper.
> 
> Ok, two things here.
> 
> I think that inlining the function reduces clarity: before we had two
> functions that roughly do one thing each, now we have one function that
> does a couple of things and works around stdlib limitations.

Here is how my thinking went:

      * Mh, if os.path.realpath is not available, we'll be calling
        abspath twice. It's not a big deal performance wise, but it's
        only needed because we want to have a "portable" realpath.
      * This helper function is not what we really want, there is no
        place in the code where we want "use abspath if realpath is not
        there"
      * This helper function reduces clarity (adding a delegation) for
        no reason, as it's used nowhere else.

I'd be happy to restore bzrlib.osutil.realpath. It just seems
unecessary.

> More importantly, I think this is flawed in that the condition to be
> able to save the last element can be applied recursively:
> You say that you are preserving the last element in case it is a
> symlink, but what if there is are two symlinks? I.e. if preserving the
> tail element saves you from one, what happens with two? 

I should probably add a comment about that.

We want to traverse symlinks, but we want to be able to refer to
version-controlled symlinks.

When the user refers to a version controlled symlink, the symlink is the
last element in the path, so we need to special-case that.

When any element in the path (but the last) is a symlink, we want to
traverse it to either: 1. get the real relative path 2. fail if the
symlink goes outside of the tree.

I do not see why you would think this logic is flawed, the fact the
element is special case is entirely intentional and (I think) covered in
the new tests.

When the last element in the path is a "." or a "..", and the previous
element is a symlink, I really do not know what we should be doing. At
the moment, trailing "." and ".." are stripped by abspath, as they were
before. That means that "/absolute/path/to/symlink/." is resolved to
"path/to/symlink", though I can imagine that would not be the expected
behaviour.

-- 
                                                            -- ddaa
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051010/703c583f/attachment.pgp