Does an echo work in an if?

Question

Consider the following function:

function testForBinary {
    someBin=$(command -v binary)
        # Test if binary is installed
        if [[ -n $someBin ]]; then
            installSuccess="Success"
            echo ${installSuccess}
            return
        else
            # This should never be reached
            return 1
        fi
}

taken from (Context):

function installAndTestForDialog() {
# dialog allows me to create a pretty installer
# It is a required dependency since XXXXX
# Name Removed as I'm attempting modularization
# of a script from Github that's one huge Script
See https://unix.stackexchange.com/questions/85249/why-not-use-which-what-to-use-then
local dialogBin
local updated=false
local dialogInstallSuccess

dialogBin=$(command -v dialog)
if [[ -z $dialogBin ]]; then
    printf &quot;Installing dialog... &quot;
    if [[ $updated -eq 0 ]]; then
        apt-get update &gt; /dev/null 2&gt;&amp;1
        updated=true
    fi
    # Debian Based OS
    apt-get -y install dialog &gt; /dev/null 2&gt;&amp;1

    # Test if dialog is installed to verify installation
    # above was successful.
    dialogBin=$(command -v dialog)
    if [[ -n $dialogBin ]]; then
        # I cannot get this to echo success
        # if I echo from here
        dialogInstallSuccess=&quot;Success&quot;
        # Moving it here doesn't help either  Why??
        echo ${dialogInstallSuccess}
        return
    else
        # This should never be reached
        return 1
    fi
fi    

}

I'm attempting to treat installSuccess as a boolean, but what am I doing wrong. If I write the function as above, and then add:

isInstalled=$(testForBinary)
echo "$isInstalled"

isInstalled returns a blank line. I know this isn't true because when I run command -v binary outside the function, the path to binary results.

Output (Context):

Function and Variable Output Test
=====================
# These are other tests
# I'm performing, but note
# the blank line, which should
# print Success 
2.9-MODULAR
192.168.1.227
false
false
(blank line)

Don't exit from a function unless you want the script that's running it to exit. Use return instead. see Difference between return and exit in Bash functions — cas, Jul 07 '21 at 07:32
Re: Extra fi, I'll correct that. I copied this from a JetBrainz based IDE. Re: Shellcheck. IntelliJ uses shellcheck on conjunction with BASH Support Pro. Scripting is not my forte — eyoung100, Jul 07 '21 at 21:23
@StephenKitt binary is any binary executable. I replaced it to make it generic. I'm actually testing for dialog I can provide the output I get for context, along with the entire function — eyoung100, Jul 07 '21 at 21:25
Why should that be echoed? It's after a return, the execution has already moved out of the function. — muru, Jul 07 '21 at 22:46
... and dialogInstallSuccess is never set because dialogBin is still empty and you end up in the else block. — Freddy, Jul 07 '21 at 22:53
@Freddy Unless I've missed something dialogBin is never empty. I do see now that I need to test it the second time, which I have now fixed. The output in question is still blank. The solution is probably staring me in the face, but I can't see it — eyoung100, Jul 07 '21 at 23:21
@ibuprofen checking path both inside and outside of JetBrainz yields: PATH=/home/username/bin:/home/username/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/opt/bin:/usr/lib/llvm/11/bin:/usr/lib/llvm/12/bin — eyoung100, Jul 08 '21 at 03:26
I figured it out!! The If test for -z $dialogBiin can't be nested with the -n $dialogBin test. Removing the nesting creates the desired output — eyoung100, Jul 08 '21 at 04:44
You don't need to capture the output of command -v into a variable. command returns an exit code of 0 on success (i.e. dialog is in your path or defined as a function or alias), non-zero on failure. So all you need is if command -v dialog; then ... ; else ... ; fi — cas, Jul 08 '21 at 04:51
It’s really late…but I don’t think you’re escaping $dialogBin correctly. It may be expanding and thus producing a false for non-zero length test. Use ‘if [[ -n “$dialogBin” ]]’ instead. — bretonics, Jul 08 '21 at 04:54
Also, you're massively over-complicating this. Just run apt update and apt install dialog if dialog is not already installed, and be done with it. Or just apt update and apt install dialog without bothering to check if it's already installed - the worst that will happen is that a newer version of dialog (and its dependencies) might be installed. — cas, Jul 08 '21 at 04:57
@cas I'm trying to keep the original author's code as intact as possible. I'm sure there are quicker ways to achieve the desired goal... — eyoung100, Jul 08 '21 at 05:04
why? if the original code is garbage (and it is), replace it. — cas, Jul 08 '21 at 05:07

cas · Answer 1 · 2021-07-08T07:35:59.593

5

This doesn't need to be anywhere near as complicated and fragile as you're making it.

installAndTestForDialog() {
  if command -v dialog &> /dev/null; then
    return 0
  else
    apt-get update &> /dev/null
    apt-get -y install dialog &> /dev/null
  fi
}

The function will return 0 if dialog is already installed. Otherwise it will attempt to install it and will return the exit code of the apt-get install command.

As @muru points out in a comment, this can be condensed to just:

if ! command -v dialog; then
    apt-get update
    apt-get install -y dialog
fi > /dev/null 2>&1

Or just try to install it without bothering to check if it's already installed:

{ apt-get update && apt-get -y --no-upgrade install dialog ;} >/dev/null 2>&1

The --no-upgrade option prevents dialog from being upgraded if it's already installed.

edited Jul 08 '21 at 07:35

answered Jul 08 '21 at 05:03

cas

78,579

If the shell running this isn't bash or some other shell that supports &>, you will need to use >/dev/null 2>&1 instead of &> /dev/null. – cas Jul 08 '21 at 05:10
I'd probably just condense it to if ! command -v dialog; then apt-get update; apt-get install -y dialog; fi > /dev/null 2>&1. The return code of the if block as a whole would be 0 if command -v dialog didn't fail. – muru Jul 08 '21 at 05:20
@muru, yeah, but i wanted it to show handling both cases - even if the success case is basically a no-op. – cas Jul 08 '21 at 05:22

score 0 · Accepted Answer · answered Jul 08 '21 at 04:57

function installAndTestForDialog {
# dialog allows me to create a pretty installer
# It is a required dependency since Pinecraft 1.1

# See https://unix.stackexchange.com/questions/85249/why-not-use-which-what-to-use-then
    local dialogBin
    local updated=false
    local dialogInstallSuccess

    dialogBin=$(command -v dialog)
    if [[ -z "$dialogBin" ]]; then
        printf "Installing dialog... "
        if [[ $updated -eq 0 ]]; then
            apt-get update > /dev/null 2>&1
            updated=1
            apt-get -y install dialog > /dev/null 2>&1
        fi
    # This was the issue.  The install of dialog
    # cannot be included with the post install test
    # Moving it up, to "break the nesting" solves the issue 
    fi
        # Test if dialog is installed to verify installation
        # above was successful.
        dialogBin=$(command -v dialog)
        if [[ -n "$dialogBin" ]]; then
            dialogInstallSuccess="Success"
            echo "${dialogInstallSuccess}"
            return
        else
            # This should never be reached
            return 1
        fi
        # Moved the fi that was here up. See comment above
}

The above function produces the desired result. I hate when I write something that is syntactically correct but contains faulty logic

FYI, in case you're wondering - it wasn't me who down-voted this. I rarely downvote anything (only answers that I think are dangerously wrong). Even though this is a bloated, over-complicated pile of steaming garbage, if it solves your problem and achieves your side-goal of minimising changes to the original script then who am i to judge? — cas, Jul 08 '21 at 05:20

Does an echo work in an if?

See https://unix.stackexchange.com/questions/85249/why-not-use-which-what-to-use-then

2 Answers2