nathanhi / pyfatfs

Python based FAT12/FAT16/FAT32 implementation with VFAT support
https://pypi.org/project/pyfatfs/
MIT License
29 stars 14 forks source link

As well as files or devices on the local filesystem, allow opening of file-like objects. #16

Closed dseeley closed 2 years ago

dseeley commented 2 years ago

To avoid having to create temporary files on local media, allow manipulating file-like objects in-memory.

dseeley commented 2 years ago

In my opinion though, having a function serving multiple different purposes (opening a file or setting a file pointer) will lead to confusion ("why can I pass a file-like object as filename?) and maintenance will become more difficult in the future (harder to unit test).

Is there any specific reason to have this in the open function instead of just offering a public set_fp function that I'm overlooking? If not, would you mind to expose this functionality in a new function instead?

Hi - perhaps this is merely ignorance of the interface on my part!

I'm trying to open a fat filesystem that is stored as a BytesIO object:

    cidata_fatfs = PyFatFS(cidata__fat_bytestream)
    cidata_fatfs.writetext('/meta-data', META_DATA)

It seems the constructor requires a filename; is there another way to achieve what I'm attempting?

dseeley commented 2 years ago

It seems the constructor requires a filename; is there another way to achieve what I'm attempting?

Hi @nathanhi - have you had any more thoughts on this?

I was trying to avoid modifying the signature of PyFatFS constructor - so it needs to take a filename. This filename then has its __fp property set (after being opened, (or not, in the case of my PR)) in PyFat.open. Is there a way to avoid any of this?

nathanhi commented 2 years ago

Sorry for the delay! I went ahead and implemented my previous proposal:

https://github.com/nathanhi/pyfatfs/compare/feature/set_fp?expand=1

It can be used like this:

in_memory_fs = BytesIO(b'your_raw_filesystem_data')
pf = PyFat()
pf.set_fp(in_memory_fs)

Would this work for you? Implementing it like this would need the least compromise with regards to the API.

dseeley commented 2 years ago

Would this work for you? Implementing it like this would need the least compromise with regards to the API.

Hi - thanks for getting back! I see this enables opening a BytesIO PyFat object, but does this enable me to open a PyFatFS with a BytesIO PyFat? Is there a way to otherwise write to the PyFat object?

nathanhi commented 2 years ago

Would this work for you? Implementing it like this would need the least compromise with regards to the API.

Hi - thanks for getting back! I see this enables opening a BytesIO PyFat object, but does this enable me to open a PyFatFS with a BytesIO PyFat? Is there a way to otherwise write to the PyFat object?

Yes, that's why I hid my answer - I overlooked that fact. I just released 1.0.0, which not only adds support for formatting a new filesystem, but also said set_fp function as well as an pyfatfs.PyFatFS.PyFatBytesIOFS class, which should provide exactly what you need:

https://pyfatfs.readthedocs.io/en/stable/pyfatfs.PyFatFS.html#pyfatfs.PyFatFS.PyFatBytesIOFS

You can also probably use the make_fs function from the integration tests as an example:

https://github.com/nathanhi/pyfatfs/blob/master/tests/test_PyFatFS.py#L21

Looking forward to your feedback!

dseeley commented 2 years ago

Looking forward to your feedback!

Brilliant - works perfectly! Thanks!