Making install script more robust #233
Conversation
install.sh
Outdated
| force_build=$1 | ||
| local_version="v$(grep "^version" Cargo.toml | cut -d '"' -f 2)" | ||
|
|
||
| if [ "$1" = "-f" ] || [ "$1" = "--force" ]; then |
There was a problem hiding this comment.
while conceptually better than just checking if there is an argument to the script, this would break many configs, and this is not even proper parsing (it would have to be modified to accomodate other arguments).
I don't think this is needed
There was a problem hiding this comment.
and this is not even proper parsing (it would have to be modified to accomodate other arguments)
I know but does the script more elaborate parsing? You were checking the first argument of the script, that could be anything. This is very unpredictable and it is a bad design choice handling this type of input.
I've chosen -f because it is a common pattern in cli application, but we could make it so if the user input anything other than one the script will exit and print an usage string.
# if $1 is not empty and not 1 exit
if [ -n $1 ] && [ $1 != 1 ]; then
echo "Usage: install.sh <1>"
echo "1: force install"
exit 1
fi
|
If you want we can maintain the script without structural change and only convert it to a POSIX shell for better compatibility |
|
That would make the PR go from 'meh' to 'great' :-) I think the CI will pick up automatically the new interpreter However, another user had proposed a change on install.sh since you opened this PR (that allows seeing in a plugin's manager window the output of cargo build while installing), so you may want to rebase your changes, or maybe create a new PR altogether |
|
It is the same script but now can run on every POSIX complaint OS. Made che changes and rebase on new master |
Hi,
This PR tries to make the install script more robust, and more portable to other system other than mainstream Linux distros.
Changes: