Skip to content

Fix double extraction when extract ice is used with from'gh-r'#761

Merged
alberti42 merged 3 commits intozdharma-continuum:mainfrom
alberti42:fix-extract
Mar 6, 2026
Merged

Fix double extraction when extract ice is used with from'gh-r'#761
alberti42 merged 3 commits intozdharma-continuum:mainfrom
alberti42:fix-extract

Conversation

@alberti42
Copy link
Copy Markdown
Contributor

Problem

When using extract'!' (or extract'!!') together with from'gh-r', the downloaded archive was extracted twice:

  1. First by the inline ziextract call in the gh-r download code
  2. Then again by the ∞zinit-extract-hook, which runs at !atclone-pre

On the second pass, the original archive no longer exists (it was deleted after the first extraction). The --auto detection then falls back to the file command, which identifies unrelated files (e.g., .jar files, which are technically zip archives) and attempts to extract them — producing errors and potentially corrupting the plugin directory.

Example error output:

[ziextract] detected a zip archive in the file lib/some-library-1.2.3.jar.
[ziextract] ERROR: the file `lib/some-library-1.2.3.jar' doesn't exist.

The same issue also affected the tarball download path.

Root cause

The bug went unnoticed because extract'!' was typically used with cloned repositories (where only the hook runs), not with from'gh-r' releases (where both the inline call and the hook run).

Fix

Guard the inline ziextract calls in both the gh-r and tarball download paths so they only run when the extract ice is not set. When the ice is set, the ∞zinit-extract-hook handles extraction exclusively, avoiding the duplication.

Documentation

Updated the extract ice description in README.md to document the previously undocumented prefix modifiers:

Syntax Behavior
extract Auto-detect and extract archives
extract'!' Extract and flatten one directory level
extract'!!' Extract and flatten two directory levels
extract'-' Extract but keep the archive file
extract'!file.tar.gz' Extract a specific file (useful for cloned repos containing archives)

@alberti42
Copy link
Copy Markdown
Contributor Author

@vladdoster Let me know if you need a minimal repro to test this bug and how it is fixed.

@alberti42
Copy link
Copy Markdown
Contributor Author

@vladdoster You had started to look into it. Do you need anything from me?
@pschmitt Any time from your side to get a look and give a first feedback / indication?

Perhaps you could start reproducing the bug?

I find this patch super useful. Because the feature was not documented and broken, I did not know it existed for a long time. So, when I was installing apps/plugins from the GitHub releases, I always had to manually move the files in atclone to flatten the structure. But if you use ! this is not needed at all; it makes zinit ... commands much leaner rather than having to deal with boilerplate code...

@pschmitt
Copy link
Copy Markdown
Member

pschmitt commented Mar 5, 2026

Would be nice to have zunit tests for these, but other than that lgtm.

@alberti42
Copy link
Copy Markdown
Contributor Author

Would be nice to have zunit tests for these, but other than that lgtm.

Yes, indeed. Good idea. I will create some tests.

@alberti42 alberti42 force-pushed the fix-extract branch 2 times, most recently from d12d49d to 687df15 Compare March 5, 2026 23:34
@github-actions github-actions Bot added the tests label Mar 5, 2026
@github-actions github-actions Bot added the ci/cd label Mar 5, 2026
@alberti42 alberti42 force-pushed the fix-extract branch 2 times, most recently from cd662a1 to 4611bad Compare March 5, 2026 23:53
@github-actions github-actions Bot removed the ci/cd label Mar 5, 2026
… in extract'!'

When using extract'!' with from'gh-r', ziextract was called twice:
1. First by the gh-r download code (line 407) — extracting the archive directly
2. Then by the ∞zinit-extract-hook — calling ziextract --auto --move

On the second call, the .tar.gz was already gone, so --auto mode fell through to file-command detection, which identified .jar files as zip archives
and tried to extract them — corrupting the plugin and producing errors.

Fix: Both the gh-r download path (line 407) and the tarball path (line 272) now skip their direct ziextract call when the extract ice is set,
deferring entirely to the ∞zinit-extract-hook which handles it correctly.
@alberti42
Copy link
Copy Markdown
Contributor Author

Would be nice to have zunit tests for these, but other than that lgtm.

@pschmitt I added the new tests and further improved documentation for the ice extract

If I understood it correctly, there is a bug upstream in markdownlint-cli. I made a commit, d345854, to the Zinit main branch. This is a temporary fix to Zinit CI/CD, and it should be reverted once a fix is found upstream.

I filed an igorshubovych/markdownlint-cli#606.

It passes all tests. I dare to merge this PR. Hopefully no regression of any kind. 🤞

@alberti42 alberti42 merged commit 807590f into zdharma-continuum:main Mar 6, 2026
19 checks passed
@alberti42 alberti42 deleted the fix-extract branch March 6, 2026 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants