[rfc] Proposal: fake symlinks support on windows

Alexander Belchenko bialix at ukr.net
Fri May 19 09:21:15 BST 2006


Martin Pool пишет:
> On 18 May 2006, Alexander Belchenko <bialix at ukr.net> wrote:
[skip]
>>So at this moment I'd like to hear comments from developers that 
>>interested in this symlink simulation. What my simple os-patching plugin 
>>is missed? What additional tests should be added, improved?
> 
>>In attachment you'll find patch for selftest subsystem of bzr. This 
>>changes is not depend on windows nor cygwin. I've add test_symlinks.py 
>>module with tests somewhat specific for my plugin. This tests looks 
>>trivial for unix where  I want to keep it separately and independetly in 
>>main bzrlib code with reason to split namespaces.
>>
>>I think this patch is worth to include in main bzr.dev.
> 
> I think this is fine - essentially it makes the tests be skipped if
> there are no symlinks, rather than just silently passing.  That's a
> useful improvement.

Thanks. Is it means that you give +1 on patch for selftest?

> We do have a specification which uses some finer distinctions than just
> TestSkipped; this is a step in that direction and when we do add more
> subclasses we can grep through and adjust them.

> 
>># Written by Alexander Belchenko for Bazzar-NG project
>># win32 fake symlinks support
>># based on cygwin symlinks simulation
>>
>>"""Fake symlinks support on win32"""
>>
>>import sys
>>
>>if sys.platform == "win32":
>>    import errno
>>    import os
>>    from stat import (S_ISREG, S_ISDIR, S_ISLNK, ST_MODE, ST_SIZE,
>>                      S_ISCHR, S_ISBLK, S_ISFIFO, S_ISSOCK)
>>    
>>    import bzrlib.osutils
>>    
>>    from win32_symlinks import *
>>
>>    # patching os module
>>    os.symlink = symlink
>>    os.readlink = readlink
> 
> 
> To bring this into bzr proper I'd rather not patch the os module but
> rather update all callers to either go through bzrlib.osutils or the
> workingtree.

Hmmm...
Some tests explicitly call os.symlink/os.readlink and os.path.islink 
functions so make all symlinks operations go through workingtree is 
caused to make this tests fails on win32 with fake symlinks. Probably in 
this situation is better to provide osutils.* functions but this 
proposal was previously rejected by Aaron. Or may be provide osutils.* 
functions and include symlinks methods inside WorkingTree class and link 
this methods to corresponding osutils functions. In the latter case 
direct usage of os.symlink/os.readlink and os.path.islink functions 
should be vetoed for overall bzrlib code.

> 
>>#!/usr/bin/python
>># This code written by Alexander Belchenko for the Bazaar-NG project
>>#
>># Cygwin's symlink file format:
>># '!<symlink>path/to/target\0'
>># file should have SYSTEM attribute in windows
>>
>>
>>__all__ = ['symlink', 'readlink']
>>
>>
>>import errno
>>import os
>>
>># from pywin32 package (http://pywin32.sf.net)
>>from win32file import (GetFileAttributes, SetFileAttributes,
>>                       FILE_ATTRIBUTE_SYSTEM, FILE_ATTRIBUTE_READONLY)
>>from win32com.shell import shell
>>from pythoncom import (CoCreateInstance, CLSCTX_INPROC_SERVER,
>>                       IID_IPersistFile)
>>import pywintypes
>>
>>
>>def symlink(src, dst):
>>    """Create a symbolic link (in cygwin format) pointing to src named dst.
>>    """
>>    if os.path.isfile(dst):
>>        raise OSError, errno.EEXIST     # 'File exists' error
>>
>>    f = file(dst, 'wb')
>>    f.write('!<symlink>%s\0' % src)
>>    f.close()
> 
> 
> The close should be a try/finally just as generally good practice.

Ok. I understand.

> 
>>    # change attributes of file to System
>>    SetFileAttributes(dst, FILE_ATTRIBUTE_SYSTEM)
>>
>>
>>def readlink(path):
>>    """Return a string representing the path to which the symbolic link points.
>>    The result may be either an absolute or relative pathname;
>>    if it is relative, it may be converted to an absolute pathname
>>    using os.path.join(os.path.dirname(path), result).
>>    """
>>    if os.path.isfile(path):            # cygwin symlink to file or directory
>>        # check if this file is cygwin symlink
>>        f = file(path, 'rb')
>>        data = f.read()
>>        f.close()
>>
>>        if GetFileAttributes(path) & FILE_ATTRIBUTE_SYSTEM \
>>            and data.startswith('!<symlink>') \
>>            and data.endswith('\0'):
>>                return data[10:-1]
>>        else:
>>            raise OSError, errno.EINVAL     # 'Invalid argument' error
>>
>>    elif os.path.isfile(path+'.lnk'):   # cygwin symlink to directory
>>        path = path + '.lnk'
>>
>>        # Cygwin create symlink to directory as windows shell shortcut
>>        # with READONLY file attribute.
>>        if GetFileAttributes(path) & FILE_ATTRIBUTE_READONLY:
>>            # this code to work with shortcut is based on pywin32 manual
>>            # see 'win32com.shell and Windows Shell Links'
>>            shortcut = CoCreateInstance(shell.CLSID_ShellLink,
>>                                        None,
>>                                        CLSCTX_INPROC_SERVER,
>>                                        shell.IID_IShellLink
>>                                       )
>>            try:
>>                shortcut.QueryInterface(IID_IPersistFile).Load(path)
>>                target = shortcut.GetDescription()
>>    
>>                # [bialix 20060517]
>>                # we probably could ensure that this shortcut
>>                # is really cygwin symlink to directory
>>                # in some complex way
>>                # but I'm not sure that it will works for all cygwin versions
>>                if 0:
>>                    name, find_data = shortcut.GetPath(shell.SLGP_RAWPATH)
>>                    if 0 != find_data[0] \
>>                        or [-109205.] * 3 != map(float, find_data[1:4]):
>>                            raise pywintypes.com_error
>>    
>>                return target
>>    
>>            except pywintypes.com_error:
>>                # if we get this error then path is not shortcut
>>                # or this file is not cygwin symlink
>>                pass
>>
>>        # if we are here then path is not [cygwin] symlink
>>        raise OSError, errno.EINVAL     # 'Invalid argument' error
> 
> 
> These branches are large enough that they should be split into separate
> functions, e.g. _read_cygwin_symlink, etc.

OK. I rework this part. This also helps avoid ImportError problems with 
currently created bzr.exe that does not contains inside all required 
libraries.

> 
>>    else:
>>        raise OSError, errno.ENOENT     # 'No such file or directory' error
> 
> 
> Can you really conclude that this is ENOENT, as opposed to say EACCESS?
> I suppose they'll come from os.path.isfile?

I try to reproduce Cygwin behaviour as close as possible, and Cygwin 
produce "[Errno2] No such file or directory" error. So I search actual 
name for this error number with following code:

import errno
for i in dir(errno):
     if getattr(errno, i) == 2:
         print i

Result is:

ENOENT

I have a feelings that error numbers on windows very often is 
inconsistent, or probably quantity and quality of windows errors does 
not corresponds to linux one.

> 
> 
>>if __name__ == "__main__":
>>    # manual testing
>>    if os.path.isfile('test_symlink'):
>>        os.remove('test_symlink')
>>
>>    symlink('../', 'test_symlink')
>>
>>    print readlink('test_symlink')
>>
>>    print readlink('dir_symlink')
> 
> 
> OK, so to test this better, we would want to make a subclass of
> TestCaseInTempDir, and split each of these into separate tests.  You
> would also want to test pointing to a nonexistent file (which should
> make no difference atm) and failures such as the symlink not existing,
> or the directory containing it not existing.  Some of these will be
> windows specific but more can be cross-platform, which will help use
> define what behaviour is common.

Those manual testing have the sense only on windows+cygwin platform. I 
already include slightly changed version of this test into my patch for 
selftest, into module test_symlinks.py: test named test_symlink_readlink.

> I'm cautiously positive on merging this if those things are fixed, but
> would like an opinion from someone who uses windows or cygwin more
> often.

There is at least Aaron and John who might say strong word about my work.
But anyone else who can and have some opinions will do comment this.

--
Alexander





More information about the bazaar mailing list