fix: deduplicate ZipStore central directory on close

[nuglifeleoji]

ZIP files are append-only: writing the same key a second time adds a new entry without removing the old one. When an array is resized, zarr.json is rewritten, which accumulates duplicate filenames in the central directory and produces:

UserWarning: Duplicate name: 'zarr.json'

Many zip readers (e.g. KDE Ark, Windows Explorer) show only the first entry for a duplicated name, making the stored data appear stale or corrupt. The duplicates also waste space.

Fix: ZipStore.close() now calls _dedup_central_directory() before closing the file. It scans filelist in reverse and keeps only the last (most recent) entry for each filename, so the on-disk central directory is clean. This follows the approach suggested in the issue by @mkitti.

store = ZipStore("data.zip", mode="w")
arr = zarr.create_array(store, shape=(5,), dtype="i4")
arr.resize((10,))   # rewrites zarr.json
store.close()

# Before: zarr.json appeared twice in the central directory
# After:  only one entry for zarr.json

Closes #3580

Made with Cursor

[d-v-b]

what happens if that scan is interrupted?

[nuglifeleoji]

Good point — the original code updated filelist and NameToInfo in two separate steps, so an interrupt between them would leave the object inconsistent. Fixed in the latest commit: both new structures are built upfront, then swapped in together, so if anything raises before the swap the original state is still intact.

[nuglifeleoji]

The CI failures are caused by tests that expected the 'Duplicate name' UserWarning to still be emitted. Since this PR deduplicates entries on close, the warning is no longer raised and those tests fail with 'DID NOT WARN'.

Two options:

1. Keep the dedup silent (current approach) and update all test assertions to not expect the warning
2. Emit the warning from close() when dedup is needed, so existing tests pass

Which approach would you prefer? Happy to update accordingly.

[mkitti]

NameToInfo already has a unique set of filenames in its keys and the latest zipinfo entries as its values, I believe. Please double check this.

If so, we could just NameToInfo to update the filelist. By using zipfile.getinfo to retrieve information from NameToInfo we could implement everything using public API.

[mkitti]

Suggested change

	for info in reversed(self._zf.filelist):
	for info in reversed(self._zf.infolist()):

Rather than accessing filelist directly here, use the public API infolist().

[mkitti]

Looking at NameToInfo I have a few questions and observations:

1. Rather than constructing seen, a set, and deduped, a list, would it not be easier to just construct a dict?
2. That dict seems equivalent to NameToInfo.
3. The original NameToInfo seems to have what we want already. It contains a unique set of filenames as its keys and the latest version of each file as its values (double check this).
4. Perhaps all that we need to do is set the contents of filelist to the values of NameToInfo?

self._zf.filelist = list(self._zf.NameToInfo.values())

There might even be a way to do this completely with the public API.

# zipfile.infolist() is public API
_filelist = self._zf.infolist()
# Make sure that the public API returns the SAME Python object as zipfile.filelist, which is NOT public API
assert _filelist is self._zf.filelist, "The assumption that zipfile.infolist() returns zipfile.filelist is not valid"
# zipfile.namelist() is public API
_namelist = np.unique(self._zf.namelist())
# Manipulate the list in place
_filelist.clear()
# zipfile.getinfo is just NameToInfo.get
_filelist.extend({self._zf.getinfo(name) for name in _namelist})

[nuglifeleoji]

Thanks for the review, @mkitti!

I checked the CPython zipfile source and the NameToInfo is updated via dict assignment (NameToInfo[name] = zinfo), so later writes always overwrite earlier ones. This means NameToInfo already holds the latest ZipInfo for each filename. I also confirmed that infolist() returns self.filelist directly (not a copy), and getinfo(name) is just NameToInfo[name].

What I changed:
Since NameToInfo is already correct, only filelist needs to be rebuilt. I replaced the seen/deduped loop with:
unique_names = dict.fromkeys(self._zf.namelist()) # public API, preserves order, deduplicates
self._zf.filelist = [self._zf.getinfo(name) for name in unique_names] # public API
NameToInfo no longer needs to be updated at all. The new list is computed before assignment, so atomicity is preserved.

One note on your public-API snippet:
The set comprehension {self._zf.getinfo(name) for name in _namelist} loses insertion order, and np.unique() sorts names alphabetically — both could reorder the entries in filelist. I used dict.fromkeys() instead to deduplicate while preserving first-seen order, with no numpy dependency needed.

Thanks so much on your review, and I am really grateful and eager to solve other problems