search

kiwix / libkiwix

Common code base for all Kiwix ports
https://download.kiwix.org/release/libkiwix/
GNU General Public License v3.0
112 stars 55 forks source link

Unable to host books with FileDescriptor on `Server` #1015

Closed MohitMaliFtechiz closed 2 months ago

MohitMaliFtechiz commented 8 months ago

We introduced the hosting books feature for KiwixCustomApps a few days back, and now we are using FileDescriptor to read the zim files from the asset folder in https://github.com/kiwix/kiwix-android/pull/3516. While refactoring the server functionality to use the FileDescriptor to host the zim file on Server we have seen that the server is unable to host books with FileDescriptor.

I don't know if it is possible to host zim files with FileDescriptor, if possible how much time it will take to implement. I am creating this ticket for this bug to further conversation on this.

veloman-yunkan commented 4 months ago

Support in libzim for opening ZIM files via a file descriptor boils down to a hack that just converts the fd into a Unix-specific file path that is valid within the same process and as long as the original fd stays open. openzim/libzim#860 didn't change that.

Now I am not sure that API-wise it makes sense to pretend that we support opening ZIM files via file-descriptor. Instead we better support only file-paths and let the clients convert fds to paths on their own.

However, opening ZIMs via fds is different in that it also requires supplying offset and data size values. So currently (i.e. with openzim/libzim#860 recently merged) a ZIM-reference (the data required to open a ZIM archive) is one of the following:

  1. A collection of file paths (a single file being just a special case)
  2. A collection of (fd, offset, data-chunk-size) triplets

I think that we should unify those two cases into "a collection of (file-path, offset, data-chunk-size) triplets", introduce in libzim a class say (ArchiveRef) for that data and just extend the rest of openzim/kiwix code so that APIs that previously took an std::string representing the file path start accepting an ArchiveRef too.

@kelson42 @mgautierfr What do you think?

kelson42 commented 2 months ago

@veloman-yunkan @mgautierfr To me this ticket is basically a duplicate of https://github.com/openzim/libzim/issues/852 and I would like to close ot at such.

mgautierfr commented 2 months ago

I think this issues around fd are a bit different, as they may have different root cause:

However, (if this issue is not already fixed) we should tackle this thinking globally about an api to allow us to read data from source for which we don't have a path or we cannot directly reopen it.

So no need to have several issues about that, I will close this issue in favor of openzim/libzim#852.