ShellCheck warning: "Iterating over ls output is fragile. Use globs. [SC2045]"

Question

I am getting a ShellCheck warning [SC2045] for the second line in the code below. Is it fine to ignore it as I am making sure the directory is not empty before I try the last ls?

 if [ "$(ls -A "$retryDir")" ]  ; then
    for thisRetryFile in $(ls "$retryDir"/*.tar.gz) ; do
        scp -o ConnectTimeout=30  "$thisRetryFile"  \             
              "$remoteUser@$remoteHost:$remotePath" >> "$BACKUPLOG"
    done
 fi

UPDATE: After reading the post comments. I have changed the line to:

for thisRetryFile in "$retryDir"/*.tar.gz ; do

This has removed the warnings.

Answer 1

Use the loop with a glob, and also set nullglob to avoid executing scp in case the pattern doesn't match anything. And you don't need the outer if condition either, since the for with nullglob effectively takes care of that:

shopt -s nullglob

for thisRetryFile in "$retryDir"/*.tar.gz; do
    scp -o ConnectTimeout=30  "$thisRetryFile" \
          "$remoteUser@$remoteHost:$remotePath" >> "$BACKUPLOG"
done

If you want to catch the case when no file matches the pattern, you can write like this, without using shopt -s nullglob:

for thisRetryFile in "$retryDir"/*.tar.gz; do
    if [ -f "$thisRetryFile" ]; then
        scp -o ConnectTimeout=30  "$thisRetryFile" \
            "$remoteUser@$remoteHost:$remotePath" >> "$BACKUPLOG"
        break
    else
        echo "warn: no tar.gz file in dir: $retryDir"
    fi
done