Added install.sh script
#625
Conversation
|
Though this script is not as advanced as |
There was a problem hiding this comment.
I think we'll need some more time to think about where we want to go with this script.
Some parts of it, such as installing Luarocks, service the Github CI. Other parts, such as the usage, service our users and the README. Unfortunately, it seems that at times they pull in opposite directions.
install.sh
Outdated
| install_pallene | ||
| ;; | ||
| --help) | ||
| usage 0 |
There was a problem hiding this comment.
Put the exit 0, outside the usage, instead of inside it.
There was a problem hiding this comment.
Readability; make the function only do one thing.
install.sh
Outdated
| cd $CLONE_DIR | ||
| wget -O - https://luarocks.org/releases/luarocks-$LUAROCKS_VERSION.tar.gz | tar xzf - | ||
| cd luarocks-$LUAROCKS_VERSION | ||
| ./configure --with-lua=/usr/local |
There was a problem hiding this comment.
Not a big fan of hardcoded paths in an installation script.
There was a problem hiding this comment.
But Lua internals will always be installed in /usr/local prefix. If there is a valid, MYCFLAGS way to install Lua to a different prefix, we should make it variable. Otherwise, it is fine.
There was a problem hiding this comment.
Nevermind. Now LUA_PREFIX env variable controls related to install and lua prefix.
install.sh
Outdated
| if [ "${PALLENE_LOCAL:-0}" -eq 1 ]; then | ||
| SUPERUSER= | ||
| LOCAL_FLAG=--local | ||
| fi |
A tool can be useful to multiple usecases, just because it is working for both CI and users doesn't mean it's bad in general. We don't need something so powerful like localua.sh, which does everything regarding Lua under the sun. We need a simple script which is really useful in CI and increases UX by not allowing users to read through the entire readme and copy pastE commands. Users can install the entire Pallene ecosystem just with a single command (Here ecosystem means, Pallene, Lua internals and Pallene Tracer ;)). If users want, say they want to update just the Lua internals, they can do it in a single command too. So, I cannot see how it is not for the users. Is the script super feature-rich? No, we don't need that. |
|
|
||
| # Versions | ||
| LUA_VERSION=5.4.7 | ||
| LUAROCKS_VERSION=3.9.1 |
There was a problem hiding this comment.
Unlike the LUA_VERSION and PT_VERSION, the LUAROCKS_VERSION is only there because our CI needs a Lua version. In theory we could always go with a more recent version. (And in fact, it's probably about time we bump this to a newer Luarocks...)
| CURR_DIR=$(pwd) | ||
|
|
||
| # Where to install/locate Lua | ||
| LUA_PREFIX=${LUA_PREFIX:-/usr/local} |
There was a problem hiding this comment.
Please initialize everything that has a default value here on top. That way it's easier to see what all the configuration options are. (For example, PALLENE_LOCAL, etc)
| echo | ||
| echo "Pallene Tracer:" | ||
| echo " Use PT_TESTS=1 to run Pallene Tracer tests after installation, e.g." | ||
| echo " PT_TESTS=1 ./install.sh ptracer" |
There was a problem hiding this comment.
Can we remove the PT_TESTS functionality? I would rather have the CI run those in a separate step than integrate them into the installation script. It complicates the script, and also introduces a dependency on busted.
| echo | ||
| echo " Use LUA_PLAT to define Lua platform. Defualt is 'linux'." | ||
| echo " Available platforms: guess aix bsd c89 freebsd generic ios linux linux-readline macosx mingw posix solaris" | ||
| echo " e.g. LUA_PLAT=linux-readline ./install.sh lua" |
There was a problem hiding this comment.
Default plat should be "guess"
In the documentation, we should encourage users to use linux-readline option.
| usage | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
I wonder if there is a way to make the script smarter, and only install the things that have a new version.
Changelog:
install.sh.ci.ymlscript accordingly. If this PR gets merged, we may not need to put our hands on this script ever again (hopefully), Yay!Resolves #620