I made this script when I started downloading the latest stable linux kernel's source from https://www.kernel.org to compile it myself, now I have improved it a bit and decided to create a public repo for it on github (https://github.com/Tom1380/latestkernel).
This is the code (mind you this is the first time I made bash scripts):
lk (stands for latest kernel):
#!/usr/bin/env bash
cd ~
if [ ! -d "linux-stable" ]; then
mkdir linux-stable
fi
if [ ! -f "/etc/latest_kernel" ]; then
if [[ $EUID > 0 ]] ; then
echo "/etc/latest_kernel does not exist and you are not running the script as root, " \
"please fix that either by manually running 'sudo create_etc_lk', " \
"or rerun the script as root to automatically fix it."
exit
else
touch /etc/latest_kernel
echo "$(uname -r)" > /etc/latest_kernel
fi
fi
wget=$(wget --output-document - --quiet https://www.kernel.org/ | grep -A 1 "latest_link")
wget=${wget##*.tar.xz\">}
wget=${wget%</a>}
latest_acknowledged=$(</etc/latest_kernel)
if [ "$wget" == "$latest_acknowledged" ]; then
echo "You have the latest stable kernel downloaded: $wget"
# Uncomment (take out the #) the first 4 lines below if you want to be prompted when you are using a different kernel than
# the latest one while already having installed it
# runningkernel=$(uname -r)
# if [ $runningkernel != $latest_acknowledged ]; then
# echo "But you are running $runningkernel"
# fi
exit
fi
echo "Updated kernel available: $wget, you have $latest_acknowledged."
if [[ $EUID > 0 ]] ; then
echo "If you wish to download the latest kernel, rerun the script as root."
exit
fi
# Since there is only a '>', /etc/latest_kernel will be overwritten entirely.
echo "$wget" > /etc/latest_kernel
echo "Writing latest kernel available in /etc/latest_kernel."
echo "Preparing to parse link to latest kernel for wget."
wget=$(wget --output-document - --quiet https://www.kernel.org/ | grep -A 1 "latest_link")
wget=${wget##*<a href=\"}
wget=${wget%\">*}
echo "Done parsing."
cd ~/linux-stable
echo "Changed cwd to ~/linux-stable to download kernel source."
echo "Downloading, this may take up to 10 minutes."
wget $wget
echo "Finished downloading..."
echo "Uncompressing the kernel's source."
tar xvfJ linux-$(</etc/latest_kernel).tar.xz
echo "Done uncompressing the kernel's source."
rm linux-$(</etc/latest_kernel).tar.xz
echo "Done removing the old archive, end of the script."
create_etc_lk:
#!/usr/bin/env bash
if [[ $EUID > 0 ]] ; then
echo "You need root privileges for this script."
exit
fi
touch /etc/latest_kernel && echo "$(uname -r)" > /etc/latest_kernel
install:
#!/usr/bin/env bash
if [[ $EUID > 0 ]] ; then
echo "You need root privileges for this script."
exit
fi
cp lk /usr/bin/lk
cp create_etc_lk /usr/bin/create_etc_lk
cp purgelatestkernel /usr/bin/purgelatestkernel
purgelatestkernel:
#!/usr/bin/env bash
if [[ $EUID > 0 ]] ; then
echo "You need root privileges for this script."
exit
fi
cp lk /usr/bin/lk
cp create_etc_lk /usr/bin/create_etc_lk
cp purgelatestkernel /usr/bin/purgelatestkernel
PS: Feel free to clone the repo if you think lk might be helpful to you
2 Answers 2
Good stuff:
- Yay for putting your code on github. I see a lot of folks afraid to wait until everything is perfect to do that. But having the backup and history of git is too handy to wait like that. So I love seeing somebody doing that from day 1 of a project.
- Yay for use the double square bracket form of
ifand checking for common mistakes. - Yay for using
env. As you can see from my github I'm not so great at this one, but it is a good idea.
Suggestions:
Move common code into a library. Your
EUIDcheck is the most obvious culprit. To create a shell library you should create a file of functions and thensourceit into each of your commands. So to translate your check into a function is easy:function validate_root { if [[ $EUID > 0 ]] ; then echo "You need root privileges for this script." exit fi }Then use the library later like so:
source lib.sh validate_root # be root or exitMove common data into the library. The most common item is
/etc/latest-kernel... what if you want to move it somewhere else? If it is in a variable in one place it will be a lot easier to do. There's no reason not to throw those into the common shell library file too.- It looks like you made a cut and paste error on
purgelatestkernelbut since I can check out the github version I can see what you meant there. Generally you're not supposed to edit code during code review put I think you'd be ok to fix that part up since you're not altering the code you meant to review. Avoid duplicate code by looping in the
install:for file in lk create_etc_lk purgelatestkernel do echo copying $file to /usr/bin cp $file /usr/bin doneThis also makes it easy to add other things like a validation of permissions or check for return values without insane amounts of error-prone cut and paste.
- Instead of having the option for a prompt be something that requires a source code alteration why not make it conditional based on a variable that possibly could be altered with a command line option?
-
\$\begingroup\$ Dude I had no idea bash had something similar to imports! This is gold! Thanks, I'm editing the files right now and pushing them to the repo. \$\endgroup\$Tommaso Thea– Tommaso Thea2017年05月15日 11:34:45 +00:00Commented May 15, 2017 at 11:34
Exit code
When a script fails, it should exit with a non-zero exit code, so that other scripts using it could respond accordingly, take appropriate actions or clean up.
For example, it's a good idea to exit with non-zero in the root user validation part:
if [[ $EUID > 0 ]] ; then
echo "You need root privileges for this script."
exit 1
fi
Download once and reuse
The script downloads https://www.kernel.org/ twice,
and then extracts different parts from it.
That's unnecessary.
You could download once and extract the different parts into different variables.
And those variables could reflect their purpose better, for example version and url, respectively.
Delay filesystem writes
The script creates the directory ~/linux-stable up front,
before doing anything else.
It would be better to delay that until the directory is actually needed.
Also, instead of checking if the directory doesn't exist,
you could write more compactly using the -p flag of mkdir.
mkdir -p linux-stable
Pointless touch
You could drop the touch statement here:
touch /etc/latest_kernel echo "$(uname -r)" > /etc/latest_kernel