Skip to content
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

decompress.ps1: Rewrite extract_xxx functions #3204

Merged
merged 18 commits into from
Apr 26, 2019

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Mar 10, 2019

PART II of #3169

  • Combine lessmsi_config, extract_lessmsi into extract_msi (unique function, switch by MSIEXTRACT_USE_LESSMSI)
  • Rewrite unpack_inno to extract_inno and handle InnoSetup in dl_urls with extract_inno
  • Use Expand-Zipfile function in scoopinstaller/install as new extract_zip
  • Use &(file_path 7zip 7z.exe) instead of 7z shim to avoid error caused by scoop install 7zip 7zip-zstd; scoop uninstall 7zip-zstd
  • extract_xxx now have similar params ($path, $to, $recurse)
  • Remove unused condition in depends.ps1

Test Cases are as follow:

Describing Decompression function

  Context 7zip extractionLoading 7z1900-x64.msi from cache
Checking hash of 7z1900-x64.msi ... ok.
Extracting 7z1900-x64.msi ... done.
Linking D:\Scoop\apps\7zip\current => D:\Scoop\apps\7zip\19.00
'7zip' (19.00) was installed successfully!
    [+] extract normal compressed file 179ms
    [+] extract nested compressed file 495ms
    [+] extract nested compressed file with different inner name 235ms
    [+] works with '$recurse' param 89ms

  Context msi extraction
Loading lessmsi-v1.6.3.zip from cache
Checking hash of lessmsi-v1.6.3.zip ... ok.
Extracting lessmsi-v1.6.3.zip ... done.
Linking D:\Scoop\apps\lessmsi\current => D:\Scoop\apps\lessmsi\1.6.3
'lessmsi' (1.6.3) was installed successfully!
    [+] extract normal MSI file 1.71s
    [+] extract empty MSI file using lessmsi 317ms
    [+] works with '$recurse' param 1.11s

  Context inno extraction
Loading innounp048.rar from cache
Checking hash of innounp048.rar ... ok.
Extracting innounp048.rar ... done.
Linking D:\Scoop\apps\innounp\current => D:\Scoop\apps\innounp\0.48
'innounp' (0.48) was installed successfully!
    [+] extract Inno Setup file 246ms
    [+] works with '$recurse' param 169ms

  Context zip extraction
    [+] extract compressed file 117ms
    [+] works with '$recurse' param 112ms

Please test and review. @r15ch13 @rasa @Ash258 @h404bi

PS. After merge, this PR will fix #3120, fix #3198, fix #3287, fix #3230, fix #3080, fix #3374, fix #3339, fix #3388

@niheaven
Copy link
Member Author

Fix #3080 in 079c4a5

@chawyehsu
Copy link
Member

Could you add more tests to increase the test coverage of the decompress functionality? Such as extracting .tar.*, etc.

@niheaven
Copy link
Member Author

I'm trying to, but further test cases need 7zip, lessmsi, innounp (and maybe dark?). I'm not sure appveyor could install these tools just for testing. If it could, I could add more. Otherwise, I would try to seperate some function except for actually extraction for testing param passthru.

@r15ch13
Copy link
Member

r15ch13 commented Mar 18, 2019

7-Zip 18.05 is available on Appveyor: https://www.appveyor.com/docs/windows-images-software/#tools
Will try adding lessmsi and innounp (test branch: https://github.com/lukesampson/scoop/tree/appveyor-tests)

@Ash258
Copy link
Contributor

Ash258 commented Mar 18, 2019

You could always use scoop installing as backup plan, if there is no better solution. Create the last test case in which you just do something like bin\scoop.ps1 refresh; bin\scoop.ps1 install <whatever you like> in beginning. And then test these extractions.

@niheaven
Copy link
Member Author

niheaven commented Mar 18, 2019

It's better to use scoop installed 7zip lessmsi and innounp, since decompress.ps1 use these ones. Using just 7z x will lead to some errors in practice, so I change it to &(file_path 7zip 7z.exe).

@niheaven
Copy link
Member Author

niheaven commented Mar 18, 2019

You could always use scoop installing as backup plan, if there is no better solution. Create the last test case in which you just do something like bin\scoop.ps1 refresh; bin\scoop.ps1 install <whatever you like> in beginning. And then test these extractions.

I'll try to make some more test cases.

@r15ch13
Copy link
Member

r15ch13 commented Mar 18, 2019

Just downloads lessmsi and innounp: 1facb58
Will not work by using file_path() though.

https://ci.appveyor.com/project/lukesampson/scoop/builds/23155191/job/ij3b30cvq9as63hk

@niheaven niheaven force-pushed the rewrite-decompress branch 9 times, most recently from df1d165 to 5c75413 Compare March 19, 2019 10:05
@niheaven
Copy link
Member Author

Sorry for so many force-push, I've tested CI with installing tools by scoop and achieved the goal with a simplified install_app_ci function forked from install_app.

Now I could add more test cases with these installed tools.

PS. I'm not familiar with Pester's Tag, and I want to make some tweak in test\bin\test.ps1 that it wouldn't test Decompress.Tests.ps1 while decompress.ps1 is not changed. Please review the two test files and make sure it works.

@niheaven niheaven force-pushed the rewrite-decompress branch 11 times, most recently from 3390bed to 7fd9c2b Compare March 21, 2019 03:46
@Ash258
Copy link
Contributor

Ash258 commented Apr 18, 2019

Is it all?
I should have time today in office to test some manifests installation and today / tomorrow it could be merged

SideNote: Could you look into ScoopInstaller/Extras#2021 (comment) in next PR? Edit Expand-7zip function with some optional [ValueFromRamainingArguments] $Arguments # Or just string whatever and then call 7z x .. @Arguments

@niheaven
Copy link
Member Author

It should be enough for a single PR.

I'll add some universal exe unpacker functions in the next PR, and check the commit for NSIS unpacker.

@Ash258
Copy link
Contributor

Ash258 commented Apr 18, 2019

Tested:

  • acmesharp for simple zip archive
  • skype for innosetup
  • chroma for tar.gz
  • gnupg1 for exe => .7z overwrite
  • aws for msi
  • brotli for tar.xz
  • coreutils for tar.lzma

@niheaven
Copy link
Member Author

niheaven commented Apr 18, 2019

Thanks, will you do more tests on it?

@Ash258
Copy link
Contributor

Ash258 commented Apr 18, 2019

I don't think so. I would put it into wild.

Copy link
Contributor

@Ash258 Ash258 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Ash258
Copy link
Contributor

Ash258 commented Apr 21, 2019

Could you please adopt deprecation warning to old functions (extract_7z, ..) after #3341 is merged??
#3341 (comment)

Just merge master and add `Show-DeprecationMessage $MyInvocation 'New-Name'

@niheaven
Copy link
Member Author

Okay, let's move on.

@niheaven
Copy link
Member Author

It's ready to merge. @r15ch13 @Ash258

@r15ch13
Copy link
Member

r15ch13 commented Apr 26, 2019

@r15ch13
Copy link
Member

r15ch13 commented Apr 26, 2019

Also install_app_ci should check if the program is already available. Otherwise it will always download (or load the file from cache) and install it on each run of Invoke-Pester

@r15ch13
Copy link
Member

r15ch13 commented Apr 26, 2019

Fixed with 79a6d5c f919a3c ff15d1d

@niheaven
Copy link
Member Author

@r15ch13 Thanks, I'm not familliar with Pester...

@niheaven niheaven deleted the rewrite-decompress branch April 29, 2019 03:31
r15ch13 pushed a commit that referenced this pull request Apr 29, 2019
ref: ScoopInstaller/Extras#2021 (comment), #3204 (comment)

- Add new param `Overwrite` to `Expand-7ZipArchive`, with following values
  - "All": 7z param `-aoa`, overwrite All existing files without prompt, behave the same with default
  - "Skip": 7z param `-aos`, Skip extracting of existing files
  - "Rename": 7z param `-aou`, aUto rename extracting file
- Add new param `Switches` to `Expand-7ZipArchive` and `Expand-InnoArchive`
  - It could pass all unrecognized params to `7z` and `innounp`
- Patch `extract_7zip` and `unpack_inno` to adapt to `Expand-XXX`

Usage: ScoopInstaller/Extras#2021, ScoopInstaller/Extras#2070
r15ch13 added a commit that referenced this pull request May 12, 2019
- The main bucket of Scoop has been separated to https://github.com/ScoopInstaller/Main
- Requires PowerShell 5 and newer #3330
- Improve and refactor decompression functions #3204 #3409 #3411
- Move scoop config from `~\.scoop` to `~\.config\scoop\config.json` #3242
- Add `scoop hold` and `scoop unhold` for excluding programs from updates #3444
- Refactored some Core functions #3314 #3416
- Small fix for starting processes on Windows 7 #3415

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
Co-authored-by: Chawye Hsu <h404bi@outlook.com>
Co-authored-by: Jakub Čábera <cabera.jakub@gmail.com>
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.

4 participants