The script runs fine, and it seems to be working correctly. I'm looking for some criticism of the structure of the code, errors, bad practices, beginner's pitfalls, and bad code in general. I'm looking forward to improving my understanding of shell scripting.
#!/bin/bash
#Setting variables according to user preferences.
echo "What is it going to be your hostname?"
read -r yourname
echo "Is this a laptop, (1)YES, (0)NO?"
read -r laptopyn
echo "the CPU is INTEL(1) or AMD(0)"
read -r cpu
echo "If you would like to reenter the information type (1)YES; (2)NO"
read -r again
#Loop back to first echo if again -eq 1
#if [ $again -eq 1 ]
#Setting hostname.
echo ">>>>Setting you up Champ"
echo "$yourname are you sure?"
sleep 2
hostnamectl set-hostname "$yourname"
echo "Your hostname is set to:"
hostname
sleep 2
#Updating OS & disable NtwrkMngr-wait
echo ">>>>Starting updates"
sudo dnf -y upgrade
sudo dnf -y upgrade --refresh
sudo dnf -y update
sudo dnf -y groupupdate core
sudo fwupdmgr refresh --force
sudo fwupdmgr update --force
sudo systemctl disable NetworkManager-wait-online.service
echo ">>>>You are up to date 1/10 "
sleep 1
#Downloading, Installing & Cp already customized conf file to correct path
echo ">>>>Installing Kitty terminal"
sudo dnf install wget -y
sudo dnf -y install vim
sudo dnf -y install kitty
#conf file missing the correct font:.Iosevak
sudo cp -r ~/basicfedorasetup/kitty.conf ~/.config/kitty/
echo ">>>>Kitty INSTALLED and setup 2/10"
#Enabling rpm-fusion & flatpak Repositories
echo ">>>>Sarting with the repo's"
sudo dnf install -y https://download1.rpmfusion.org/nonfree/fedora/rpmfusion-nonfree-release-$(rpm -E %fedora).noarch.rpm
echo ">>>>rpmfusion INSTALLED 3/10"
sudo dnf install -y flatpak
flatpak remote-add --if-not-exists flathub https://dl.flathub.org/repo/flathub.flatpakrepo
echo ">>>>flatpack INSTALLED 4/10"
sleep 2
#Installing essential software "for my use case & preferences"
echo ">>>>Installing essentials"
sudo dnf install -y 'google-roboto*' 'mozilla-fira*' fira-code-fonts
flatpak install flathub -y com.mattjakeman.ExtensionManager
sudo dnf install -y unzip p7zip p7zip-plugins unrar
sudo dnf install -y gnome-tweaks
sudo dnf install -y gimp
sudo dnf install -y gimp-devel
sudo dnf install -y nautilus-python
echo ">>>>google-roboto mozilla-firra,flathpak,unzip,gnome-tweaks,gimp,nautilus-python; INSTALLED 5/10"
#Installing some of my most ,non native, used commands
echo ">>>>Installing Commands"
sudo dnf install -y tmux
sudo dnf install -y iostat
sudo dnf install -y htop
sudo dnf install -y copr-cli
rpm -q cronie
rpm -q cronie-anacron
sudo dnf install -y cronie
sudo dnf install -y cronie-anacron
echo ">>>>tmux,iostat,htop,copr-cli,cronie,cronie-anacron; INSTALLED 6/10"
#Installing most of the software with visual interface that i'm using
echo ">>>>Installing Basic Software"
flatpak install -y flathub com.google.Chrome
flatpak install -y flathub com.discordapp.Discord
flatpak install -y flathub com.spotify.Client
sudo dnf copr enable -y jerrycasiano/FontManager
sudo dnf install -y font-manager
echo ">>>Chrome,Discord,Spotify,Font-Manager; INSTALLED 7/10"
sleep 2
#If marked as 1(YES), then installing battery optmazation for laptops
if [ "$laptopyn" -eq 1 ]
then
echo ">>>>Starting Laptop Optimazation"
sudo dnf -y install tlp tlp-rdw
sudo systemctl mask power-profiles-daemon
sudo dnf install -y powertop
sudo powertop --auto-tune
echo ">>>>Laptop Optimazation DONE 8/10"
else
echo ">>>>This isn't a laptop"
echo ">>>>OK 8/10"
sleep 2
fi
#Multimedia drivers
echo ">>>>Sound&Video"
sudo dnf update -y @sound-and-video
sudo dnf install -y Multimedia
sudo dnf install -y ffmpeg ffmpeg-libs libva libva-utils --allowerasing
echo ">>>>Sound&Video drivers DONE 9/10"
#CPU brand specific drivers.
if [ "$cpu" -eq 1 ]
then
echo ">>>>Installing Intel drivers"
sudo dnf -y swap libva-intel-media-driver intel-media-driver --allowerasing
else
echo ">>>>Installing AMD drivers"
sudo dnf -y swap mesa-va-drivers mesa-va-drivers-freeworld
sudo dnf -y swap mesa-vdpau-drivers mesa-vdpau-drivers-freeworld
fi
echo "CPU drivers DONE 10/10"
#Done, maybe
echo ">>>>ALL SET AND READY"
sleep 2
echo ">>>>>Your machine will be restarted in 5sec for all changes to take place"
sleep 5
sudo reboot
4 Answers 4
Documentation
Add comments to the top of the file describing the purpose of the code, the expected input and any possible important side effects.
# Automate post install process on Fedora
# This is what I mean by post install...
# Some important side effects are...
Comments
The comment phrase Setting variables is not necessary since it is obvious what the read command does. This is a simpler comment:
# Set user preferences.
I also think adding a space after the # character makes comments easier to read.
Unused code
Since this code is unused, it should be removed:
echo "If you would like to reenter the information type (1)YES; (2)NO"
read -r again
#Loop back to first echo if again -eq 1
#if [ $again -eq 1 ]
Keep track of future enhancements in a plain text file elsewhere.
Naming
For variables formed by multiple words, I recommend using snake_case to make them easier to read. For example, use your_name instead of yourname.
The variable named laptopyn is hard to understand. Again, I recommend using snake_case as well prefixing it with is_, which is common for boolean variables: is_laptop.
Indentation
Some of the indentation is misleading. For example:
echo "$yourname are you sure?"
sleep 2
hostnamectl set-hostname "$yourname"
echo "Your hostname is set to:"
hostname
sleep 2
When I see the hostname commands indented, expect them to be inside an if/else or loop construct. I would eliminate the indentation there.
In the following code, I would add indentation to the echo statements since they are inside the if/else construct:
if [ "$cpu" -eq 1 ]
then
echo ">>>>Installing Intel drivers"
sudo dnf -y swap libva-intel-media-driver intel-media-driver --allowerasing
else
echo ">>>>Installing AMD drivers"
Consistency
I find execution slightly difficult to follow because there is a vast difference between what is printed at the start of a section and the corresponding end eg:
echo ">>>>Sarting with the repo's" echo ">>>>rpmfusion INSTALLED 3/10"
echo ">>>>Installing essentials" echo ">>>>google-roboto mozilla-firra,flathpak,unzip,gnome-tweaks,gimp,nautilus-python; INSTALLED 5/10"
Instead, you should just echo "Starting something..." and then: "done" (on the same line even). Or "Starting something..." and then: "Finished something" so that we can figure out when a certain section starts and when it ends.
I would move up the section that adds rpmfusion, just after dnf update, so that it gets installed as soon as possible given that subsequent lines may depend on it.
Regrouping
There is one section dedicated to dnf update so this line is an outlier:
sudo systemctl disable NetworkManager-wait-online.service
There should be a dedicated section for networking I think. What you did for Sound & video looks sensible.
Some lines could be merged because they are logically related:
sudo dnf install -y gimp
sudo dnf install -y gimp-devel
Being user-friendly
Rebooting without prompting for confirmation is not a good idea, because you may have uncommitted work in another window (you never know). In fact you might not need to reboot, unless you've upgraded the Linux kernel. To find out if a reboot is required, here is one way for this OS family.
Being user-friendly 2
Don't ask the user for information that can be inferred without too much effort. If you want to determine the CPU type, have a look at the commands lscpu or dmidecode, with some grepping if necessary. Look for specific strings to positively identify the hardware. Here is a nice little challenge for you to up your skills ;)
Instead of a simple if/else block consider using case/esac even if you have just two options right now eg:
case $cpu in
AMD)
echo ">>>>Installing AMD drivers"
sudo dnf -y swap mesa-va-drivers mesa-va-drivers-freeworld
sudo dnf -y swap mesa-vdpau-drivers mesa-vdpau-drivers-freeworld
;;
Intel)
echo ">>>>Installing Intel drivers"
sudo dnf -y swap libva-intel-media-driver intel-media-driver --allowerasing
;;
*)
echo -n "Processor unknown, exit!"
exit 1
;;
esac
If in doubt, we exit. I think this is a desirable approach: if you install the wrong drivers because the user makes a mistake for example, this could mess your graphical configuration and your desktop might not load upon reboot. You'll have to sort out the mess in console.
Logging
By the way, it would be nice to log everything to a file somewhere for postmortem analysis. By running your script in a GNU screen or tmux session, you could do that automatically, and you are indeed installing tmux.
Or do it yourself either on the command line, or by adding instructions to your script. With console redirection in Linux we can copy the output of both stdout and stderr to a log file. If you add this at the top of your script right after the shebang:
LOG_FILE="/root/fedora-install.log"
exec > >(tee -a "$LOG_FILE") 2>&1
This should do the trick nicely :) Source. I am not sure this is the absolute best way though.
shellcheck
shellcheck has some warnings, I recommend you install and use it to improve your scripts and learn a few things in the process.
Update or upgrade
Apparently dnf upgrade is now an alias for dnf update - see: https://github.com/rpm-software-management/dnf5/issues/441
Alternative tools
I understand this is a learning exercise to get familiar with Bash, and I think this is a good use case. But for this kind of stuff I would normally suggest Ansible. It has a lot of modules for common and not so common tasks. If for example you need to patch files reliably, this could get tedious in Bash. So far you've done very straightforward things but your requirements may evolve over time.
Automating the Installation
But in fact you can also take the logic further and automate the OS install itself. For Fedora this is how it can be done: Automating the Installation with Kickstart. I am only mentioning this for completeness - because I don't think you work as sysadmin for a webhost or something like that. But if you are building a lab, or testing a lot of distros then automation is a must.
echo "What is it going to be your hostname?" read -r yourname
Since you are using Bash, then your read has options to prompt and to allow editing:
read -er -p "What is it going to be your hostname? " yourname
Consider putting your functional blocks in functions instead of just indenting them and then just calling each of those if the previous function succeeds, e.g.:
#!/usr/bin/env bash
get_prefs() {
#Setting variables according to user preferences.
local again=1
while (( again == 1 )); do
echo "What is it going to be your hostname?" >&2
read -r yourname
echo "Is this a laptop, (1)YES, (0)NO?" >&2
read -r laptopyn
echo "the CPU is INTEL(1) or AMD(0)" >&2
read -r cpu
echo "If you would like to reenter the information type (1)YES; (2)NO" >&2
read -r again
#Loop back to first echo if again -eq 1
done
}
set_hostname() {
#Setting hostname.
echo ">>>>Setting you up Champ" >&2
echo "$yourname are you sure?" >&2
sleep 2
hostnamectl set-hostname "$yourname"
echo "Your hostname is set to:" >&2
hostname >&2
}
update_os() {
#Updating OS & disable NtwrkMngr-wait
echo ">>>>Starting updates" >&2
sudo dnf -y upgrade
sudo dnf -y upgrade --refresh
sudo dnf -y update
sudo dnf -y groupupdate core
sudo fwupdmgr refresh --force
sudo fwupdmgr update --force
sudo systemctl disable NetworkManager-wait-online.service
echo ">>>>You are up to date 1/10 " >&2
}
get_prefs &&
set_hostname &&
sleep 2 &&
update_os &&
sleep 1
...
Also consider redirecting prompts and info messages to stderr as I'm doing above instead of stdout.
I also moved the applicable sleep statements out of the functions and have them explicit between function calls to aid clarity, etc.
Regarding echo "Is this a laptop, (1)YES, (0)NO?" >&2 - why not accept Y or N instead of 1 or 0? Ditto for echo "the CPU is INTEL(1) or AMD(0)" >&2 - why not I or A?
Explore related questions
See similar questions with these tags.