4
\$\begingroup\$

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

asked May 14, 2017 at 13:52
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

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 if and 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 EUID check is the most obvious culprit. To create a shell library you should create a file of functions and then source it 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 exit
    
  • Move 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 purgelatestkernel but 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
    done
    

    This 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?
answered May 14, 2017 at 20:39
\$\endgroup\$
1
  • \$\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\$ Commented May 15, 2017 at 11:34
1
\$\begingroup\$

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
answered May 16, 2017 at 21:54
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.