3
\$\begingroup\$

Solved task is pretty straightforward: mounting partition if not mounted already, to start executable.

There is a couple of checks and notifications because it is supposed to be launched from X and there are no way to see stdout/stderr stream. I have started with much larger version and a lot of error handling, that is why it has a function despite being so simple. On the other hand, it is much more cleaner of what is going on, when you read it and it will be easier to come back to it later on.

#!/usr/bin/env bash
DEVICE="/dev/sda5"
PARTITION="/mnt/Games"
EXECUTABLE="$PARTITION/MultiMC/MultiMC"

launch() {
    if ! findmnt "$PARTITION"; then
        DETAILS=`SUDO_ASKPASS="$(which ssh-askpass)" sudo --askpass mount "$DEVICE" "$PARTITION" 2>&1`
        [ $? -ne 0 ] && return 1
    fi
    notify-send "Launching.."
    exec "$EXECUTABLE"
}

launch || notify-send "Launching failed" "$DETAILS"
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Terminology: a partition is a part of a disk (such as /dev/sda5). The filesystem directory to which we attach its filesystem (/mnt/Games) is called a mount point. \$\endgroup\$ May 20 at 9:52
  • 1
    \$\begingroup\$ Does your system have pmount? That could be configured to let ordinary users mount the filesystem. \$\endgroup\$ May 20 at 9:54
  • \$\begingroup\$ @TobySpeight no I doesn't. I have some trouble configuring fstab to mount that partition that is why I ended up with writing the wrapper. Literally same /dev/sda5 /mnt/Games works if you will execute sudo mount command but didn't not work if you will place it into fstab under "defaults,user" and do mount (have some permission issues launching executable) \$\endgroup\$
    – outoftime
    May 20 at 10:44
  • \$\begingroup\$ That should work for you - might be worth describing that problem on Unix & Linux to find out if you can solve it. Might help reduce the complexity here! \$\endgroup\$ May 20 at 10:46
4
\$\begingroup\$

[ $? -ne 0 ] is a well-known over-complication - you can replace that with a simple || in the pipeline:

DETAILS=`SUDO_ASKPASS="$(which ssh-askpass)" sudo --askpass mount "$DEVICE" "$PARTITION" 2>&1` \
    || return $?

And in fact we can have a whole chain here:

launch() {
    findmnt "$PARTITION" \
        || DETAILS=`SUDO_ASKPASS="$(which ssh-askpass)" sudo --askpass mount "$DEVICE" "$PARTITION" 2>&1` \
        || return $?
    notify-send "Launching.."
    exec "$EXECUTABLE"
}

If you have chronic installed, that could replace the tedious saving of output into DETAILS:

NAME
       chronic - runs a command quietly unless it fails

SYNOPSIS
       chronic [-ev] COMMAND...

DESCRIPTION
       chronic runs a command, and arranges for its standard out and standard
       error to only be displayed if the command fails (exits nonzero or
       crashes).  If the command succeeds, any extraneous output will be
       hidden.

So,

launch() {
    findmnt "$PARTITION" \
        || SUDO_ASKPASS="$(which ssh-askpass)" \
           chronic sudo --askpass mount "$DEVICE" "$PARTITION" \
        || return $?
    notify-send "Launching.."
    exec "$EXECUTABLE"
}
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the chronic tip. That looks handy. \$\endgroup\$
    – chicks
    May 20 at 16:21
  • 1
    \$\begingroup\$ You have forgot about DETAILS variable in case of chronic or how to send output to notify-send without this var? \$\endgroup\$
    – outoftime
    May 21 at 2:32
3
\$\begingroup\$

Shell Errorhandling

There's one thing I would have done differently (though it's not relevant to the small piece of code here) and that is the error handling inside the function.

  1. You can use set -e to automatically exit the function, you could eliminate the [ $? -ne 0 ] && return 1 line with that.
  2. Optionally, I would only have set that flag inside the function only, using local -.

Backticks

Another thing is the use of backticks. I believe you could just write that

DETAILS=$(
    SUDO_ASKPASS="$(which ssh-askpass)" \
        sudo --askpass mount "$DEVICE" "$PARTITION" 2>&1
)

avoiding deeply indented code like that and making it a bit easier to read.

\$\endgroup\$
3
  • \$\begingroup\$ set -e is not an option as you have no chance to sent notification about what went wrong because it terminates bash process. \$\endgroup\$
    – outoftime
    May 21 at 5:12
  • \$\begingroup\$ Can you explain what you mean by local - ? I have a trouble to find out what it is. \$\endgroup\$
    – outoftime
    May 21 at 5:21
  • \$\begingroup\$ It's in the Bash manpage, it causes a subsequent set call to be only effective locally. \$\endgroup\$
    – uli
    May 21 at 6:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.