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(utils): improve unzip/extract #141

Merged
merged 3 commits into from
Jan 26, 2024
Merged

Conversation

lxl66566
Copy link
Contributor

fix #98

  • (breaking change): fix extracting rootless tar.
    • previous code assumes that the tar file has a dir as root (all_members[0]), but some tar file has a flat structure without a root dir.
  • rewrite tar safety check and extract function
  • add zip support
  • support extract to specific dir, which is useful for external execution

@dvershinin
Copy link
Owner

@lxl66566 thank you for your contribution, but it looks like it's going to be a breaking change. because the function of unzip will change from "extract useful contents of release to current directory" to "extract release archive to current directory", which is evident from your updated test. Could you update it to keep the previous behavior for archives like WordPress?

@lxl66566
Copy link
Contributor Author

@dvershinin "extract useful contents of release to current directory" thinks only files in the first root dir are useful contents. Actually most of the archives has the same structure, but some others has not.

lastversion --assets unzip https://github.com/lxl66566/test
lastversion --assets unzip https://github.com/eza-community/eza --filter='(?=.*x86_64)(?=.*linux)(?=.*gnu)(?=.*\.tar\.gz)'

lastversion will not extract anything when a tar file is rootless.

If we are to find a solution that accommodates both of our perspectives, "judge and deal with both circumstance seperately" may be an answer, but can also cause confusion to users.

What do you think about it?

@dvershinin
Copy link
Owner

@lxl66566 I just prefer to keep backward compatibility unless absolutely required. For me personally lastversion is a critical infra tool, so something probably will break when changing from rootless unzip to unzip with top-level directory. Is it possible to check if there's one top-level directory only then place its contents only. Otherwise, the new behavior. I realize you've fixed the bug, I just wish it won't break how it works now for most source archives on github.

Copy link

sonarcloud bot commented Jan 26, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@dvershinin dvershinin merged commit d9cd7b6 into dvershinin:master Jan 26, 2024
5 of 6 checks passed
@dvershinin
Copy link
Owner

@lxl66566 thank you, it works great!

@lxl66566
Copy link
Contributor Author

@dvershinin Thank you for reviewing this PR!

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.

unzip/extract options can't handle .7z files for some reason
2 participants