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

fix(decompress): Fix nested Zstd archive extraction #4608

Merged
merged 2 commits into from
Dec 30, 2021

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Dec 29, 2021

Also fix test cases error

Description

There're some stupid mistakes in Expand-ZstdArchive() which are totally introduced by me (#4372). This one fixes them, and bypasses decompress test.

How Has This Been Tested?

image

Checklist:

  • I have read the Contributing Guide.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@rashil2000
Copy link
Member

Do you have a manifest to test before and after this change?

@niheaven
Copy link
Member Author

Run the test, since before and after the PR, msys type manifest always works well...

The error only occured when destination dir is not archive dir.

PS, test will hault before since waiting for interaction of overwriting, and a wrong function name...

@niheaven
Copy link
Member Author

I made a test manifest:

test.json
{
    "version": "11.2.0-2",
    "description": "GNU Compiler Collection (Mingw-w64 port from MSYS2 project)",
    "homepage": "http://mingw-w64.org/doku.php",
    "license": "GPL-3.0-or-later,ZPL-2.1,...",
    "architecture": {
        "64bit": {
            "url": [
                "https://mirror.msys2.org/mingw/mingw64/mingw-w64-x86_64-binutils-2.37-4-any.pkg.tar.zst"
            ],
            "hash": [
                "a518d2630c11fe363abd394763d0bb82fdde72386ffb58d87ecc8f46cbe878d6"
            ]
        },
        "32bit": {
            "url": [
                "https://mirror.msys2.org/mingw/mingw32/mingw-w64-i686-binutils-2.37-4-any.pkg.tar.zst"
            ],
            "hash": [
                "20b2e5beefb831c665ac5b1be4feb447e0f4e543e15c0fd5855dc0c258a29a02"
            ]
        }
    },
    "extract_to": "test"
}

Before:

image

After:

image

@niheaven
Copy link
Member Author

@rashil2000 Do you have some time to test this PR (I've attached a testing manifest)? I've some test fixes that based on this...

@niheaven niheaven merged commit d5cb860 into develop Dec 30, 2021
@niheaven niheaven deleted the fix-zstd-extraction branch December 30, 2021 17:04
@chawyehsu
Copy link
Member

chawyehsu commented Jan 12, 2022

Looks like the tests of Expand-ZstdArchive are false negative. They all fail on CI despite pass on local machine...

https://ci.appveyor.com/project/chawyehsu/scoop/builds/42173333/job/g5alk2c1s63s50aj#L245

@niheaven Could you please take a look at this. I don't know why the Core's appveyor CI stopped.

@niheaven
Copy link
Member Author

niheaven commented Jan 12, 2022

It's caused by wrong zstd.exe extraction and Mock, and will be fixed in #4651 @chawyehsu

https://ci.appveyor.com/project/niheaven/scoop/builds/42177522/job/l8b7kyla9deyiohx

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.

3 participants