0
\$\begingroup\$

The following is a bash script named wrapper.sh:

#!/bin/bash

data="ZAA:huhu;ZBB:blub;___uname" # actually coming from a Python script

i=1 # counter
while true ; do # loop infinitely
    segment=$(echo $data | cut -d ';' -f$i)
    if [ -z "$segment" ] ; then # is segment empty?
        break
    fi
    if [[ $segment == ___* ]] ; then # is this a command?
        cmd="${segment:3}" # strip command prefix
        $cmd "${@:2}" # run *interactive* command
        # cat | $cmd "${@:2}" # "OPTIONAL": pass data from stdin ONLY IF THERE IS A PIPE (?)
    else
        varname=$(echo $segment | cut -d ':' -f1)
        vardata=$(echo $segment | cut -d ':' -f2)
        export $varname=$vardata # export environment variable
    fi
    i=$((i + 1)) # increment counter
done

# Just a test
echo $ZAA
echo $ZBB

The above script has to accomplish the following goals:

  • Pass its own parameters to a Python script, which processes them and returns a data string (intentionally left out of the above example)
  • Set (export) an arbitrary number of environment variables. Their names and contents are provided in data.
  • Run a command also provided in data and pass on all but the first argument given to the shell script.

Goal the code does not achieve yet: If a pipe is passed to the script (as in some_program | ./wrapper.sh), the pipe is supposed to passed on to the command called by wrapper.sh. The statement cat | $cmd "${@:2}" basically does this, but it hangs if no pipe is passed to wrapper.sh.

Why? Well actually, this thing started as a pure Python script, which was calling the command through subprocess.Popen. But it turned out insanely hard and error-prone to run an interactive command like this where an actual human user is supposed to interact with the (running) command. This is something bash is infinitely better at.

Example:

(env) user@box:~> ./wrapper.sh 
Linux
huhu
blub
(env) user@box:~> ./wrapper.sh omitted
Linux
huhu
blub
(env) user@box:~> ./wrapper.sh omitted -a
Linux X.X 4.X.XXX #1 SMP XXX 2018 (XXX) x86_64 x86_64 x86_64 GNU/Linux
huhu
blub

I am not exactly a frequent bash coder, so any ideas on how to improve the above are highly welcome. I might also have overlooked "edge cases" where my script does something strange - I'd love to know.

\$\endgroup\$
1
  • 4
    \$\begingroup\$ I'm not following what you are really trying to accomplish with this seemingly insane system. Perhaps you would be better off including the entire Python-and-Bash system here fore review, so that we can understand the context and suggest a better mechanism entirely. \$\endgroup\$ Feb 14, 2019 at 17:58

2 Answers 2

1
\$\begingroup\$
  • The infinite loop with break, counter, and multiple invocations of cut looks clumsy at best. Consider instead

    IFS=";"
    for segment in $data; do
        # your logic here
    done
    
  • Similarly, cutting the segment is unnecessary:

        IFS=":"
        set $segment
        export $1=$2
        IFS=";"
    

    does the same job.

  • I don't understand the goal not achieved yet. The script has no knowledge where its input coming from. Whoever calls your script is in the position to redirect it at will. Perhaps, close stdin before invoking it?

\$\endgroup\$
0
\$\begingroup\$

Overcomplicated and inefficient parsing

The parsing of data is both overcomplicated and inefficient:

  • The loop is confusing, because each step has to decide between multiple possible action: split arguments, or run a command, or break. Ideally each step does one kind of thing, that would be very easy to understand

  • As the other review pointed out, invoking cut in a loop is inefficient. In most steps it's invoked multiple times. There are much better ways to split strings than cut.

If there are no spaces in data (as in the example), then the loop can be replaced with:

cmd=${data#*;___}       # extract the suffix, chopping off the beginning until ";___"
vars=${data%%;___*}     # extract the prefix, chopping off the end from ";___"
vars=(${vars//[:;]/ })  # replace : and ; with space, and convert to an array

for ((i = 0; i < ${#vars[@]}; i += 2)); do
    export ${vars[i]}=${vars[i+1]}
done

"$cmd" "${@:2}"

I think this is easier to understand and efficient.

Flawed justification to use Bash instead of Python

Why? Well actually, this thing started as a pure Python script, which was calling the command through subprocess.Popen. But it turned out insanely hard and error-prone to run an interactive command like this where an actual human user is supposed to interact with the (running) command. This is something bash is infinitely better at.

It's not at all clear why Bash is better at whatever "this" is. It should not be insanely hard and error-prone to run interactive programs from Python. If you post that Python code (in another question), a reviewer is likely to be able to guide you back to sanity.

Minor Bash issues

Don't echo and pipe. Use here strings instead. For example, instead of echo $data | cut -d ';' -f$i, write cut -d ';' -f$i <<< "$data".

Double-quote variables used in command arguments. Instead of echo $data, write echo "$data".

Double-quote variables used as commands. Instead of $cmd "${@:2}", write "$cmd" "${@:2}".

Instead of i=$((i + 1)), the arithmetic expression ((i++)) is probably easier to read, write and understand.

\$\endgroup\$

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