Fix problem creating ZIP archive with lots of entries

[smithdh]

Intended to fix #2004. Initially in draft.

[abh3]

Let me know when this is ready for review.

[smithdh]

Thanks Andy. I'd been making some last changes; but it's ready, I'll move it out of draft now, so it could be reviewed. I expect depending on the feedback we can consider this, or not, for including in 5.5.5 (since that's targeted for tagging tomorrow).

[abh3]

I don't see anything wrong here. The question is does this use checkpointing? If so, perhaps one needs to enable checkpointing on the server and see if another problem crops up, The checkpointing allows additions to the zip archive in-place.

[abh3]

Oh yes. while I didn't specifically approve, I am OK if you feel that my comment is not applicable. Just ask Guilherme to merge it.

[smithdh]

(I just made a force-push, only changed a typo in code comment).

Thank you. I just checked that it does appear to work on a server with checkpointing enabled (in the no error case); infact appending against an existing archive fails against a server without checkpointing enabled. But, following on from your comment, we could verify more. e.g. that the checkpointing is giving the protection we want: keeping the archive valid if there's a problem during addition of files (I saw we don't use the kXR_ckpXeq->kXR_write sequenced version of write during CloseArchive and I'm not sure if that will properly preserve integrity if the checkpoint changes are discarded; and I didn't try to change that in this PR).

I'll suggest to Guilherme to merge, so we can address #2004 now.

[abh3]

Interesting. If you know what you are doing you don't need to subsequently use CkpXeq. The idea here is to protect the index of the zipfile. If we overwrite that then it gets checkpointed. After that it's all appends and checkpointing isn't explcitly necessary (assuming the whole index was overwritten). Upon failure all changes are discarded and the saved data is restored. I suppose we should independently verify that this actually is what is happening.

…

On Fri, 5 May 2023, David Smith wrote: (I just made a force-push, only changed a typo in code comment). Thank you. I just checked that it does appear to work on a server with checkpointing enabled (in the no error case); infact appending against an existing archive fails against a server without checkpointing enabled. But, following on from your comment, we could verify more. e.g. that the checkpointing is giving the protection we want: keeping the archive valid if there's a problem during addition of files (I saw we don't use the kXR_ckpXeq->kXR_write sequenced version of write during CloseArchive and I'm not sure if that will properly preserve integrity if the checkpoint changes are discarded; and I didn't try to change that in this PR). I'll suggest to Guilherme to merge, so we can address #2004 now. -- Reply to this email directly or view it on GitHub: #2006 (comment) You are receiving this because your review was requested. Message ID: ***@***.***>