LinuxQuestions.org - [SOLVED] Need Someone to Review my Shell Script

Page 1 of 2

Show 50 post(s) from this thread on one page

- Programming (https://www.linuxquestions.org/questions/programming-9/)

- - Need Someone to Review my Shell Script (https://www.linuxquestions.org/questions/programming-9/need-someone-to-review-my-shell-script-4175655738/)

anon033

06-14-2019 09:14 PM

Need Someone to Review my Shell Script

I have been learning shell scripting for the past few days and decided to write my first script. Basically this script just creates all my directories I need in my home directory and installs my dotfiles. I was wondering if anyone could take a look at my shell script and give me some feedback, what can be done in a more minimal fasion and just if there is anyway I can improve my script. Thank you in advance, here it is:

Code:

#/bin/sh



# create directories

mkdir -p {$HOME/docs,$HOME/docs/notes,$HOME/docs/pdfs,$HOME/docs/dev,$HOME/dl,$HOME/vids,$HOME/music,$HOME/pics,$HOME/.config,$HOME/.builds}



# move dotfiles into docs/dev and move back into dotfiles

cd $HOME && mv dotfiles docs/dev && cd docs/dev/dotfiles



# install config

cd config



cp -r dunst $HOME/.config/dunst

cp -r irssi $HOME/.config/irssi

cp -r mpd $HOME/.config/mpd

cp -r mpv $HOME/.config/mpv

cp -r nvim $HOME/.config/nvim

cp -r zathura $HOME/.config/zathura



# install home

cd ../home



cp -r .Xresources $HOME/.Xresources

cp -r .ksh_history $HOME/.ksh_history

cp -r .kshrc $HOME/.kshrc

cp -r .mpd $HOME/.mpd

cp -r .ncmpcpp $HOME/.ncmpcpp

cp -r .profile $HOME/.profile

cp -r .scripts $HOME/.scripts

cp -r .tmux.conf $HOME/.tmux.conf

cp -r .w3m $HOME/.w3m

cp -r .xsession $HOME/.xsession

doas cp -r fonts/* /usr/local/fonts/

doas cp -r systemerror.png /usr/local/share/backgrounds/



echo "dotfiles installed"

I removed my edits as they were from me not paying attention and would just be confusing.

scasey

06-14-2019 10:17 PM

Personally, I’d use absolute paths in that script, mostly for clarity....I’ll concede that’s a style preference. The style you’re using with cd commands is not “wrong”, I just find it hard to follow.

Using $HOME means the script expects to be run by the user being configured. Yes? I’d put a comment in noting that and insure the script permissions and all source directories permissions are as they need to be.

Bottom line is: Does it work? Does it do what you expect?

More minimal? adduser can be configured (defaults?) to copy defaults from /etc/skel/*. If that’s set up as you want, the script is not needed at all ;).

anon033

06-14-2019 10:28 PM

Quote:

Originally Posted by scasey (Post 6005459)

There is only one thing that isn't working. So I am testing this on a Linux Mint machine and I am looking to replace all the cp -rs for config to just be

Code:

cp -r config/. $HOME/.config

when I run that manually it works, but when I run it in a script it says

Code:

cp: cannot stat 'config/.': No such file or directory

I am confused why it keeps saying that only when in a script

scasey

06-14-2019 11:19 PM

Quote:

Originally Posted by FOSSilized_Daemon (Post 6005462)

There is only one thing that isn't working. So I am testing this on a Linux Mint machine and I am looking to replace all the cp -rs for config to just be

Code:

cp -r config/. $HOME/.config

when I run that manually it works, but when I run it in a script it says

Code:

cp: cannot stat 'config/.': No such file or directory

I am confused why it keeps saying that only when in a script

That command says “copy the file named . in the config directory ” ...there is no such file
Do you mean

Code:

cp -r config/.*

.?? perhaps

Or, to my original point...is there a config directory in the in the directory the script is in when the command is executed?
Or my second original point, is the directory being copied readable by the user running the script?

Use set -x to see what’s happening.

ntubski

06-15-2019 06:53 AM

Quote:

Originally Posted by FOSSilized_Daemon (Post 6005441)

Code:

# move dotfiles into docs/dev and move back into dotfiles

cd $HOME && mv dotfiles docs/dev && cd docs/dev/dotfiles

This will fail to cd back if the mv fails for any reason. I suggest a subshell instead:

Code:

(cd $HOME && mv dotfiles docs/dev)

Quote:

Originally Posted by scasey (Post 6005471)

That command says “copy the file named . in the config directory ” ...there is no such file

Every directory contains a hard link to itself named ".", so I think this should work.

scasey

06-15-2019 09:09 AM

Quote:

Originally Posted by ntubski (Post 6005568)

Every directory contains a hard link to itself named ".", so I think this should work.

Oh! Excellent point. Then I suspect the issue is that the source dirs are not readable by the user running the script.
OR there is no directory in the current directory named config... ;)

individual

06-15-2019 09:43 AM

Here is a simplified version of your script, with comments.

Code:

# Just specify $HOME once.

mkdir -p "$HOME"/{docs,docs/notes,docs/pdfs,docs/dev,dl,vids,music,pics,.config,.builds}



# Move dotfiles into the $HOME/docs/dev directory, without cd'ing into $HOME

# EDIT: This line seems unnecessary since you could just stay in your original dotfiles directory.

# mv dotfiles "$HOME/docs/dev"



# Find is a really useful command that everyone should be at least somewhat familar with.

# You have to specify the regex type as egrep or awk if you want to alternate patterns '|'.

# EDIT2: If you aren't in the dotfiles directory you'll need to specify it.

find dotfiles -regextype egrep -regex '.*(dunst|irssi|mpd|mpv|nvim|zathura)' -exec cp -r \{} "$HOME/.config" \;



# This is the same as above except for the destination directory, which is $HOME.

find dotfiles -regextype egrep -regex '.*(.Xresources|.ksh_history|.kshrc|.mpd|.ncmpcpp|.profile|.scripts|.tmux.conf|.w3m|.xsession)' -exec cp -r \{} "$HOME" \;

BW-userx

06-15-2019 10:41 AM

I agree you should to absolute paths and or reserved words HOME USER etc... as needed.
dealing with dot files

Code:

#!/bin/bash



#create a dir called temp add this file and chmod +x filename

#then run it



#declare your dirs you will be working with



dofiles=$HOME/temp/dotfiles

CopyDOTSto=$HOME/temp/CopyDotsHere



#dynamically create dirs to be used

mkdir -pv $dofiles

mkdir -pv $CopyDOTSto



#create some test dot files

touch $dofiles/.{a..z}.txt



# for considering dot files (turn on dot files)

#bash build in

shopt -s dotglob 



#change dir into where dotfiles are located

cd $dofiles



#echo location you're at now just for piece of mind

echo $(pwd)



#now that you're in the dot file directory

#copy all of the dot files into where I want them

cp -rv * $CopyDOTSto



#show results

ls -la $CopyDOTSto

when in doubt double quote "$var"

average_user

06-16-2019 06:04 AM

I'd suggest using a lint-like tool for shell scripts called Shellcheck available at https://www.shellcheck.net and its command line interface https://github.com/koalaman/shellcheck.

Of course it's just the first step in reviewing the code and perhaps the only one that can be done automatically but at least you can get rid of the most visible and obvious bugs at the very early stage.

wpeckham

06-16-2019 06:39 AM

I noticed that your shebang "#!" line specified the sh shell. If you are using a different shell for your session (bash, perhaps) the behavior may differ due to the existence or behavior of different shell internals. Even if it is the same shell but called under a different name, named as /bin/sh activates the posix behavior for most shells, and that CAN make a difference. Here it should not, but it is a factor you should always consider.

orbea

06-16-2019 08:17 AM

Quote:

Originally Posted by wpeckham (Post 6005849)

For example the brace expansions are a bash feature and not portable.

In bash.

Code:

$ echo {foo,bar}

foo bar

And with dash.

Code:

$ echo {foo,bar}

{foo,bar}

anon033

06-16-2019 01:29 PM

Quote:

Originally Posted by orbea (Post 6005881)

For example the brace expansions are a bash feature and not portable.

In bash.

Code:

$ echo {foo,bar}

foo bar

And with dash.

Code:

$ echo {foo,bar}

{foo,bar}

I did not know that was a bashism, see this is why I hate so many beginner books so much. Don't tell me that is a Linux thing if it is in fact a bashism. Thank you for the heads up.

anon033

06-16-2019 01:42 PM

Quote:

Originally Posted by orbea (Post 6005881)

For example the brace expansions are a bash feature and not portable.

In bash.

Code:

$ echo {foo,bar}

foo bar

And with dash.

Code:

$ echo {foo,bar}

{foo,bar}

Do you by chance know of a POSIX way to do something similar to that or do I have to just do them one at a time?

edit:

mkdir -p $HOME/docs $HOME/docs/notes $HOME/docs/pdfs $HOME/docs/dev $HOME/dl $HOME/vids $HOME/music $HOME/pics $HOME/.config $HOME/.builds

Mechanikx

06-16-2019 02:32 PM

Quote:

Originally Posted by FOSSilized_Daemon (Post 6005999)

I did not know that was a bashism, see this is why I hate so many beginner books so much. Don't tell me that is a Linux thing if it is in fact a bashism. Thank you for the heads up.

If you're interested you should check out Unix Shell Programming 4th edition (Shell Programming in Unix, Linux and OS X) by Kochan and Wood:

https://www.amazon.ca/Shell-Programm...gateway&sr=8-1

Its focus is the posix shell. And near the end it discusses some features unique to common shells. From what I remember all the examples, or most, are tested in multiple different shells, bash, ksh, etc.

anon033

06-16-2019 02:34 PM

Quote:

Originally Posted by Mechanikx (Post 6006028)

Thank you so much, I will definitely get this!

All times are GMT -5. The time now is 12:57 AM.

Page 1 of 2

Show 50 post(s) from this thread on one page