Skip to content

New function to efficiently remove the last entries from an archive #333

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

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

depth42
Copy link

@depth42 depth42 commented Aug 29, 2024

Changes proposed in this PR

In this pull request, I added a new function for incrementally removing the last entries from an archive. This is very handy, when you want to remove and re-add entries add the end of an archive as you don't have to rewrite the archive in full.
The function is: Archive.removeAllEntries(fromEntry entry: Entry)

Tests performed

I implemented trhee tests for the function following the existing test pattern.

  • testRemoveFromEntryUncompressed
  • testRemoveFromEntryCompressed
  • testRemoveEntriesFromArchiveWithZIP64EOCD

@weichsel
Copy link
Owner

weichsel commented Jan 6, 2025

Thanks for providing this PR. I like the idea of using truncate as fast-path when possible but I wonder if it wouldn't be better to keep that an implementation detail.
We could check wether the entry to remove is the last one and use the code in the PR as alternate but private code path. (and provide a generic remove multiple entries at once API as follow-up)

Frank Illenberger added 2 commits May 23, 2025 15:49
With support for efficient truncation when removed entries are contiguous at the end of the archive.
@depth42
Copy link
Author

depth42 commented May 23, 2025

It took a while, but I've implemented your suggestion in the pull request: I've added a function for removing multiple entries from an archive (this fixes #109), with a fast-path for truncation if the entries to remove are contiguous at the end of the archive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants