PyFilesystem / pyfilesystem

Python filesystem abstraction layer
http://pyfilesystem.org/
BSD 3-Clause "New" or "Revised" License
287 stars 63 forks source link

Update Dokan wrapper to 1.0.0 #258

Closed Liryna closed 7 years ago

Liryna commented 8 years ago

@arekbulski I updated regarding your comments. I have found only 2 print, is there still remaining ?

Liryna commented 8 years ago

Thank you for your feedback @lurch I fixed them

lurch commented 8 years ago

I'll try to find time to test this on my Windows7 laptop one evening this week...

Liryna commented 8 years ago

:+1: You will simply need to download and install last Dokan RC https://github.com/dokan-dev/dokany/releases/download/v1.0.0-RC4/DokanSetup_redist.exe If it does not found dokan1.dll, you can find it in system32 or syswow64 depending of python version used.

arekbulski commented 8 years ago

@Liryna I would like you to look at #241 and #242 and review these two. Either incorporate those into yours or advise to reject them. I will review myself a bit later but this is outside my area.

Liryna commented 8 years ago

Thats where a part of my changes comes :)

Liryna commented 8 years ago

@lurch fixed :+1:

Liryna commented 8 years ago

Done, I hope this will be the last change.

arekbulski commented 8 years ago

There is a saying among game developers: "Nothing is e-v-e-r done." ^^

Liryna commented 7 years ago

@happy-monk Thank you for testing it ! Just to resume the situation, you have issue to create folder for now ? but the device mount without issue, you can read/write a file ?

I will take a look at the CreateFile.

lurch commented 7 years ago

Ditto, thanks for looking at this @happy-monk :+1: (makes me glad I didn't prematurely merge it! ;) )

happy-monk commented 7 years ago

@Liryna Yes and yes. @lurch :metal:

jeffswt commented 7 years ago

@Liryna There seems to be some major problems in your commit.

First problem: Your filesystem tends to work well on reading and writing, but not creating folders. Any attempt to create a folder instead creates a file with the same name at the target folder. This makes files' hierarchical manipulations impossible. This bug occurs on all filesystems. Screenshot as follows:

Bug_1

Second problem: The dokan wrapper literally removes files, albeit correctly in the filesystem, but it seems to raise exceptions on the way deleting the files. I do not quite understand the specific implementations, but the debugging messages are indeed irritating. This bug occurs on all filesystems. Debugging message as follows:

Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 234, in 'calling callback function'
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 291, in wrapper
    return func(self, *args)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 206, in wrapper
    res = func(*args, **kwds)
  File "C:\Programs\Python3\lib\site-packages\fs\errors.py", line 219, in wrapper
    return func(*args,**kwds)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 492, in Cleanup
    self._pending_delete.remove(path)
KeyError: '/New folder'

Third problem: Minor, but sometimes effects overall experience, that drives may not be unmounted after quite a long time. I haven't found out the reason, but the code and the debugging traceback is here.

>>> from fs.osfs import OSFS
>>> from fs.expose import dokan
>>> f = OSFS('D:\\Desktop')
>>> mp = dokan.mount(f, 'G:\\')
>>> mp.unmount()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1046, in unmount
    raise OSError("the filesystem could not be unmounted: %s" %(self.path,))
OSError: the filesystem could not be unmounted: G:\
>>> dokan.unmount('G:\\')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1002, in unmount
    raise OSError("filesystem could not be unmounted: %s" % (path,))
OSError: filesystem could not be unmounted: G:\
>>>
^C
C:\Users\Administrator>python
Python 3.5.1 (v3.5.1:37a07cee5969, Dec  6 2015, 01:38:48) [MSC v.1900 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from fs.expose import dokan
>>> dokan.unmount('G:\\')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1002, in unmount
    raise OSError("filesystem could not be unmounted: %s" % (path,))
OSError: filesystem could not be unmounted: G:\
>>> 

Fourth problem: ZipFS could not be exposed to dokan. I am not sure whether this is a problem, but this could be related to implementation methods which are not yet implemented. Debugging methods as follows:

Python 3.5.1 (v3.5.1:37a07cee5969, Dec  6 2015, 01:38:48) [MSC v.1900 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from fs.zipfs import ZipFS
>>> from fs.expose import dokan
>>> f = ZipFS('D:\\minetest-0.4.14-win32.zip')
>>> f.listdir()
['minetest-0.4.14']
>>> mp = dokan.mount(f, 'H:\\')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 981, in mount
    mp = MountProcess(fs, path, kwds)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1039, in __init__
    cmd = cmd % (repr(pickle.dumps((fs, path, dokan_opts, nowait), -1)),)
TypeError: cannot serialize '_io.BufferedReader' object
>>>

Would be grateful if you could solve these problems.

Liryna commented 7 years ago

@ht35268 I will fix the create folder issue but other reported problem that you point does not come from my commit and I don't have the knowledge to fix them.

Feel free to create a pull request with the fix 👍

lurch commented 7 years ago

Fourth problem: ZipFS could not be exposed to dokan.

I expect this might be a problem with ZipFS, rather than Dokan. (ISTR something similar being mentioned before a long time ago) IIRC the 'fix' is to use the foreground=True option in the dokan.mount call (as this then means the filesystem being exposed (ZipFS in this case) doesn't need to be pickled).

Liryna commented 7 years ago

Create directory and error self._pending_delete.remove(path) KeyError: '/New folder' are fixed 👍 and cannot reproduce the issue Third problem of @ht35268

jeffswt commented 7 years ago

@Liryna I can't reproduce the third problem either :P

Seems to me that it is a rare problem which has nothing to do with pyFilesystem or your dokan wrapper. Maybe it's just a bug in dokan but never mind.

@lurch It appears to be definite that the third problem has something to do with ZipFS and not dokan wrapper.

jeffswt commented 7 years ago

@Liryna You don't seem to use Python3 much. I created a pull request to your repo and it could fix Python3 compatibility problems.

jeffswt commented 7 years ago

Fifth problem: dokan wrapper is not synchronized with Python, changes in the dokan wrapper would not be shown in pyFilesystem and neither the other way.

bug

Liryna commented 7 years ago

Good to know that all issue you faced are now fixed.

You don't seem to use Python3 much. I created a pull request to your repo and it could fix Python3 compatibility problems.

@ht35268 my first python project here 👍 I tried with python 3 and got an error so I tought it was pyFilesystem that was not py3 compatible. Thank you for the PR it has been merged !

And for the Fifth problem, I have absolutely not idea how to do this, will need @lurch here.

jeffswt commented 7 years ago

Well, okay. I'll see if I can solve this issue myself. Currently working on it.

@Liryna Also there's a sixth problem :P

Chrome doesn't save files into _dokan-wrapped-pyfilesystem_s, neither does any other program, i.e. notepads, wordpads, mspaints e.t.c. I'm currently solving this problem by using... VHDs and wrapped them inside MemoryFSs. This is indeed awkward, I must say.

image

You see you can't create any file here, it might be a direct consequence of improper ZwCreateFile implementation?

image

jeffswt commented 7 years ago

And Problem six has nothing to do with dokan library.

lurch commented 7 years ago

Fifth problem: dokan wrapper is not synchronized with Python, changes in the dokan wrapper would not be shown in pyFilesystem and neither the other way.

Yeah, that's kinda related to the ZipFS problem, and is solved by specifying foreground=True. With foreground=False it actually pickles the exposed FS (e.g. MemoryFS), effectively making a copy of it, and the dokan expose process then unpickles and serves this copied FS.

jeffswt commented 7 years ago

Anyway, there are still problems with this wrapper, e.g. Problem 6. Wonder if we use threading for the dokan expose process to serve the FS. I've been annoyed by the pickling process when it eats up lots of memory. Trying to solve problem 5 and not 6.

Will be pushed to Liryna/pyfilesystem.

lurch commented 7 years ago

Not my area of expertise, but given that various layers of the OS filesystem may do different amounts of buffering, maybe it's not even possible for the Dokan and Python processes to share the same 'live view' of the same filesystem? shrug

Liryna commented 7 years ago

Chrome doesn't save files into dokan-wrapped-pyfilesystems, neither does any other program, i.e. notepads, wordpads, mspaints e.t.c. I'm currently solving this problem by using... VHDs and wrapped them inside MemoryFSs. This is indeed awkward, I must say.

@ht35268 this is "probably" related to getfilesecurity that are not supported on pyfilesystem.

@lurch I am not sure I understand correctly, Dokan request each time pyfs if a file exists and it does, he proceeds the windows request. So for me pyfs is the only one who know the existing of files.

lurch commented 7 years ago

@lurch I am not sure I understand correctly, Dokan request each time pyfs if a file exists and it does, he proceeds the windows request. So for me pyfs is the only one who know the existing of files.

Yeah, but if you call mount with foreground=False then it fires off a separate background process. No, I dunno why, you'd need to ask the original dokan-expose author ;)

lurch commented 7 years ago

"probably" related to getfilesecurity that are not supported on pyfilesystem

Could you "fake" it, and assume that it's always allowed?

jeffswt commented 7 years ago

@lurch That is what ntfs-3g does in Linux on NTFS systems - make everything rwxrwxrwx...

jeffswt commented 7 years ago

@lurch Well, I was just trying to fix problem six, with a feeble attempt to change it to threading. Maybe I just realized why the original author did not use threading... Can anyone fix this?

This was in Line 1051 or so... I edited this code and commented the original lines. (fs/expose/dokan/__init__.py)

def __init__(self, fs, path, dokan_opts={}, nowait=False, **kwds):
    if libdokan is None:
        raise OSError("the dokan library is not available")
    _check_path_string(path)
    self.path = path
    def mount_worker_thr(data):
        MountProcess._do_mount(data)
        return
    threading.Thread(target=mount_worker_thr, args=[(fs, path, dokan_opts, nowait)]).start()
    # cmd = "try: import cPickle;\nexcept ImportError: import pickle as cPickle;\n"
    # cmd = cmd + "data = cPickle.loads(%s); "
    # cmd = cmd + "from fs.expose.dokan import MountProcess; "
    # cmd = cmd + "MountProcess._do_mount(data)"
    # cmd = cmd % (repr(cPickle.dumps((fs, path, dokan_opts, nowait), -1)),)
    # cmd = [sys.executable, "-c", cmd]
    cmd = [sys.executable, "-c", '']
    super(MountProcess, self).__init__(cmd, **kwds)

And dokan explicitly fails and refuses to mount MemoryFS.

Python 3.5.1 (v3.5.1:37a07cee5969, Dec  6 2015, 01:38:48) [MSC v.1900 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from fs.memoryfs import MemoryFS
>>> from fs.expose import dokan
>>> f = MemoryFS()
>>> mp = dokan.mount(f, 'I:\\')
Exception in thread Thread-1:
Traceback (most recent call last):
  File "C:\Programs\Python3\lib\threading.py", line 914, in _bootstrap_inner
    self.run()
  File "C:\Programs\Python3\lib\threading.py", line 862, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1057, in mount_worker_thr
    MountProcess._do_mount(data)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1096, in _do_mount
    mount(fs, path, **opts)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 996, in mount
    raise OSError("Dokan failed with error: %d" % (res,))
OSError: Dokan failed with error: -5

Traceback (most recent call last):
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 968, in check_ready
    os.stat(path)
OSError: [WinError 995] The I/O operation has been aborted because of either a thread exit or an application request: 'I:\\'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1003, in mount
    check_ready(mp)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 970, in check_ready
    check_alive(mp)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 961, in check_alive
    raise OSError("dokan mount process exited prematurely")
OSError: dokan mount process exited prematurely
>>>
jeffswt commented 7 years ago

I tried with the following code and comment and uncomment the several lines... then I suspected that Dokan must be on the same thread as the pyFilesystem.

def mount_worker_thr(fs, path, dokan_opts, nowait):
    dokan_opts['foreground'] = True
    if nowait:
        dokan_opts["ready_callback"] = False
    mount(fs, path, foreground=True)
    import time
    time.sleep(15)
    return
# mount_worker_thr(fs, path, dokan_opts, nowait)
threading.Thread(target=mount_worker_thr, args=(fs, path, dokan_opts, nowait)).start()

When only using the mount_worker_thr, it works well until now, but when we use threading, it seems to collapse immediately, which throws the '-5' error.

Exception in thread Thread-1:
Traceback (most recent call last):
  File "C:\Programs\Python3\lib\threading.py", line 914, in _bootstrap_inner
    self.run()
  File "C:\Programs\Python3\lib\threading.py", line 862, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 1061, in mount_worker_thr
    mount(fs, path, foreground=True)
  File "C:\Programs\Python3\lib\site-packages\fs\expose\dokan\__init__.py", line 996, in mount
    raise OSError("Dokan failed with error: %d" % (res,))
OSError: Dokan failed with error: -5

So are there any limitations in native dokan library?

Liryna commented 7 years ago

I can try to "fake" it probably later.

You can start dokan from another thread without problem. It is just that DokanMain will block with this newly created thread.

jeffswt commented 7 years ago

@Liryna Why can it start without problem? I experienced problems hence.

And I faked it with STATUS_SUCCESSes, yet it did not work. Is there anything wrong with my trick?

@handle_fs_errors
def GetFileSecurity(self, path, securityinformation, securitydescriptor, securitydescriptorlength, neededlength, info):
    return STATUS_SUCCESS
    # return STATUS_NOT_IMPLEMENTED

@handle_fs_errors
def SetFileSecurity(self, path, securityinformation, securitydescriptor, securitydescriptorlength, info):
    return STATUS_SUCCESS
    # return STATUS_NOT_IMPLEMENTED
happy-monk commented 7 years ago

@ht35268 Dokan throws -5 error when you try to mount path that is already mounted or wasn't unmounted properly. This has nothing to do with threads.

jeffswt commented 7 years ago

@happy-monk Got it. But does this mean I'll have to reboot the computer every time? I always have to kill python by hand... Maybe it will solve this.

And literally, I tried these operations thoroughly and it only throws -5 error in threading. I tried it without threading immediately after and no -5 errors were thrown.

happy-monk commented 7 years ago

@ht35268 You don't have to reboot, there is some timeout before the mount point unmounts automatically.

Also you can unmount manually using dokanctl or just call dokan.unmount in your Python code before mounting.

It's strange, that you have problems with threading. I have did just the same and it works as expected. Windows programming is truly unpredictable thing.

Liryna commented 7 years ago

@ht35268 the trick is not that simple 😄 securitydescriptor need to be filled with the information request in securityinformation.

The same trick as in dokan fuse has to be done here. See https://github.com/dokan-dev/dokany/blob/master/dokan_fuse/src/dokanfuse.cpp#L457-L484 We fake the request for all directories request. This code has to be convert in python 👍

wgaylord commented 7 years ago

Any progress on this?

Liryna commented 7 years ago

win32security has to be used here for ConvertStringSecurityDescriptorToSecurityDescriptor and ConvertSecurityDescriptorToStringSecurityDescriptor Is that alright as dependency ?

lurch commented 7 years ago

Is that alright as dependency ?

Yeah, other FS modules have their own custom requirements http://docs.pyfilesystem.org/en/latest/getting_started.html#prerequisites so adding another one for Dokan should be fine. Is it pip-installable?

Rondom commented 7 years ago

On 2016-10-04 13:36, Andrew Scheller wrote:

Is that alright as dependency ?

Yeah, other FS modules have their own custom requirements http://docs.pyfilesystem.org/en/latest/getting_started.html#prerequisites so adding another one for Dokan should be fine. Is it pip-installable?

If pywin32 is not pip-installable, I think a ctypes-based implementation would make for a nicer user experience, though.

I do not have the time to help with that atm. I think that would be something for a later Pull Request. Or someone convinces pywin32 to provide wheels that are distributed on PyPi.

Liryna commented 7 years ago

I just check on my environement and seems like it is present by default and not linked to pywin32 .

Python 3.5.1 (v3.5.1:37a07cee5969, Dec  6 2015, 01:38:48) [MSC v.1900 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import win32security
>>> win32security.ConvertStringSecurityDescriptorToSecurityDescriptor
<built-in function ConvertStringSecurityDescriptorToSecurityDescriptor>
>>> import pywin32
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named 'pywin32'
lurch commented 7 years ago

I think maybe you (or some other package) must have installed that? https://docs.python.org/3/search.html?q=win32security shows no results.

Rondom commented 7 years ago

On 2016-10-04 14:05, Liryna wrote:

I just check on my environement and seems like it is present by default and not linked to pywin32 .

I am pretty sure it belongs to pywin32 (or some other Python-package that you installed) ;-) I cannot link to the PyWin32-documentation because they provide their documentation in a CHM-file 😯 which seems not accessible without installing the beast. They may have documentation on it.

See also the feature request on their bugtracker. If Twisted is really relying on it, I think it is a safe bet (in terms of maintenance etc.) for Pyfilesystem as well, even though the PyPi package is one release behind. See also the discussion on the Twisted bugtracker

happy-monk commented 7 years ago

@Liryna There is no package or module that you can import like this: import pywin32. PyWin32 is the name of this package in PYPI, but in terms of Python modules it's not even a package, it's a collection of modules with names like win32api, and win32security, and pywintypes.

Personally, I'd prefer not to require 7 MB package as dependency, when all really needed is two WinAPI functions. Besides, I'm not sure how to use security descriptor returned by pywin32 with ctypes or whether it even possible.

Rondom commented 7 years ago

Tahoe-LAFS depends (via Twisted) indirectly on pypiwin32 (the name of the package on Pypi that provides wheels of PyWin32). So pyfilesystem has already a soft dependency on pywin32

wgaylord commented 7 years ago

Any news? I really want to use this... It seems to sort of work.

Liryna commented 7 years ago

I need help on this 😢

For now it seems that I have correctly converted the sddl string as I wanted but I am unable to set the argument pointer PySECURITY_DESCRIPTOR with the result. https://github.com/PyFilesystem/pyfilesystem/pull/258/commits/6969c9be6fddc0c2e8502c0defdfaface3d1977e#diff-8a34a591f687d302fe173fc731c8f267R111

I don't get where PySECURITY_DESCRIPTOR is set http://docs.activestate.com/activepython/2.5/pywin32/PySECURITY_DESCRIPTOR.html and to make it work with POINTER()

any one with true python skills could help me :D ?

Rondom commented 7 years ago

You should be able to copy into another buffer like this:

securitydescriptor[:] = finalsd

I am not sure about whether this line would do the job, but this is how you copy one buffer into another. Also make sure you test this on 2.7 once you got it working under 3. I have not worked with the buffer interface much, so I may be wrong.

happy-monk commented 7 years ago

Don't use pywin32. It doesn't cooperate with ctypes and it wouldn't give you its buffer contents. Just leave it be and use ctypes to bind the functions you need.