The readdirSync and readdir functions both have a bug when one filesystem has an empty directory
The readdirSync and readdir functions both have a bug when there's 2 filesystems with this.fss[1] having the directory but it is empty and this.fss[0] not having the directory. In that case checking the size doesn't work to know if the directory exists. In particular, the fix at https://github.com/streamich/unionfs/pull/348 is incorrect. This PR fixes both bugs and adds examples to the test suite.
@magarcia do you have any thoughts on this?
I'm working on getting the cpython test suite to pass (as part of https://github.com/sagemathinc/python-wasm which uses unionfs), and this revealed another bug in unionfs, which I've also fixed here (and added tests).
This test in the python test suite:
import os; os.listdir('test_exceptions.py')
was supposed to raise
NotADirectoryError: [Errno 54] Not a directory: 'test_exceptions.py''
but was instead raising
FileNotFoundError: [Errno 44] No such file or directory: 'test_exceptions.py'
I traced this (through WebAssembly, WASI, etc.) to how exceptions are raised in unionfs. Basically, unionfs assume that if there is any exception doing anything then it is because of ENOENT. However, there are other reasons to get an error, e.g., ENOTDIR is thrown when you try to open a file as a directory (as is the case in the test above). unionfs is just treating that as a file not found situation and looking further. I think that's the wrong behavior -- instead if an error other than ENOENT is raised, then immediately stop searching through the union and throw that exception. At least that addresses the above problem. I implemented this for readdir, readdirSync, and also asyncMethod and syncMethod. I don't know if that's enough to fully do this right.
Anyway, I hope this is helpful. I case this never gets looked at and somebody else hits this, I'm maintaining a temporary fork here with these fixes: https://www.npmjs.com/package/@wapython/unionfs
-- William
I pushed another commit that limits the scope of this change to only ENOTDIR errors. Otherwise, the latest change would cause other problems.
And also EEXIST errors, as I found when running the cpython test_gzip.py part of their test suite...
Just want to let you know I've not forgotten about this, but it looks like a potentially significant change and we're blocked on doing releases due to #741 so I feel its a bit moot to dive into until that is resolved.
I'm going to try and reach out to @streamich on other channels to see if I can get us unblocked for all the *fs packages.
Thanks. I understand, and am fine using my slight fork in the meantime.
@williamstein we've able to do releases again, if you want to pick this back up :)
A simplified version of this fix, without ENOTDIR handling, was merged: https://github.com/streamich/unionfs/pull/787/files