How to debug this distro dector in bash?

Question

I am trying to make a distribution checker for an installer but am getting errors that I can not figure out. Would someone help with this distribution checker - am I doing the variable declaration wrong here? The script gives me the errors:

read: `NAME=Fedora': not a valid identifier and blank.

Is there any other way I could achieve the same thing without uname? I'll check uname after I have fixed this so that I can have the installer work on mac as well.

#!/usr/bin/bash
#distribution detection system
NAME=$(head -n 1; grep NAME= "/etc/os-release")


#installer promt
read -p "Install tools [y/n]?" insatll_base
case "$insatll_base" in 
y|Y ) echo "installing addtional programs and tools";

#checks for fedora
read $NAME;
if [$NAME="Fedora"]; then 
dnf install cpan -y;
cpan install Menu::Item;
fi;

read $NAME;
if [$NAME="NAME=Redhat"]; then 
dnf install cpan -y;
cpan install Menu::Item;
fi;

Following that, you should *validate* `/etc/os-release` exists before calling `grep` (or more properly `sed`) and setting `NAME` and then *validate* `$NAME` is non-empty. (e.g. `unset NAME; if test -r "/etc/os-release"; then NAME=$(sed -n 's/^NAME=["]$[^"]*$["]/\1/p' /etc/os-release); fi; test -n "$NAME" || { echo "error: no NAME"; exit 1; }`) — David C. Rankin, May 18 '17 at 17:56
it fixed the errors but does not do what i want it to (check your Distribution — matt mods, May 18 '17 at 18:03
okay but i don't know if it exists that what the first check is for is it finds the fist line "NAME=" in the file then it prints the distribution name, then if it fails it would go on to try uname (yet to implement that). and why sed i'm only looking for a pattern called NAME= also wouldn't awk be able to do that as well? but i will give that a go thanks — matt mods, May 18 '17 at 18:07
Try running simply `sed -n 's/^NAME=["]$[^"]*$["]/\1/p' /etc/os-release` to confirm its functionality. Since I `unset NAME` before the `if` block, you simply check `test -n $NAME` after to confirm it is filled, otherwise handle the error. — David C. Rankin, May 18 '17 at 19:28
@DavidC.Rankin so what does the ["]\{^"] .../p mean I apologise if I'm asking a dumb question but I've never used sed before. — matt mods, May 18 '17 at 20:38
`sed -n` suppresses normal output, `["]$[^"] .../p` anything enclosed in `[xy]` is a *character class* meaning "*made up of the specified chars*" (unless preceded by a *circumflex* `'^'` which means "*NOT made up of the specified chars*"). The `p` at the end means `print` (overriding the `-n` for lines that match). So putting it together, the entire expression says find lines beginning with `NAME="` then capture (e.g. between `\(...$`) everything not a `"` up to the next `"` and replace everything (using a *back reference*) with what was captured, and then print. `:)` — David C. Rankin, May 18 '17 at 21:14
@DavidC.Rankin right thanks so there blank char references so that anything between the start point specified and end of the line is put into a variable then that arable is printed — matt mods, May 18 '17 at 21:19
Yep, you got it. It's just a lot cleaner than a double call of `grep` and `head` and gives you more control over removing the actual double-quotes present in `/etc/os-release`. (you can use `egrep` to do it all it one as well). Understand that `grep` is actually `sed`'s little brother (a subset of `sed`). Another way is to simply `source /etc/os-release` and then you can use `$NAME` directly (but I don't like sourcing files with a lot of unneeded information) — David C. Rankin, May 18 '17 at 21:25
@DavidC.Rankin cool thanks I will see if it works in the morning (I love it when my laptop dies) but yeah I'll give that a go. Also could I pipe the fail into an other test say something along the lines of uname -s then importing that into a variable. Because surely I can use try statements as a result of other try statements. — matt mods, May 18 '17 at 21:31
Yes, if the `test -n "$NAME"` fails, you can go to `uname` and also some distros use, e.g. `/etc/arch-release`, `/etc/SuSE-release`, but those are largely historical. — David C. Rankin, May 18 '17 at 21:53
@DavidC.Rankin so would it be worth including or not really anymore! — matt mods, May 18 '17 at 21:55
You can check for the existence of more than `/etc/os-release` and then act on each with something like `array=( /etc/*-release ); if [ "${#a[@]}" -gt '1' ]; then for f in "${a[@]}"; do ...handle each...; done; fi`. You will also need to check that they are not just empty files. `/etc/arch-release` is simply an empty file, so it won't help, but older SuSE distros used `SuSE-release` to actually contain the information and did not have an `os-release`. — David C. Rankin, May 18 '17 at 23:17
It might help to have a look at the `bash` util [`neofetch`](https://github.com/dylanaraps/neofetch), since it detects Linux distros... — agc, May 19 '17 at 05:13
@DavidC.Rankin so also if the OS-release fails then do checks on those files. Except for arch-release. I'll have to think of a way of doing that but if that test fails then I'll do unmade -s to get the kernel name to see if it's Darwin or BSD rather then Linux. — matt mods, May 19 '17 at 05:26
This isn't a proper [mcve], which focuses only on **a single problem**, removing code unrelated to that issue -- there are a whole bunch of separate problems here. Please fix what [shellcheck](https://shellcheck.net/) finds, and ask *tightly-focused* (individual!) questions on any remaining problems, if those remaining problems don't have solutions already (which they almost assuredly will). — Charles Duffy, May 19 '17 at 15:43
@DavidC.Rankin it did not work the variable just prints a empty line and that's it. and now the script gives me a error: line 30: read: `': not a valid identifier — matt mods, May 19 '17 at 16:16
The `...` means you fill in the code to handle each, like `case "$f" in 'os-release' ) printf "found os-release\n";; *SuSE* ) printf "found %s\n" "$f";; esac` — David C. Rankin, May 19 '17 at 16:52
`case` begins the *case statement*, `esac` (case backwards) ends it. Like `if ... fi`. See: [**Using case statements**](http://tldp.org/LDP/Bash-Beginners-Guide/html/sect_07_03.html) — David C. Rankin, May 19 '17 at 17:00
Yeah I know but I didn't know you case inside of an existing case statement unless you mean case it before the promt case — matt mods, May 19 '17 at 17:04
@mattmods Re "*case inside of a case?*": `case` statements can be nested as many as might be needed. It's seldom necessary however. — agc, May 19 '17 at 19:40

score 0 · Answer 1 · edited May 23 '17 at 12:18

0

Some observations:

There's no need to use head or grep for /etc/os-release. Just put a . before it, and all its variables are loaded.
When two then ... fi blocks contain the same code, there's no need for two if statements.
Myrddin Emrys' answer to How do I prompt for Yes/No/Cancel input in a Linux shell script? provides a good minimal example of how to use case.

Here is one way to do it:

#!/usr/bin/bash
#distribution detection system
if [ -s /etc/os-release ] ; then
    . /etc/os-release

    #installer prompt
    read -p "Install tools [y/n]?" yn
    if [ "${yn,,}" = y ] ; then
         echo "Installing additional programs and tools...";

         #checks for Fedora or Redhat
         case "$NAME" in
              Fedora|Redhat)  dnf install cpan -y
                              cpan install Menu::Item ;;
         esac
    fi
fi

edited May 23 '17 at 12:18

Community

1
1

answered May 19 '17 at 19:30

agc

7,973
2
29
50

Okay so would I just add more case statements or would I do it in the same case? – matt mods May 19 '17 at 20:06
@mattmods, Please, use the **correct** terms. The *correct* term for `Fedora|Redhat)` is a "pattern", and what comes after, from `dnf` to `::Item` is a "command list". And *yes* you can add more patterns and lists. These must go after the previous `;;` and before the `esac`. – agc May 19 '17 at 20:36
okay thank you. and for the no option would you just add an other if statement with the y changed to n. also can you add a if statement that runs if the first one fails so that it finds arch-release and Suse-release, then put that into a varable so that it can run on suse and arch? – matt mods May 19 '17 at 20:46
@mattmods, No you wouldn't do it that way... an `elseif` (for the "*n*") and an additional `case` *pattern* would be much better. The virtue of having a `case` statement is to simplify such nests of `if`s. – agc May 19 '17 at 22:43
but I still want it to install the programs if you enter n just not install all additional stuff. Or would it not matter about the pattern? – matt mods May 20 '17 at 07:16
@mattmods, Suppose there's only *two* possibilities: 'true' and 'false'. And 'true' runs 'ABC' then runs 'DEF', but 'false' runs only 'DEF'. All that's needed to handle *both* cases is this code: `if true ; then ABC ; fi ; DEF`, which can be simplified to `true && ABC ; DEF`. For only *two* simple possibilities a `case` statement is a bit of a waste. – agc May 20 '17 at 07:57
so your saying just use && instead of a separate case? – matt mods May 20 '17 at 17:35
@mattmods, Sometimes... compare `read -p "Enter [y/n]?" yn ; case $yn in y) echo "yes" ;; n) echo "no" ;; *) echo "that's not y/n" ;; esac` with `read -p "Enter [y/n]?" yn ; if [ "$yn" = y ] ; then echo "yes" ; elif [ "$yn" = n ] ; echo "no" ; else echo "that's not y/n" ; fi`. Both do the same thing, but the `case` is shorter. Such code assumes _three_ possible answers, {*yes,no,neither*}, but sometimes it's just {*yes,no*}, and sometimes there's no additional action for *no* -- in *those* cases, `read -p "Enter [y/n]?" yn ; [ "$yn" = y ] && echo "yes"` is enough. – agc May 22 '17 at 13:02

How to debug this distro dector in bash?

1 Answers1