-
-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correctly try to open split files. #880
Conversation
452bb07
to
917ad90
Compare
917ad90
to
9bf60d8
Compare
It appears that the first commit is a bit too aggressive (without speaking potential ABI break). See https://github.com/openzim/libzim/actions/runs/8882043713/job/24385796413?pr=880 I have propose another PR #881 simply testing creation of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please edit/clean-up the history of this PR (and rewrite the commit messages as needed).
2bdffb0
to
e028dc5
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few final touches (that can be made in place/without fixup commits) and we can merge.
e028dc5
to
6348ae4
Compare
One is for simple file part only, the other is for multi part only. We also introduce a static helper function `openSinglePieceOrSplitFile` to build the FileCompound as before. This helper function is put in FileImpl which can seem a bit odd but will simplify next commits.
6348ae4
to
91f78bb
Compare
New version (simpler) following your live advice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a small reservation that can be addressed before merging - the description of the second commit doesn't accurately describe the change; it would be nice if the change in user observable behaviour (opening of .zimaa files) is mentioned.
…opening. Now, it tries to open as single piece only if not ending by `.zimaa`. If it cannot find the file, or if ending with `.zimaa`, it tries multi zim part. In any case, FileImpl constructor check for size and throw a ZimFileFormatError is file's size doesn't correspond to what is the header.
91f78bb
to
48e64c7
Compare
Fix #879
First commit is not really related to this PR but recent gcc complain about the missing default destructor in
test/archive.cpp
. So I fix this in the same time. If you prefer, I can move it a different PR as it is not straightforward fix.