2
\$\begingroup\$

The expected behavior of the script is to download a file from a URL if the file is not present in the current directory and compare the md5 checksum of the file against the md5 checksum from the server. If the file is present then perform the verification again and download the file if the verification fails. As I have a poor internet connection I attempt the download and verification a second time if the initial verification fails. If verification fails on the second attempt the script exits and logs a fatal error.

The script is called with two inputs. The first input is a file that contains the full URL with filename to download. One file per line. The second input is a file that contains the expected md5checksum and the filename. Expected format of checksum file list "checksum" "filename" without the quotes.

Currently the script works as expected. As I'm in the process of teaching myself bash/sh scripting. Is there a cleaner way I can implement the expected behavior. I am aware of the continue option for wget. I choose not to use it as it felt "too easy" and not in the spirit of learning sh scripting.

The two sourced files can be found at my forks of the original repos https://github.com/CJ-Systems/log4sh and https://github.com/CJ-Systems/shlib/blob/master/functions/shlib_ansi

#!/bin/sh
#

# load log4sh
if [ -r ./lib/log4sh ]; then
    . ./lib/log4sh
else
    echo "ERROR: could not load (log4sh)" >&2
    exit 1
fi

# load shlib_ansi
if [ -r ./lib/shlib_ansi ]; then
    . ./lib/shlib_ansi
else
    echo "ERROR: could not load (shlib_ansi)" >&2
    exit 1
fi

# Download single file
downloadfile() {
    local fn=$1
    log DEBUG "${shlib_ansi_red}Downloading $bn ${shlib_ansi_none}"
    wget --quiet --show-progress --no-use-server-timestamps $fn -O $bn
}

# Compare md5 checksums
verifyfile() {
    local check_sum=$1
    exp_md5=$(cat $check_sum | grep -i $bn | cut -d' ' -f1)
    md5_local=$(md5sum $bn | cut -d' ' -f1)
    if [ $md5_local = $exp_md5 ]
    then
        log DEBUG "Checksum valid for $bn"
        return 0
    else
        log DEBUG "Checksum invalid for $bn"
        return 1
    fi
}

# Download all files in file_list
download() {
    local file_list=$1
    local check_sums=$2
    for f in $(cat $file_list)
    do
        # Get name file of file to download
        bn=$(basename $f)
        # If file not present attempt to download
        if [ ! -f $bn ]
        then
            log INFO "Downloading $bn"
            downloadfile $f
            if verifyfile $check_sums
            then
                log DEBUG "Verification of $bn successful"
            else
                log WARN "Verification of $bn failed. Retrying download"
                downloadfile $f
                if ! verifyfile $check_sums
                then
                    log FATAL "Failed to verify $bn download unsuccessful"
                    exit
                    else
                        log DEBUG "Verification successful"
                fi
            fi
        # Local copy of file exists verify checksum redownload if necessary
        else
            log INFO "File $bn already downloaded. Verifying checksum"
            if ! verifyfile $check_sums
            then
                log WARN "Verification of $bn failed. Redownloading"
                downloadfile $f
                if verifyfile $check_sums
                then
                    log INFO "Redownload and verification of $bn successful"
                else
                    log WARN "Verification of $bn failed. Retrying download"
                    downloadfile $f
                    if ! verifyfile $check_sums
                    then 
                        log FATAL "Failed to verify $bn redownload unsuccessful"
                        exit
                    fi
                fi
                else log INFO "Verification of $bn successful"
            fi
        fi
    done;
}
shlib_ansi_init auto
download $1 $2
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I don't see any value in testing files for readability before sourcing them. Just attempt, and test whether we were successful:

die()
{
    echo "$@" >&2
    exit 1
}

. ./lib/log4sh || die "ERROR: could not load (log4sh)"
. ./lib/shlib_ansi || die "ERROR: could not load (shlib_ansi)"

Lots of variable expansions are missing quotes - always use quote:

    wget --quiet --show-progress --no-use-server-timestamps "$fn" -O "$bn"
            downloadfile "$f"

We're using variables ($bn) to communicate values into functions - prefer to use function arguments for this:

# Download single URL ($1) and save it to local file ($2)
downloadfile() {
    log DEBUG "${shlib_ansi_red}Downloading $b2 ${shlib_ansi_none}"
    wget --quiet --show-progress --no-use-server-timestamps $1 -O $2
}

In verifyfile we should be able to use md5sum -c to directly check instead of using [:

# true if checksum record line in $1 matches MD5 of file $2
verifyfile()
{
    if awk -F ' [* ]' -vfile="$2" '$2==file' "$1" | md5sum --status --check 2>/dev/null
    then
        log DEBUG "Checksum valid for $bn"
        return 0
    else
        log DEBUG "Checksum invalid for $bn"
        return 1
    fi
}

In download, we have to store the entire contents of the list, which might be large, due to $(cat $file_list). It's probably better to read from this file, which will greatly reduce our memory requirement.

There's a lot of repetition in download. If we treat a non-existent file the same as a checksum-failed file, we can simplify a lot:

  1. If file not present, download it.
  2. Test checksum. If successful, we're done.
  3. Re-download.
  4. Test checksum.

That makes for a much simpler function:

download()
{
    file_list=$1
    check_sums=$2
    result=true
    while read -r f
    do
        # Get name file of file to download
        bn=$(basename "$f")
        # If file not present attempt to download
        if ! [ -f "$bn" ]
        then
            log INFO "Downloading $bn"
            downloadfile "$f" "$bn"
        fi
        if verifyfile "$check_sums" "$bn"
        then
            log DEBUG "Verification of $bn successful"
            continue;
        fi
        
        log WARN "Verification of $bn failed. Retrying download"
        if downloadfile "$f" "$bn" && verifyfile "$check_sums" "$bn"
        then
            log DEBUG "Verification of $bn successful"
        fi

        log FATAL "Failed to verify $bn download unsuccessful"
        result=false
    done <"$file_list"
    "$result"    
}

For reference, here's what Shellcheck reports:

285297.sh:22:5: warning: In POSIX sh, 'local' is undefined. [SC3043]
285297.sh:22:14: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:24:61: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:24:68: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:29:5: warning: In POSIX sh, 'local' is undefined. [SC3043]
285297.sh:29:21: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:30:19: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:30:40: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:31:24: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:32:10: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:32:23: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:44:5: warning: In POSIX sh, 'local' is undefined. [SC3043]
285297.sh:44:21: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:45:5: warning: In POSIX sh, 'local' is undefined. [SC3043]
285297.sh:45:22: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:46:14: note: To read lines rather than words, pipe/redirect to a 'while read' loop. [SC2013]
285297.sh:46:20: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:49:23: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:51:19: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:54:26: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:55:27: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:60:30: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:61:33: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:72:29: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:75:30: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:76:31: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:81:34: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:82:37: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:94:10: note: Double quote to prevent globbing and word splitting. [SC2086]
285297.sh:94:13: note: Double quote to prevent globbing and word splitting. [SC2086]
\$\endgroup\$
3
  • \$\begingroup\$ While testing if the script is called directly die will not execute if source fails to find the file. If I spawn a subshell then die will execute as expected. In both cases the standard error message is displayed unless I redirect source to dev/null. Perhaps this is why the original author had used an explicit if else instead of just using ||. \$\endgroup\$ Commented Jun 3, 2023 at 4:09
  • \$\begingroup\$ Good catch. POSIX says that If no readable file is found, a non-interactive shell shall abort. Bash (without --posix option) does reach the die. I'm not sure there's any benefit to providing our own message in addition, but I believe that the EXIT trap runs in the failure case, so we could use that if we really wanted. \$\endgroup\$ Commented Jun 3, 2023 at 10:21
  • \$\begingroup\$ At this point I'd say the script itself is feature complete. Taking into consideration the parent project I know that both referenced files will be used multiple times and feel any further attempts to try and implement a custom message would be akin to reinventing the wheel. Though I will admit it was a fun learning exercise for me. I'd vote to close the question at this time. I do have the code hosted on GitHub \$\endgroup\$ Commented Jun 3, 2023 at 23:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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