My first shell script. Input appreciated.

11-Inch-Subpoena · 2024-09-04 14:54:20

I wrote my first shell script, and I was looking for some input in how to make it better or to trim the fat from it. It's intended to be run after the base system is installed:

#!/bin/bash

echo "Now configuring your Arch system."
ln -sf /usr/share/zoneinfo/America/Chicago /etc/localtime
hwclock
pacman -Syu intel-ucode sof-firmware networkmanager wpa_supplicant nano man-db man-pages texinfo bluez bluez-utils reflector sudo alsa alsa-utils pulseaudio pulseaudio-bluetooth pulseaudio-alsa pulseaudio-equalizer git rust
systemctl enable --now reflector alsa-restore alsa-state bluetooth
amixer sset Master unmute
amixer sset Speaker unmute
amixer sset Headphone unmute
wifi=$(lspci | grep -o "Broadcom")
	if [ "$wifi" == "Broadcom" ];
		then
		pacman -Syu broadcom-wl-dkms linux-headers
		echo "Broadcom Wireless drivers installed."
	else
		echo "No Broadcom drivers needed"
	fi
echo "Which Desktop Environment would you like to install?"
select opt in Xfce MATE Cinnamon Budgie KDE none; do
	case $opt in
		Xfce)
			pacman -Syu xfce4 xfce4-goodies network-manager-applet xfce4-pulseaudio-plugin lightdm lightdm-slick-greeter
			systemctl enable lightdm
			break
			;;
		MATE)
			pacman -Syu mate mate-extra network-manager-applet mate-applet-dock blueman paprefs lightdm lightdm-slick-greeter
			systemctl enable lightdm
			break
			;;
		Cinnamon)
			pacman -Syu cinnamon network-manager-applet gnome-keyring blueberry paprefs lightdm lightdm-slick-greeter
			systemctl enable lightdm
			break
			;;
		Budgie)
			pacman -Syu budgie budgie-extras network-manager-applet budgie-desktop-view budgie-backgrounds materia-gtk-theme arc-gtk-theme papirus-icon-theme plasma-pa paprefs lightdm lightdm-slick-greeter
			systemctl enable lightdm
			break
			;;
		KDE)
			pacman -Syu plasma kde-applications sddm sddm-kcm
			systemctl enable sdddm
			break
			;;
		none)
			echo "Skipping desktop environment installation."
			break
			;;
		*)
			echo "Invalid option $REPLY"
			;;
	esac
done
useradd -m -G wheel -s bash <username>
passwd <username>
echo "visudo will open soon. Please add the following line if it does not already exist (no quotes): '%wheel      ALL=(ALL:ALL) ALL' "
EDITOR=nano visudo
systemctl enable lightdm NetworkManager
su <username> -c git clone https://aur.archlinux.org/paru.git && cd paru && makepkg -i && head -n -1 ~/.bashrc > temp && mv temp ~/.bashrc && echo "alias yay='\paru -c && \paru -Syu && \paru --devel && \paru -S --needed'" >> ~/.bashrc && echo "alias yeet='paru -Rns'" >> ~/.bashrc && echo "PS1='[\[\e[38;5;51;1;2m\]\u\[\e[0;1m\]@\[\e[38;5;51;2m\]\h\[\e[0m\]\w]\$ '" >> ~/.bashrc && cd ~ && echo "#!/bin/bash" > dm.sh && echo -en "\n" >> dm.sh && echo "dm=$(ps auxf | awk '{print $11}' | \grep -E '(lightdm|sddm)$')" >> dm.sh && echo "	if [ $dm == lightdm ];" >> dm.sh && echo "then" >> dm.sh && echo "paru -S mkinitcpio-firmware arch-update lightdm-settings" >> dm.sh && echo "else" >> dm.sh && echo "paru -S mkinitcpio-firmware arch-update" >> dm.sh && echo "fi" >> dm.sh && chmod 0775 dm.sh && ./dm.sh && rm dm.sh && rm -R ./paru
pacman -Rs rust
exit

Pastebin here

Since it's intended to be run first thing after the base system is installed, I figured that I had to use it to write and run a second script to determine if I needed to install lightdm-settings from the AUR. Any feedback would be greatly appreciated. Thank you.

Last edited by 11-Inch-Subpoena (2024-09-04 14:59:19)

Awebb · 2024-09-04 17:37:19

1. Pet peeve: It's bash, familiarize yourself with [[ ]] over [ ]. Less portable but with less surprises regarding what's in between.

2. Use $(expression), you're already using it to populate $wifi, but you're not using $wifi anywhere else.

if [ $(lspci | grep -o "Broadcom") == "Broadcom" ];

3. Maybe I'm overlooking something, but what is the break doing in the case statements?

4. Why have an explicit none instead of just letting *) handle the "none of this" case, if you're not somehow looping back to get the right answer?

5. You can let the user do the visudo part, but have you heard about sed -i/--in-place?

6. You enable lightdm near the end, irregardless of whether you've enabled sddm for KDE or don't have sddm at all by picking "none".

7. Are you a 100% certain that paru is a drop-in replacement for yay?

8. For that su <username> line, please look up what a heredoc is and how it works or at least how newlines in printf work instead of echo, because that one line is a script of its own and impossible to getting a stroke and all because you've tried to change the user mid script. A function, perhaps?

9. Yeah, fuck rust.

10. What's the exit doing at the end? Impatient?

Bonus Question: How often do you install Arch? Why do you need this?

V1del · 2024-09-04 18:22:37

Also, are you sure you want to unconditionally install mkinitcpio-firmware despite it being entirely unnecessary for 99% of people? Do you know why mkinitcpio prints the warning if those firmwares aren't present?

Regarding the visudo part, your user is not going to see that line before being dropped into visudo, consider adding either a confirmation keypress or some sleep to the visudo invocation.

Last edited by V1del (2024-09-04 18:29:48)

Trilby · 2024-09-04 18:39:59

Awebb wrote:

if [ $(lspci | grep -o "Broadcom") == "Broadcom" ];

Or better:

 if $(lspci | grep -q Broadcom);

However, I'd question the logic of this approach for a few reasons.

1) Broadcom makes hardware other than NICs, so you could have false positives from just looking for the brand name in the full lspci output.

2) What if there are multiple NICs and only one crappy broadcom one that wouldn't be used.

3) Perhaps the biggest concern, why would you assume all Broadcom chips need the wl driver? Wl kinda sucks; but in some rare cases it is the only remaining option. But most broadcom chips would use another more stable driver (brcmfmac, b43, etc).

Last edited by Trilby (2024-09-04 18:44:21)

11-Inch-Subpoena · 2024-09-04 19:16:39

Awebb wrote:

1. Pet peeve: It's bash, familiarize yourself with [[ ]] over [ ]. Less portable but with less surprises regarding what's in between.
2. Use $(expression), you're already using it to populate $wifi, but you're not using $wifi anywhere else.
if [ $(lspci | grep -o "Broadcom") == "Broadcom" ];
3. Maybe I'm overlooking something, but what is the break doing in the case statements?
4. Why have an explicit none instead of just letting *) handle the "none of this" case, if you're not somehow looping back to get the right answer?
5. You can let the user do the visudo part, but have you heard about sed -i/--in-place?
6. You enable lightdm near the end, irregardless of whether you've enabled sddm for KDE or don't have sddm at all by picking "none".
7. Are you a 100% certain that paru is a drop-in replacement for yay?
8. For that su <username> line, please look up what a heredoc is and how it works or at least how newlines in printf work instead of echo, because that one line is a script of its own and impossible to getting a stroke and all because you've tried to change the user mid script. A function, perhaps?
9. Yeah, fuck rust.
10. What's the exit doing at the end? Impatient?
Bonus Question: How often do you install Arch? Why do you need this?

1.) I'll do that. I don't see any situations where it matters here, but it's good to get in the habit now.

2.) Done.

3.) I can't remember why I put it there. It's gone now.

5.) The user (me, in this case) is still responsible for actually making the visudo entry. I just figured I'd make opening it part of the script.

6.) Yeah, that was from a previous version that didn't include KDE Plasma and SDDM. When I added KDE Plasma as an option, I added the commands to enable the relevant DM after the desktop environment was installed. I just forgot to delete LightDM from that line. I've deleted it now.

7.) Paru is not a 100% drop in replacement, but I wanted my aliases to still be "yay" and "yeet". I think I've gotten the commands and options correct in my aliases.

8.) I have made some edits:

systemctl enable NetworkManager
 tee <<-EOF > /home/<username>/pt2.sh
	git clone https://aur.archlinux.org/paru.git
	cd paru
	makepkg -i
	head -n -1 ~/.bashrc > temp && mv temp ~/.bashrc
	echo "alias yay='\paru -c && \paru -Syu && \paru --devel && \paru -S --needed'" >> ~/.bashrc
	echo "alias yeet='paru -Rns'" >> ~/.bashrc
	echo "PS1='[\[\e[38;5;51;1;2m\]\u\[\e[0;1m\]@\[\e[38;5;51;2m\]\h\[\e[0m\]\w]\$ '" >> ~/.bashrc
	cd ~
	if [ (ps auxf | awk '{print $11}' | \grep -E '(lightdm|sddm) == lightdm ];
		then
		paru -S mkinitcpio-firmware arch-update lightdm-settings 
	else
		paru -S mkinitcpio-firmware arch-update
	fi
	EOF
chmod 0777 /home/<username>/pt2.sh
su <username> -c ./pt2.sh && rm -R ./paru
rm /home/<username>/pt2.sh
pacman -Rs rust

The issue is that I can't run makepkg as root as far as I know, and I'm doing all of this immediately after installing the base system. Using su to switch to my username to install paru and some other packages from the AUR is required.

9.) I don't like keeping build dependencies, no matter what language they are. Some of my PCs are really crappy. One has an eMMC with 64GB.

10.) I thought that it would exit the chroot environment for me. I guess not.

Bonus Answer: It happens from time to time. I have nine PCs, eight of which are actually capable of running Arch. Three of those are running Ubuntu Server right now, but that could change in the future. I also inherit old PCs from time to time because I enjoy working on them. Aside from that, I was looking for something a little complex to script, and I thought of this. I really need this because of the challenge of writing it and the things I can learn about scripting along the way.

V1del wrote:

Also, are you sure you want to unconditionally install mkinitcpio-firmware despite it being entirely unnecessary for 99% of people? Do you know why mkinitcpio prints the warning if those firmwares aren't present?
Regarding the visudo part, your user is not going to see that line before being dropped into visudo, consider adding either a confirmation keypress or some sleep to the visudo invocation.

Good points. I'm deleting mkinitcpio-firmware and I'll add a confirmation keypress.

Trilby wrote:

Awebb wrote:
if [ $(lspci | grep -o "Broadcom") == "Broadcom" ];
Or better:
 if $(lspci | grep -q Broadcom);
However, I'd question the logic of this approach for a few reasons.
1) Broadcom makes hardware other than NICs, so you could have false positives from just looking for the brand name in the full lspci output.
2) What if there are multiple NICs and only one crappy broadcom one that wouldn't be used.
3) Perhaps the biggest concern, why would you assume all Broadcom chips need the wl driver? Wl kinda sucks; but in some rare cases it is the only remaining option. But most broadcom chips would use another more stable driver (brcmfmac, b43, etc).

That was written with my Macs in mind, in case I ever need to do a clean install. They're the only computers I own with Broadcom hardware that shows up in lspci, and the wl firmware (specifically the wl-dkms firmware) is the only one that works; I tried literally everything else, including the firmware in the AUR, first. One of the Macs does have an ethernet port, but I want the wifi to be available, too.

lsteeger · 2024-09-04 19:40:25

Just an additional thought.

The shellcheck package/tool will analyze a shell script for errors and possible gotchas.

Its output can be overwhelming, but it very seldom missing anything.

YMMV.

Awebb · 2024-09-04 19:40:48

if [ (ps auxf | awk '{print $11}' | \grep -E '(lightdm|sddm) == lightdm ];

1. Are you sure that line is complete?
2. Are you sure either lightdm or sddm will run at that point?
3. Why are you escaping grep's g (and paru's p elsewhere)?

lsteeger wrote:

YMMV

I've seen code double its size, so shellcheck shuts up about certain things :-)

Last edited by Awebb (2024-09-04 19:42:32)

Trilby · 2024-09-04 20:18:52

lsteeger wrote:

... but it very seldom missing anything.

That's not saying much. It doesn't miss anything that it has a propper search pattern for. But it misses everything it doesn't.

That's not a criticism of the tool. Shellcheck is great. But there's a reason it rhymes with spellcheck. A decent spell check doesn't miss anything ... except for errors in grammar, sentence structure, paragraph structure, logic and flow of the document, or even semantic sensibility of the strings of words. But people get that a spell checker checks only spelling. Similarly, shellcheck checks only <no-such-term-exists> ... and it never misses one of those.

TLDR: you can't miss if you define your target as what you hit.

Last edited by Trilby (2024-09-04 20:21:19)

11-Inch-Subpoena · 2024-09-04 20:29:51

lsteeger wrote:

Just an additional thought.
The shellcheck package/tool will analyze a shell script for errors and possible gotchas.
Its output can be overwhelming, but it very seldom missing anything.
YMMV.

I'll have to give that a download and check it out. Thanks.

Awebb wrote:

if [ (ps auxf | awk '{print $11}' | \grep -E '(lightdm|sddm) == lightdm ];
1. Are you sure that line is complete?
2. Are you sure either lightdm or sddm will run at that point?
3. Why are you escaping grep's g (and paru's p elsewhere)?

1. Shit. I need another closing parenthesis.

2. The relevant service would have been enabled (but not started) at an earlier step.

3. The examples I used as templates had those characters escaped. I'll fix it.

mountaintrek · 2024-09-05 01:57:50

Just some thoughts...

1. Check out printf.

printf -- '%(%F %T)T - Now configuring your Arch system\n'

2. Check return codes

pacman...
rc=$?

if (( rc != 0 )); then
exit 1
fi

3. Quote command subsitution

wifi="$(...)"

4. Others have mentioned [[]] vs [].

5. Single quote literals or strings with no parameter expansion.

6. An alternative using process substitution:

while read -re line; do
if [[ "${line}" == *Broadcom* ]]; then
: do something
fi
done < <( lspci )

7. git clone line is kind of long :-), consider using line continuation "\"

su <username> -c git clone https://aur.archlinux.org/paru.git \
&& cd paru

HHHmmm, that line. What about reversing the logic and put each on a separate line

cd paru || exit 1
mkepkg -i || exit 1
...

8. It may not be safe to assume commands that you will be using are on your
system. Belts and suspenders, check before using.

if ! type -p git; then printf -- 'Error\n'; exit 1; fi

The above could be turned into a function, something like:

function pgm_exist() {
local -- pgm="${1}"
local -- rc=0
if ! type -p "${pgm}" >/dev/null; then printf -- 'Error\n'; rc=1; fi
return $rc
}

declare -a pgms=(git lspci amixer)

for p in "${pgms[@]}"; do
pgm_exist "${p}"
done

Same goes for directories and files. If you rely on them, make sure they exist.

Additional References:

google shell style guide
https://google.github.io/styleguide/shellguide.html

The Linux Command Line
https://www.linuxcommand.org/tlcl.php

Unofficial Bash FAQ
http://mywiki.wooledge.org/BashFAQ

11-Inch-Subpoena · 2024-09-05 15:04:52

mountaintrek wrote:

Just some thoughts...
1. Check out printf.
printf -- '%(%F %T)T - Now configuring your Arch system\n'

2. Check return codes
pacman...
rc=$?
if (( rc != 0 )); then
exit 1
fi
3. Quote command subsitution
wifi="$(...)"
4. Others have mentioned [[]] vs [].
5. Single quote literals or strings with no parameter expansion.
6. An alternative using process substitution:
while read -re line; do
if [[ "${line}" == *Broadcom* ]]; then
: do something
fi
done < <( lspci )
7. git clone line is kind of long :-), consider using line continuation "\"
su <username> -c git clone https://aur.archlinux.org/paru.git \
&& cd paru
HHHmmm, that line. What about reversing the logic and put each on a separate line
cd paru || exit 1
mkepkg -i || exit 1
...
8. It may not be safe to assume commands that you will be using are on your
system. Belts and suspenders, check before using.
if ! type -p git; then printf -- 'Error\n'; exit 1; fi
The above could be turned into a function, something like:
function pgm_exist() {
local -- pgm="${1}"
local -- rc=0
if ! type -p "${pgm}" >/dev/null; then printf -- 'Error\n'; rc=1; fi
return $rc
}
declare -a pgms=(git lspci amixer)
for p in "${pgms[@]}"; do
pgm_exist "${p}"
done
Same goes for directories and files. If you rely on them, make sure they exist.
Additional References:
google shell style guide
https://google.github.io/styleguide/shellguide.html
The Linux Command Line
https://www.linuxcommand.org/tlcl.php
Unofficial Bash FAQ
http://mywiki.wooledge.org/BashFAQ

I'm assuming that commands, files, and directories exist because the script installs/creates them as it executes. I should implement the return code check, though. I'll do that.

I removed the

$wifi

variable, since it's a single use variable. I'm now using

if [[ "$(lspci | grep -o "Broadcom")" == "Broadcom" ]];

I'm going to eliminate the double quotes where possible. Just not right now, because I'm in the middle of a shitload of work.

I've implemented double bracketing.

I did change some other things. In order to check for lightdm, I decided to use

if [[ "$(pacman -Q | grep -o 'lightdm')" == 'lightdm' ]];

, I don't know why I didn't realize I could do that earlier. I also improved my use of heredoc:

systemctl enable NetworkManager
su <username> -c cat <<-EOF
	$(git clone https://aur.archlinux.org/paru.git)
	$(cd paru)
	$(makepkg -i)
	$(head -n -1 ~/.bashrc > temp && mv temp ~/.bashrc)
	$(echo "alias yay='paru -c && paru -Syu && paru --devel && paru -S --needed'" >> ~/.bashrc)
	$(echo "alias yeet='paru -Rns'" >> ~/.bashrc)
	$(echo "PS1='[\[\e[38;5;51;1;2m\]\u\[\e[0;1m\]@\[\e[38;5;51;2m\]\h\[\e[0m\]\w]\$ '" >> ~/.bashrc)
	$(cd ~)
	$(if [[ "$(pacman -Q | grep -o 'lightdm')" == 'lightdm' ]];
	then
	paru -S arch-update lightdm-settings 
	else
	paru -S arch-update
	fi)
	$(rm -R ./paru)
EOF
pacman -Rs rust

As for your other recommendations, I'll try them out. I'll also check out the resources you've linked. Thank you.

Last edited by 11-Inch-Subpoena (2024-09-06 09:49:08)

Arch Linux

#1 2024-09-04 14:54:20

My first shell script. Input appreciated.

#2 2024-09-04 17:37:19

Re: My first shell script. Input appreciated.

#3 2024-09-04 18:22:37

Re: My first shell script. Input appreciated.

#4 2024-09-04 18:39:59

Re: My first shell script. Input appreciated.

#5 2024-09-04 19:16:39

Re: My first shell script. Input appreciated.

#6 2024-09-04 19:40:25

Re: My first shell script. Input appreciated.

#7 2024-09-04 19:40:48

Re: My first shell script. Input appreciated.

#8 2024-09-04 20:18:52

Re: My first shell script. Input appreciated.

#9 2024-09-04 20:29:51

Re: My first shell script. Input appreciated.

#10 2024-09-05 01:57:50

Re: My first shell script. Input appreciated.

#11 2024-09-05 15:04:52

Re: My first shell script. Input appreciated.

Board footer