Bash functions cp_vrfy, mv_vrfy, etc., do not actually verify success

[mkavulich]

Expected behavior

There is a set of bash functions named "*_vrfy" that are meant to verify the success of a system command, such as cp, mv, etc. In the past, I have thought these were mostly harmless, if redundant, since for most functions, any failure will result in a descriptive error

Current behavior

As it turns out, these function are dangerously masking failures rather than giving more information about them. For example, if you want to verify a file was copied with cp_vrfy, but that file does not exist, a message will be printed out, but the exit code will be set to 0, and the program will happily hum along with no failure. This is in opposition to the UNIX built-in cp function, which does fail for a non-existent file.

These functions are not adding functionality, they are taking away functionality, and are actively harmful to the workflow's utility.

Machines affected

Likely all, tested on Jet and Cheyenne

Steps To Reproduce

Attached is a script that can be run from the ush/ directory that demonstrates this incorrect and pernicious behavior. It must be run from a successfully created experiment directory (this is another frustrating thing about the bash utilites: they are very hard to test on their own without an existing experiment already set up)

1. Copy attached script to an experiment working directory
2. Run the script with the utility to test as an argument
3. See the results

• The functions cp_vrfy, mv_vrfy, mkdir_vrfy, rm_vrfy, and ln_vrfy never produce a failure even when they should.
• The function cd_vrfy produces the same failure as cd
• The function ln does not fail when creating a broken link; this is the one additional functionality we might need (but ln_vrfy does not currently provide)

Detailed Description of Fix

I don't think this bug is worth fixing, these functions should be deprecated and removed. See below.

Possible Implementation

Use the "set -e" flag to have scripts exit on an error, so we can use the built-in unix utilities instead of these custom functions.

We should also ditch the _vrfy functions (aside from maybe ln_vrfy, which will need some work to function as desired to fail when a broken link is created). They are redundant to bash's built-in ability to exit on an error, and represent a huge amount of bulk and unnecessary potential failure modes to the workflow.

Output

This is the above-mentioned test script in its entirety. Again, it must be run from a valid experiment directory in order to get the "var_defns.sh" file.

#!/bin/bash

# The "set -e" command tells the script to exit when it reaches an error.
# We should make this flag standard for all scripts
set -e
scrfunc_fp=$( readlink -f "${BASH_SOURCE[0]}" )
scrfunc_fn=$( basename "${scrfunc_fp}" )
scrfunc_dir=$( dirname "${scrfunc_fp}" )
exptdir=$( dirname "$0" )
# Source necessary files.
. $exptdir/var_defns.sh
. $USHdir/source_util_funcs.sh

# Declare an array with the valid commands to verify
declare -a valid=("cp" "mv" "rm" "ln" "mkdir" "cd")

if [ $# -lt 1 ]; then

  echo "Please provide a command to test: ${valid[*]}"
  exit 1

fi

validarg=0
for i in "${valid[@]}"; do
   if [ "$1" == "$i" ]; then
      echo "Testing ${i}_vrfy"
      validarg=1
   fi
done

if [ $validarg -eq 0 ]; then
  echo "Invalid argument provided: valid args are:"
  echo "${valid[*]}"
  exit 1
fi

# Attempt a bad command, see if it fails
echo ""
echo "*********************"
echo "This command should fail, but it probably doesn't!"
echo "*********************"
echo ""
set -x
${1}_vrfy "test1" "test2"
set +x
echo "Error status: $?"

# Demonstrate a correct failure with a true bad command:
echo ""
echo "*********************"
echo "This command MIGHT fail as expected:"
echo "*********************"
echo ""
set -x
${1} "test1" "test2"
echo "Error status: $?"

echo "If we got success for both commands, this message will be displayed"

[mkavulich]

Just a follow-up on my original comment about ln_vrfy; this one can be removed too! I forgot that we now have the function create_symlink_to_file, which has the functionality of ln plus the desired error checking. So now it's safe to say that every single one of these *_vrfy functions can be removed from the workflow.