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

Fixed MINGW/MSYS2 OS detection and zip extraction #192

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matteopt
Copy link

No description provided.

@jorgebucaran
Copy link
Owner

Hey, thanks a lot for your contribution! Sorry for the delay in reviewing it. I'm having a hard time finding time to test it properly, so I can't move forward at this time. If someone with more free time and a Windows terminal can help review, that would be amazing and I'd be happy to take another look. 🙏

Ping @tasop-rsconnect @M4RC3L05

@M4RC3L05
Copy link

M4RC3L05 commented Apr 30, 2023

Hey, thanks a lot for your contribution! Sorry for the delay in reviewing it. I'm having a hard time finding time to test it properly, so I can't move forward at this time. If someone with more free time and a Windows terminal can help review, that would be amazing and I'd be happy to take another look. 🙏

Ping @tasop-rsconnect @M4RC3L05

Working on a clean msys2 install using mingw64 shell
Screenshot 2023-04-30 101626

Should we still change the readme to have it state that for msys2 to check for either unzip or bsdtar. Just to make sure that the user allways check to make sure it works, idk, wdyt? @jorgebucaran @paramtt

@jorgebucaran
Copy link
Owner

Yeah, we should definitely add that to the system requirements. But hey, I did a quick search and it looks like Windows has this built-in expand command that's always there. Maybe we could use that instead? Or is it not the same thing?

@M4RC3L05
Copy link

M4RC3L05 commented Apr 30, 2023

Yeah, we should definitely add that to the system requirements. But hey, I did a quick search and it looks like Windows has this built-in expand command that's always there. Maybe we could use that instead? Or is it not the same thing?

Since this is running on msys2 the command expand gets mapped to the linux conterpart, to convert tabs to spaces, it is a different shell from cmd or powershell.
As @paramtt said on the issue linked to this PR it fallbacks to bsdtar that comes bundled.
I believe that it is ok to use that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants