Simplify lib/fetch.sh:check_md5sum()

The following changes were made in order to make check_md5sum a
single-purpose function.
    1) Use md5sum directly to compute and compare rather than doing it
       manually.
    2) Remove creation of $builddir/md5sum since those files are not
       used in abe
    3) Return error when md5sum returns error.
       a) return error on file not found
       b) return error if computed md5sum doesn't match entry.
    4) Return error when the ${getfile} is not in the md5sums file.
    5) Move permissive logic for non-present ${getfile} into
       check_md4sum caller.
    4) Check ${force} on return from check_md5sum() to ignore md5sum
       failure.

Change-Id: I69d5371953ebf32e2e0d0dc207ccd37e65b3225c

2 files changed, 57 insertions, 41 deletions

diff --git a/lib/fetch.sh b/lib/fetch.sh
index 6bfdd3a3..3b4c654f 100755
--- a/lib/fetch.sh
+++ b/lib/fetch.sh
@@ -111,10 +111,13 @@ fetch()
 	return 1
     fi
 
+    # Fetch only supports fetching files which have an entry in the md5sums file.
+    # An unlisted file should never get this far anyway.
     dryrun "check_md5sum ${getfile}"
-#    if test $? -gt 0; then
-#	return 1
-#    fi
+    if test $? -gt 0 -a x"${force}" != xyes; then
+	error "md5sums don't match!"
+	return 1
+    fi
 
     create_stamp "${stampdir}" "${stamp}"
 
@@ -184,54 +187,34 @@ fetch_http()
     return 0
 }
 
+# This is a single purpose function which will report whether the input getfile
+# in $1 has an entry in the md5sum file and whether that entry's md5sum matches
+# the actual file's downloaded md5sum.
 check_md5sum()
 {
 #    trace "$*"
 
+    # ${local_snapshots}/md5sums is a pre-requisite.
     if test ! -e ${local_snapshots}/md5sums; then
-	fetch_http md5sums
-	if test $? -gt 0; then
-	    error "couldn't fetch md5sums"
-	    return 1
-	fi
+        error "${local_snapshots}/md5sums is missing."
+        return 1
     fi
 
-    local dir="`dirname $1`/"
-    if test x"${dir}" = x"."; then
-	local dir=""
+    local entry=
+    entry=$(grep "${1}" ${local_snapshots}/md5sums)
+    if test x"${entry}" = x; then
+        error "No md5sum entry for $1!"
+        return 1
     fi
 
-    # Drop the file name from .tar to the end to keep grep happy
-    local getfile=`echo ${1}`
+    # Ask md5sum to verify the md5sum of the downloaded file against the hash in
+    # the index.  md5sum must be executed from the snapshots directory.
+    pushd ${local_snapshots} &>/dev/null
+    dryrun "echo \"${entry}\" | md5sum --status --check -"
+    md5sum_ret=$?
+    popd &>/dev/null
 
-    newsum="`md5sum ${local_snapshots}/$1 | cut -d ' ' -f 1`"
-    oldsum="`grep ${getfile} ${local_snapshots}/md5sums | cut -d ' ' -f 1`"
-    # if there isn't an entry in the md5sum file, we're probably downloading
-    # something else that's less critical.
-    if test x"${oldsum}" = x; then
-	warning "No md5sum entry for $1!"
-	return 0
-    fi
-
-    if test x"${oldsum}" = x"${newsum}"; then
-	notice "md5sums matched"
-	# We don't need to pass $2 to get_builddir() in this case because the
-	# builddir is always based on a tarball and therefore we don't have a
-	# special builddir for a combined binutils and gdb.
-	local builddir="`get_builddir $1`"
-	rm -f ${builddir}/md5sum
-	echo "${newsum} > ${builddir}/md5sum"
-	return 0
-    else
-	error "md5sums don't match!"
-	if test x"${force}" = x"yes"; then
-	    return 0
-	else
-	    return 1
-	fi
-    fi
-
-    return 0
+    return $md5sum_ret
 }
 
 # decompress and untar a fetched tarball
diff --git a/testsuite/test.sh b/testsuite/test.sh
index aa082f33..78863ef3 100755
--- a/testsuite/test.sh
+++ b/testsuite/test.sh
@@ -668,6 +668,39 @@ if test ! -e "${local_snapshots}/md5sums.bak"; then
 else
     pass "Found ${local_snapshots}/md5sums.bak"
 fi
+
+# Download a clean/new copy for the check_md5sum tests
+rm ${local_snapshots}/infrastructure/gmp-5.1.3.tar.xz*
+fetch_http infrastructure/gmp-5.1.3.tar.xz 2>/dev/null
+
+out="`check_md5sum 'infrastructure/gmp-5.1.3.tar.xz' 2>/dev/null`"
+if test $? -gt 0; then
+    fail "check_md5sum failed for 'infrastructure/gmp-5.1.3.tar.xz"
+else
+    pass "check_md5sum passed for 'infrastructure/gmp-5.1.3.tar.xz"
+fi
+
+# Test with a non-infrastructure file
+out="`check_md5sum 'infrastructure/foo.tar.xz' 2>/dev/null`"
+if test $? -gt 0; then
+    pass "check_md5sum failed as expected for 'infrastructure/foo.tar.xz"
+else
+    fail "check_md5sum passed as expected for 'infrastructure/foo.tar.xz"
+fi
+
+mv ${local_snapshots}/infrastructure/gmp-5.1.3.tar.xz ${local_snapshots}/infrastructure/gmp-5.1.3.tar.xz.back
+echo "empty file" > ${local_snapshots}/infrastructure/gmp-5.1.3.tar.xz
+
+# Test an expected failure case.
+out="`check_md5sum 'infrastructure/gmp-5.1.3.tar.xz' 2>/dev/null`"
+if test $? -gt 0; then
+    pass "check_md5sum failed as expected for nonmatching 'infrastructure/gmp-5.1.3.tar.xz file"
+else
+    fail "check_md5sum passed unexpectedly for nonmatching 'infrastructure/gmp-5.1.3.tar.xz file"
+fi
+
+mv ${local_snapshots}/infrastructure/gmp-5.1.3.tar.xz.back ${local_snapshots}/infrastructure/gmp-5.1.3.tar.xz
+
 # ----------------------------------------------------------------------------------
 echo "============= find_snapshot() tests ================"