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

Error downloading ShellCheck on Mac with v2.1.0 #60

Closed
rpastro opened this issue Feb 2, 2023 · 23 comments · Fixed by #62
Closed

Error downloading ShellCheck on Mac with v2.1.0 #60

rpastro opened this issue Feb 2, 2023 · 23 comments · Fixed by #62

Comments

@rpastro
Copy link

rpastro commented Feb 2, 2023

@carlocorradini Unfortunately v2 is still not working on Mac.

rpastro@Rodrigos-MBP ngls % npx shellcheck
[INFO ] ShellCheck binary not found or invalid, downloading to '/Users/rpastro/ngls/ngls/node_modules/shellcheck/bin/shellcheck'
[INFO ] Downloading 'https://github.com/koalaman/shellcheck/releases/download/v0.9.0/shellcheck-v0.9.0.darwin.x86_64.tar.xz' to '/var/folders/wl/gf_x6dwn6gn083lnyknddz340000gn/T/5HUi69/shellcheck.download'
[INFO ] Extracting '/var/folders/wl/gf_x6dwn6gn083lnyknddz340000gn/T/5HUi69/shellcheck.download' to '/var/folders/wl/gf_x6dwn6gn083lnyknddz340000gn/T/5HUi69'
dyld[92369]: missing symbol called
zsh: abort      npx shellcheck

Has anything changed in v2.1.0 compared to v1.1.0 on how the downloaded file is extracted?

@carlocorradini
Copy link
Collaborator

Try:

rm -rf node_modules
npm install

If does not work: https://gist.github.com/adrienjoly/e29a6e73fb7e701eefd80ff9bde9abeb

I think the error is due to the decompression libraries that depend on native code.

@carlocorradini
Copy link
Collaborator

@rpastro See this

@carlocorradini
Copy link
Collaborator

I hate .tar.xz compression...
Moreover, there are no good libraries in Node.js that handle decompression easily and with a wide support (if you have a suggestion 🤯).
The only way to solve this is detect darwin arm64 and use the "native" tar. This solution is very very ugly but is the only one I came up with until this is fixed :/

Let me know what you think 😥

@rpastro
Copy link
Author

rpastro commented Feb 2, 2023

I tried overriding the lzma-native library back to 7.0.1 and it seems to have solved the issue for Mac M1. I don't know, however, if this could cause issues for other deployments.

  "overrides": {
    "@felipecrs/decompress-tarxz@^4.0.0": {
        "lzma-native": "7.0.1"
    }
  },
rpastro@Rodrigos-MBP ngls % npm list lzma-native
[email protected] /Users/rpastro/ngls/ngls
└─┬ [email protected]
  └─┬ @felipecrs/[email protected] overridden
    └── [email protected] overridden

How were you decompressing the tar files in v1.1.0?

@carlocorradini
Copy link
Collaborator

Wow thanks!
The only problem with older versions I noticed is on windows 😅
Thanks again for using your M1 as a testing platform.

@carlocorradini
Copy link
Collaborator

carlocorradini commented Feb 2, 2023

V1 is only a shell script and a super simple js file :/
See https://github.com/gunar/shellcheck/tree/911a2de0e45a79024fe6dd5ab39b78e99fc06cc6

@carlocorradini
Copy link
Collaborator

I've tested lzma-native": "7.0.1" on Linux x64 and Windows x64 and it is working

@carlocorradini
Copy link
Collaborator

@rpastro
See this for more information.
I've asked to publish a version with lzma-native": "7.0.1" under a different tag name.
I'm hoping for a quick response.

@carlocorradini
Copy link
Collaborator

I apologize for the inconvenience, but as far as I can see, supporting Apple Silicon is not simple...

@rpastro
Copy link
Author

rpastro commented Feb 2, 2023

Thanks @carlocorradini. I really appreciate your prompt responses.

@felipecrs
Copy link

felipecrs commented Feb 2, 2023

Guys... let me help you out. We (re)package all ShellCheck binaries in tar.gz format for use in the VSCode ShellCheck extension: https://github.com/vscode-shellcheck/shellcheck-binaries/releases

It even includes a binary for Apple M1.

I'd suggest downloading from there instead.

@carlocorradini
Copy link
Collaborator

I'm crying 😭

@carlocorradini
Copy link
Collaborator

@felipecrs
A dummy question: why not also including Windows?

@felipecrs
Copy link

felipecrs commented Feb 3, 2023

The Windows one is already in zip format, so we didn't bother. But I don't mind, if you want to add it to CI too, let's do it.

@carlocorradini
Copy link
Collaborator

@felipecrs
If you are not bothered by it.
It is simply for convenience (everything in one place/repo and format).
It also allows to remove the dependency decompress-unzip.
Thank you one again

@felipecrs
Copy link

felipecrs commented Feb 3, 2023

Here's the script, please send a PR. It should be simple. I can merge it for you today yet.

https://github.com/vscode-shellcheck/shellcheck-binaries/blob/main/scripts/download_and_archive.sh

@felipecrs
Copy link

felipecrs commented Feb 3, 2023

Did you start working on it already? If not, I can do it.

@carlocorradini
Copy link
Collaborator

Yup, working on it :)

@carlocorradini
Copy link
Collaborator

@felipecrs Done

@carlocorradini
Copy link
Collaborator

@rpastro Working on it
You will have native binaries

@carlocorradini
Copy link
Collaborator

@rpastro
See PR #21

@rpastro
Copy link
Author

rpastro commented Feb 4, 2023

@carlocorradini. I'll test it on the M1 as soon as the new version is released. Thanks a lot for your support.

@rpastro
Copy link
Author

rpastro commented Feb 4, 2023

@carlocorradini v2.2.0 worked perfectly. Thanks again.

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 a pull request may close this issue.

3 participants