Fix #6515 "Expand-7zipArchive won't remove top folder if $ExtractDir depth exceeds 2" - Idea 1 #6516
Conversation
WalkthroughUpdated changelog and refactored decompression handlers in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Decompress as lib/decompress.ps1
participant External as 7z/tar/unzip/msiexec
participant FS as FileSystem
Caller->>Decompress: Expand-*(archive, DestinationPath, ExtractDir?)
Decompress->>Decompress: Trim/normalize DestinationPath\nDetect archive type (IsTar?)
alt ExtractDir provided
Decompress->>FS: Create tempDestination under $env:TEMP
alt Non-tar
Decompress->>External: Invoke extraction (FilePath + ArgumentList, include -ir!ExtractDir\*)
External-->>Decompress: Exit status
Decompress->>FS: Move tempDestination\ExtractDir -> Original DestinationPath
else Tar
Decompress->>External: List/extract tar (FilePath + ArgumentList)
External-->>Decompress: Exit status
Decompress->>FS: Arrange/move tar contents -> DestinationPath
end
Decompress->>FS: Remove tempDestination and cleanup logs (only if leaf)
else No ExtractDir
Decompress->>External: Invoke extraction -> DestinationPath
External-->>Decompress: Exit status
end
Decompress->>FS: Optional remove original archive parts (Get-ChildItem -Path / Remove-Item -Path)
Decompress-->>Caller: Return status/result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
o-l-a-v
changed the title
movedir
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)lib/decompress.ps1(4 hunks)
🔇 Additional comments (4)
lib/decompress.ps1 (4)
97-106: TEMP extraction for ExtractDir looks good; verify spaces in ExtractDir.7‑Zip’s -ir! filter can be sensitive to spaces. Our arg is one token, which should be fine. Please verify with an ExtractDir containing spaces.
115-118: Invocation/logging improvements LGTM.Switch to -FilePath/-ArgumentList and friendly_path -path is clear and consistent.
130-135: Non-tar cleanup LGTM; tar path covered by earlier fix.Moving from TEMP to destination and removing TEMP dir looks correct for non-tar flows.
287-303: Zip: TEMP extraction for ExtractDir LGTM.Casting to [string], using TEMP GUID, moving back, and cleanup are correct.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/decompress.ps1 (2)
95-95: Broaden tar detection to include tzst/tlz variantsCurrent regex misses .tzst/.tar.zst and .tlz. Consider:
-$IsTar = ((strip_ext -fname $Path) -match '\.tar$') -or ($Path -match '\.t[abgpx]z2?$') +$IsTar = ((strip_ext -fname $Path) -match '\.tar$') -or + ($Path -match '\.(tgz|tbz2?|txz|tlz|tzst)$') -or + ($Path -match '\.t(ar\.)?(gz|bz2?|xz|lz|zst)$')This improves robustness across common compressed-tar extensions.
98-103: Nit: Prefer ToString() for GUID and consistent path joinNo functional issue, but this is more idiomatic and explicit:
-$DestinationPathTemp = [System.IO.Path]::Combine($env:TEMP, [guid]::NewGuid().'Guid') +$DestinationPathTemp = [System.IO.Path]::Combine($env:TEMP, ([guid]::NewGuid().ToString()))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)lib/decompress.ps1(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (2)
CHANGELOG.md (1)
18-18: Changelog entry looks goodAccurately describes the new temp-extraction behavior for 7z/zip with ExtractDir.
lib/decompress.ps1 (1)
287-306: Zip temp-extraction flow LGTMTrims trailing slashes, extracts to TEMP when ExtractDir is set, then moves and cleans up. Matches objectives and avoids prior empty-dir pitfalls.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
18-18: Clarify behavior, reference #6093, and add a breaking note.Spell out "extract to temp first, then move," include the feature request link, and note potential manifest impact.
- - **decompress:** Expand to temp directory if `$ExtractDir` is specified with `Expand-7zipArchive` and `Expand-ZipArchive`, to make it more robust ([#6515](https://github.com/ScoopInstaller/Scoop/issues/6515)) + - **decompress:** Extract to a temporary directory first when `$ExtractDir` is specified for `Expand-7zipArchive` and `Expand-ZipArchive`, then move into `$ExtractDir` to avoid path-collision/cleanup issues ([#6515](https://github.com/ScoopInstaller/Scoop/issues/6515), [#6093](https://github.com/ScoopInstaller/Scoop/issues/6093)) + - BREAKING: Changes extraction semantics when `extract_dir` is used; manifests relying on post-extraction cleanup workarounds may need updates.
o-l-a-v
changed the title
movedir
This isn't specifically meant to remove leftovers ( As far as I'm aware, there aren't any related workarounds to delete leftovers after $ExtractDir. |
2 participants
Description
Expand-7zipArchivehave had problems with apps where:$ExtractDiris named the same as a dir above it (issue [Bug] Missing folders after installing packages making applications behave incorrectly #6011, PR fix (decompress):Expand-7zipArchiveonly delete temp dir /$extractDirif it is empty #6092)$ExtractDirthat are two levels deep or more (issue [Bug] Expand-7zipArchive won't remove top folder if $ExtractDir depth exceeds 2 #6515).$ExtractDirmore robust I previously suggested to extract to TEMP first, then move content from there (feature request [Feature] decompress - Extract to isolated temp directory outside$DestinationPathif-ExtractDirto avoid problems like #6011 #6093).This PR tries to fix all that.
Motivation and Context
$DestinationPathif-ExtractDirto avoid problems like #6011 #6093.Merging this might break some manifests that have workarounds to delete leftovers after
$ExtractDir. Here are some I know of:How Has This Been Tested?
Checklist:
developbranch.Summary by CodeRabbit
New Features
Bug Fixes
Documentation