dvershinin / lastversion

Find the latest release version of an arbitrary project
https://lastversion.getpagespeed.com
BSD 2-Clause "Simplified" License
376 stars 33 forks source link

fix(utils): improve unzip/extract #141

Closed lxl66566 closed 9 months ago

lxl66566 commented 9 months ago

fix #98

dvershinin commented 9 months ago

@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 commented 9 months ago

@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 commented 9 months ago

@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.

sonarcloud[bot] commented 9 months ago

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 commented 9 months ago

@lxl66566 thank you, it works great!

lxl66566 commented 9 months ago

@dvershinin Thank you for reviewing this PR!