Better caching and other fixes

openscopeproject / ZipROFS

FUSE file system with transparent access to zip files as if they were folders.

MIT License

11 stars 5 forks source link

Better caching and other fixes #4

Closed JuniorJPDJ closed 3 years ago

JuniorJPDJ commented 3 years ago

is_zipfile caching
holding ZipExtFile per file handle, not per path (keeps file position even for the same file opened more than once) - IMHO it's better and more simple/clean solution than whole caching class
PyCharm .gitignore
requirements.txt
typing fixes
style fixes (PyCharm was literally crying about them)
configurable cache size
fixed fusepy package name (fuse is original upstream package name and fusepy is renamed version in ubuntu)
block reads in non-seekable file (not gonna happen anyway)
fixed race condition in access to the same ZipFile / ZipExtFile, which resulted in weird seeking, causing double ZipFile reads

Fixed #2, fixes #3, resolves last issues from #1

wvffle commented 3 years ago

JuniorJPDJ commented 3 years ago

$ ./runtest.sh 
Running ziprofs tests...
Mounting filesystem
--------------------------
Running test: zip is directory
PASS
--------------------------
Running test: tree
PASS
--------------------------
Running test: reading file content #1
PASS
--------------------------
Running test: reading file content #2
PASS
--------------------------
Running test: running script
PASS
Killing ziprofs
5/5 tests passed.

Funny, tests passed on my machine..

JuniorJPDJ commented 3 years ago

Ubuntu renamed fusepy's package from fuse to fusepy, arch have it like in upstream.

JuniorJPDJ commented 3 years ago

And don't use ZipFile's internal fields, including locks. It's never a good idea to rely on implementation details of a library that can change any time and break your code.

There's no reference in ZipExtFile back to ZipFile, and I don't feel like maintaining own reference list is a good idea, but that would be necessary to write this locks at my own. You see your point, but I don't feel like second way is good neither.

ATM I did pretty awful workaround, let's see what you will say about it :D

qu1ck commented 3 years ago

I've been reading ZipFile code and I'm certain that the way we are using it ZipExtFile will allways be seekable. It's only not seekable if you initialize ZipFile with a stream file-like object instead of a path like we do. So we can get rid of the manual position tracking and as a result custom ZipExtFile as well.

JuniorJPDJ commented 3 years ago

You are right, I was trying to tell it to you. FS will not work with non-seekable zip files, and the only case when ZipExtFile is non-seekable is non-seekable zip file.

This is no good, you can get in a situation where ziplock() will try to return lock of a ZipFile which got evicted from cache and destroyed.

Now reference to ZipFile will be hold in ZipExtFile so we are good I bet.

I'm removing custom ZipExtFile and falling to pair of dicts - for ZipFile and ZipExtFile like you suggested. Also gonna raise error when trying reads on non-seekable file.

JuniorJPDJ commented 3 years ago

Is it ok now? :D

qu1ck commented 3 years ago

Thanks for contributing, I added better example and merged it. Glad someone else found my small weekend project useful.

JuniorJPDJ commented 3 years ago

It's the best zip FS I used until today and I was looking at whole github and google ;) Just linked it in some places.