install.sh: Change shell interpreter from sh to bash#4965
Closed
raptorswing wants to merge 1 commit intoDioxusLabs:mainfrom
Closed
install.sh: Change shell interpreter from sh to bash#4965raptorswing wants to merge 1 commit intoDioxusLabs:mainfrom
raptorswing wants to merge 1 commit intoDioxusLabs:mainfrom
Conversation
The syntax on line 16 and elsewhere, i.e. `if [[ -t 1 ]]; then` doesn't work in sh - it's a bash feature. Tested on Ubuntu 25.10. The current install script errors with: ``` ./install.sh: 16: [[: not found ./install.sh: 50: [[: not found ./install.sh: 69: [[: not found #=O#- # # curl: (22) The requested URL returned error: 404 ``` Changing to `#!/bin/bash` solves the problem. It was probably working in many places because some systems symlink /bin/sh to /bin/bash.
Contributor
|
I don't think that's enough, as in docs curl piped to sh. Why not convert the script to posix instead? LLMs are good at that. |
Author
D'oh, good point. Opened a separate PR to update the docs: DioxusLabs/docsite#594
Rewriting it to be POSIX shell compatible is a good idea if it truly needs to run under /bin/sh, and maybe we'd get lucky asking an LLM to do it, but I'm not familiar enough with POSIX shell scripting to confidently review the LLM's output. If updating to bash is undesirable, that's OK, I can withdraw these PRs or you can close them. |
Member
|
Closing in favor of #5058 which properly implements sh idioms |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
[[ ]]syntax on line 16 and elsewhere, i.e.if [[ -t 1 ]]; thendoesn't work in sh - it's a bash feature.Tested on Ubuntu 25.10. The current install script errors with:
On this system, /bin/sh is symlinked to dash.
Changing to
#!/bin/bashsolves the problem, and the script works as expected.It was probably working in many places because some systems symlink /bin/sh to /bin/bash.