Improve wcurl version comparison mechanism

We've been having a some problems with version comparison in the
script.  The approach we've taken so far is to decompose the current
curl version into two variables (major and minor), and then perform
comparisons against these components separately.  It works, but it's
confusing.

Another possible approach would be to use a C-style version
normalization (basically a printf "%02d%02d%02d") and then perform
comparisons against the versions we want.  The problem is that these
versions need also to be normalized, which can be confusing as well.

I decided to implement the second approach but abstract it as a simple
function that can take a regular version string like "8.16.0" as well as
a comparison operator that will then be passed onto to "test".  This
reads much nicer and abstracts the complexities of version
normalization away.  Unfortunately due to the limitations of shell
scripting it's not easy to deduplicate code in this scenario, but that
should be OK for now.

Signed-off-by: Sergio Durigan Junior <[email protected]>

Author: Sergio Durigan Junior

tests/tests.sh

@@ -246,6 +246,35 @@ testCurlBinaryOption()
     assertContains "Verify whether 'wcurl' invokes the binary specified by '--curl-binary'" "${bin}" "${curlbin}"
 }
 
+testCurlVersionComparison()
+{
+    # Verify that using a very old curl version correctly omits
+    # --parallel, --no-clobber and --parallel-max-host.
+    url='example.com'
+    curlbin="${ROOTDIR}/tests/curl-mock-version"
+    ret=$(CURL_MOCK_VERSION="1.0.0" ${WCURL_CMD} --dry-run --curl-binary "${curlbin}" "${url}" "${url}")
+    assertNotContains "Verify whether 'wcurl' correctly omits --parallel when using very old curl" "${ret}" '--parallel'
+    assertNotContains "Verify whether 'wcurl' correctly omits --no-clobber when using very old curl" "${ret}" '--no-clobber'
+    assertNotContains "Verify whether 'wcurl' correctly omits --parallel-max-host when using very old curl" "${ret}" '--parallel-max-host'
+
+    # Verify that using a curl version that's >= 7.66.0 and < 7.83.0
+    # correctly adds --parallel but omits --no-clobber and --parallel-max-host.
+    ret=$(CURL_MOCK_VERSION="7.66.0" ${WCURL_CMD} --dry-run --curl-binary "${curlbin}" "${url}" "${url}")
+    assertContains "Verify whether 'wcurl' correctly adds --parallel when using curl >= 7.66.0" "${ret}" '--parallel'
+    assertNotContains "Verify whether 'wcurl' correctly omits --no-clobber when using curl >= 7.66.0 && curl < 7.83.0" "${ret}" '--no-clobber'
+    assertNotContains "Verify whether 'wcurl' correctly omits --parallel-max-host when using curl >= 7.66.0 && curl < 8.16.0" "${ret}" '--parallel-max-host'
+
+    # Verify that using a curl version that's >= 7.83.0 and < 8.16.0 correctly adds --no-clobber but omits --parallel-max-host.
+    ret=$(CURL_MOCK_VERSION="7.83.0" ${WCURL_CMD} --dry-run --curl-binary "${curlbin}" "${url}" "${url}")
+    assertContains "Verify whether 'wcurl' correctly adds --no-clobber when using curl >= 7.83.0" "${ret}" '--no-clobber'
+    assertNotContains "Verify whether 'wcurl' correctly omits --parallel-max-host when using curl >= 7.83.0 && curl < 8.16.0" "${ret}" '--parallel-max-host'
+
+    # Verify that using a curl version that's >= 8.16.0 correctly adds
+    # --parallel-max-host.
+    ret=$(CURL_MOCK_VERSION="8.16.0" ${WCURL_CMD} --dry-run --curl-binary "${curlbin}" "${url}" "${url}")
+    assertContains "Verify whether 'wcurl' correctly adds --parallel-max-host. when using curl >= 8.16.0" "${ret}" '--parallel-max-host'
+}
+
 ## Ideas for tests:
 ##
 ## - URL with whitespace

wcurl

@@ -128,14 +128,49 @@ readonly UNSAFE_PERCENT_ENCODE="%2F %5C"
 # Whether to invoke curl or not.
 DRY_RUN="false"
 
+# The current version of curl.
+CURL_VERSION=""
+# The normalized curl version, in the format "XXYYZZ", where "XX" is
+# the zero-padded major version, "YY" is the zero-padded minor
+# version and "ZZ" is the zero-padded patch version.
+CURL_NORMALIZED_VERSION=""
+
 # Sanitize parameters.
 sanitize()
 {
     if [ -z "${URLS}" ]; then
         error "You must provide at least one URL to download."
     fi
 
-    readonly CURL_OPTIONS CURL_BINARY URLS DRY_RUN HAS_USER_SET_OUTPUT
+    CURL_VERSION=$(${CURL_BINARY} --version | head -n1 | cut -f2 -d' ')
+    if [ -z "${CURL_VERSION}" ]; then
+        error "Unable to determine curl version. Is curl installed?"
+    fi
+
+    CURL_NORMALIZED_VERSION=$(normalize_version "${CURL_VERSION}")
+
+    readonly CURL_OPTIONS \
+        CURL_BINARY \
+        URLS \
+        DRY_RUN \
+        HAS_USER_SET_OUTPUT \
+        CURL_VERSION \
+        CURL_NORMALIZED_VERSION
+}
+
+# Print the normalized format of a version specified as the first argument.
+#
+# The normalized version has the format "XXYYZZ", where "XX" is the
+# zero-padded major version, "YY" is the zero-padded minor version and
+# "ZZ" is the zero-padded patch version.
+normalize_version()
+{
+    version="${1}"
+    vermaj=$(printf "%s" "${version}" | cut -f1 -d.)
+    vermin=$(printf "%s" "${version}" | cut -f2 -d.)
+    verpatch=$(printf "%s" "${version}" | cut -f3 -d.)
+
+    printf "%02d%02d%02d" "${vermaj}" "${vermin}" "${verpatch}"
 }
 
 # Indicate via exit code whether the string given in the first parameter
@@ -203,36 +238,38 @@ get_url_filename()
     # No slash means there was just a hostname and no path; return empty string.
 }
 
+# Given a version (in the format MAJOR.MINOR) as the first argument
+# and an operator as the second argument, perform a comparison against
+# the current curl version.
+compare_curl_version()
+{
+    # Any of: -lt, -le, -eq, -gt, -ge
+    operator="${1}"
+    version="${2}"
+    version_to_compare=$(normalize_version "${version}")
+
+    test "${CURL_NORMALIZED_VERSION}" "${operator}" "${version_to_compare}"
+}
+
 # Execute curl with the list of URLs provided by the user.
 exec_curl()
 {
     CMD="${CURL_BINARY} "
 
-    # Store version to check if it supports --no-clobber, --parallel and --parallel-max-host.
-    curl_version=$($CMD --version | cut -f2 -d' ' | head -n1)
-    curl_version_major=$(echo "$curl_version" | cut -f1 -d.)
-    curl_version_minor=$(echo "$curl_version" | cut -f2 -d.)
-
     CURL_NO_CLOBBER=""
     CURL_PARALLEL=""
 
-    if [ "${curl_version_major}" -ge 8 ]; then
+    # --parallel is only supported since 7.66.0.
+    if compare_curl_version -ge "7.66.0"; then
+        CURL_PARALLEL="--parallel"
+    fi
+    # --no-clobber is only supported since 7.83.0.
+    if compare_curl_version -ge "7.83.0"; then
         CURL_NO_CLOBBER="--no-clobber"
-        CURL_PARALLEL="--parallel --parallel-max-host 5"
-
-        # --parallel-max-host is only supported since 8.16.0.
-        if [ "${curl_version_major}" -eq 8 ] && [ "${curl_version_minor}" -lt 16 ]; then
-            CURL_PARALLEL="--parallel"
-        fi
-    elif [ "${curl_version_major}" -eq 7 ]; then
-        # --no-clobber is only supported since 7.83.0.
-        if [ "${curl_version_minor}" -ge 83 ]; then
-            CURL_NO_CLOBBER="--no-clobber"
-        fi
-        # --parallel is only supported since 7.66.0.
-        if [ "${curl_version_minor}" -ge 66 ]; then
-            CURL_PARALLEL="--parallel"
-        fi
+    fi
+    # --parallel-max-host is only supported since 8.16.0.
+    if compare_curl_version -ge "8.16.0"; then
+        CURL_PARALLEL="${CURL_PARALLEL} --parallel-max-host 5"
     fi
 
     # Detecting whether we need --parallel. It is easier to rely on