feat: unpack type="archive" preserving the archive directory structure - #114
Merged
Conversation
fix: stop skipping every other entry after send() during archive extraction test: cover structure-preserving extraction and the entry-path helper `type="archive"` now extracts the whole asset keeping its internal layout instead of copying the file as-is or flattening matched files into one directory. This is required for multi-file tools whose files reference each other by relative path (e.g. a binary resolving a shared library via an `$ORIGIN/../lib` rpath), which the old flat extraction broke. A single wrapping top-level directory is stripped (like `tar --strip-components=1`); a configured `<binary>` is only used to locate the executable for version checks, not moved; `<file>` rules act as an include filter. The extractor generator now keys each entry by its archive-relative path (forward slashes) so consumers can rebuild the tree; the pure path math (top-level strip, zip-slip guard) lives in `ArchiveEntryPath`. Along the way, `send()` already advances the generator, so the extra `next()` after it was skipping every second entry. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR changes type="archive" handling to extract archives into the destination while preserving the archive’s internal directory structure (with optional stripping of a single wrapping top-level directory), enabling multi-file tools that rely on relative paths between extracted files.
Changes:
- Added structure-preserving archive extraction flow in
DLoad, including top-level directory stripping and zip-slip protection viaArchiveEntryPath. - Updated archive implementations (
PharAwareArchive,NullArchive,GzArchive) and theArchivecontract so generator keys are archive-relative paths (forward slashes). - Added/updated unit, integration, and acceptance tests to validate relative-path keys, stripping behavior, and structure-preserving extraction.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Module/Archive/Internal/NullArchiveTest.php | Updates expectations for archive-relative key behavior in NullArchive. |
| tests/Unit/Module/Archive/Internal/GzArchiveTest.php | Updates key expectations to use decompressed archive-relative name. |
| tests/Unit/Module/Archive/API/ArchiveEntryPathTest.php | Adds unit tests for top-level strip detection and traversal rejection. |
| tests/Integration/Module/Archive/ArchiveIntegrationTest.php | Adds real-archive integration tests for relative-path keys and preserved layout extraction. |
| tests/Acceptance/DLoadTest.php | Adds acceptance coverage for archive extraction with top-level dir stripping. |
| src/Module/Config/Schema/Action/Type.php | Updates Type::Archive docs to reflect structure-preserving extraction semantics. |
| src/Module/Archive/Internal/PharAwareArchive.php | Changes yielded keys to archive-relative paths (forward slashes). |
| src/Module/Archive/Internal/NullArchive.php | Changes yielded key to filename (archive-relative path for single file). |
| src/Module/Archive/Internal/GzArchive.php | Changes yielded key to decompressed filename (archive-relative path for gzip). |
| src/Module/Archive/Internal/Archive.php | Updates internal example to use archive-relative keys. |
| src/Module/Archive/ArchiveEntryPath.php | Introduces helper for top-level strip detection and zip-slip guarding. |
| src/Module/Archive/Archive.php | Updates public Archive interface docs to define archive-relative key semantics. |
| src/DLoad.php | Implements new Type::Archive extraction path preserving structure and stripping wrapping dir. |
| dload.xsd | Updates schema documentation for archive processing semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Update the README (en/ru/es/zh) type table and the dload-fetch-tool skill to describe the new archive behaviour: the whole asset is unpacked preserving its directory layout, a single wrapping top-level directory is stripped, `<file>` acts as an include filter and `<binary>` only locates the executable. The old wording ("forces unpacking even for .phar files") no longer captures what the type does.
Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The broad `/^rapira-.*/` matched every published asset; anchor on `-php8.5-` so OS/arch detection only ever sees the PHP-embedded release variants. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refresh locked dependencies: internal/path 1.2.0 -> 1.3.0, testo/psalm and dev tooling bumps. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… Linux test: assert extracted archive entries by their in-archive names Copying a compressed tar entry through the `phar://` stream produced an empty file on Linux (green on Windows), so structure-preserving extraction wrote 0-byte files; `PharFileInfo::getContent()` decompresses reliably across platforms. The acceptance test also asserted the host-OS binary name (`rr` + extension) against the Windows-only mock asset, which never matches `rr.exe` on Linux — archive mode never renames, so assert the in-archive names instead. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`PharData::addFromString()` on a tar produced entries that read back empty once the archive was gzip-compressed and reopened on Linux (green on Windows), failing the structure-preserving extraction test. Build the tar from real on-disk files via `buildFromDirectory()` and drop the intermediate uncompressed tar so only the `.tar.gz` remains. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A tar.gz created via PharData and reopened within the same process reads its entries back as empty on Linux (green on Windows), so building the fixture on the fly made the structure-preserving extraction test fail in CI. Commit pre-built nested.zip and nested.tar.gz fixtures and read them instead — mirroring how real downloaded archives behave. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…pression Detecting the wrapping top-level directory for `type="archive"` iterated the whole extract() generator once just to read entry names, which for a `.gz` asset decompressed it an extra time (and left a temp file behind). Add Archive::entries() — a cheap paths-only listing: gzip/single-file variants return the name without touching the stream, phar-based archives read only the manifest — and use it for strip detection instead of a throwaway extraction pass. Addresses the Copilot review note. Everything here is internal to dload, so adding the interface method is not a BC break. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hermetic archive acceptance test used the Windows-only mock zip, so on the Linux CI runner the binary-detection and `<file>` include-filter branches of the archive extraction were never executed, leaving the patch below the coverage target. Add a configurable mock asset (alongside the existing `useMock` test seam) and an acceptance test driven by the nested fixture whose binary has no extension, exercising strip, binary detect/chmod/locate, a matched `<file>` rule and a skipped unmatched entry across platforms. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 What was changed
type="archive"now unpacks the whole asset into the destination preserving the archive''s internal directory structure, instead of copying the archive as-is (no rules) or flattening matched files into one folder.tar --strip-components=1(e.g.pkg-1.0/bin/app→bin/app).<binary>in archive mode is used only to locate the executable inside the extracted tree (for version checks) and gets the executable bit — it is not moved.<file>rules, when present, act as an include filter.Archive::extract()generator now keys each entry by its archive-relative path (forward slashes) so consumers can rebuild the tree. Path math (top-level strip + zip-slip guard) lives in the newArchiveEntryPath.Why?
Multi-file tools ship binaries that resolve siblings by relative path (e.g. a binary loading a shared library via an
$ORIGIN/../librpath). The old extraction either left the archive unpacked or flattened files into a single directory, which breaks those relative links. Structure-preserving extraction keepsbin/andlib/in place relative to each other.Checklist
ArchiveEntryPath(strip / zip-slip), updated Gz/Null key expectationstar.gzandzip: relative-path keys and structure restoreReview notes
Generator::send()already advances the generator to the next entry; the archive loop must not callnext()afterwards or it silently skips every second file (fixed here — the original flat loop was correct because it never callednext()aftersend()).type="archive"the version re-check inaddTask()is skipped, andgetLocalBinary()looks at the destination root while the binary now lives in a subdirectory (bin/) — so re-runs always re-download. Making version-skip work for archive tools needs a dedicated binary-path resolve.