Merge pull request #3561 from n0tl3ss/fix/python-build-download-progress

python-build: show download progress in interactive terminals

plugins/python-build/bin/python-build

@@ -425,6 +425,18 @@ http_head_aria2c() {
   aria2c --dry-run --no-conf=true ${ARIA2_OPTS} "$1" >&4 2>&1
 }
 
+# Shows interactive output of a program
+# while also logging it.
+# Usage:
+# command <switches forcing interactive output to non-terminal> 2>&1 | interactive_output; rc=${PIPESTATUS[0]}
+interactive_output() {
+  if [ -n "$VERBOSE" ]; then
+    cat >> "$LOG_PATH"
+  else
+    tee -a "$LOG_PATH" >&3
+  fi
+}
+
 http_get_aria2c() {
   # aria2c always treats -o argument as a relative path
   local out dir_out;
@@ -447,7 +459,26 @@ http_get_aria2c() {
     dir_out="$HOME"
   fi
 
-  if aria2c --allow-overwrite=true --no-conf=true -d "${dir_out}" -o "${out}" ${ARIA2_OPTS} "$1" >&4; then
+  local aria2c_progress=
+  # aria2c prints status to stdout rather than stderr.
+  # As a result, when stdout is redirected, it becomes line-buffered,
+  # and status messages (which are divided by CRs) are not seen
+  # until a summary message with some LFs is printed.
+  # So we have to print summaries often as a crutch.
+  # But need to make the interval reasonable because each summary is
+  # several lines long which, made too often, becomes obnoxious.
+  [ -t 3 ] && aria2c_progress="--summary-interval=5"
+  local cmd
+  cmd=(aria2c --allow-overwrite=true --no-conf=true -d "${dir_out}" -o "${out}" ${aria2c_progress} ${ARIA2_OPTS} "$1")
+ 
+  if [ -t 3 ]; then
+    "${cmd[@]}" 2>&1 | interactive_output
+    ret=${PIPESTATUS[0]}
+  else
+    "${cmd[@]}" >&4
+    ret=$?
+  fi
+  if [ "$ret" -eq 0 ]; then
     [ -n "$2" ] || cat "${dir_out:-.}/${out}"
   else
     false
@@ -471,7 +502,21 @@ http_get_curl() {
   local curl_is_snap;
   if [[ $(command -v curl) == "/snap/"* ]]; then curl_is_snap=1; fi
   if [[ -n $2 && -n $curl_is_snap ]]; then out="$HOME/$(basename "$2")"; else out="$2"; fi
-  curl -q -o "${out:--}" -sSLf ${CURL_OPTS} "$1" || return $?
+  local curl_progress=
+  if [ -t 3 ]; then
+    curl_progress="--no-silent"
+  fi
+  local cmd
+  cmd=(curl -q -o "${out:--}" -sSLf ${CURL_OPTS} ${curl_progress} "$1")
+  if [ -t 3 ]; then
+    exec 5>&1
+    "${cmd[@]}" 3>&1 1>&5 2>&3 3>&- | interactive_output
+    ret=${PIPESTATUS[0]}
+    exec 5>&-
+    [ "$ret" -eq 0 ] || return "$ret"
+  else
+    "${cmd[@]}" || return $?
+  fi
   if [[ -n $out && -n $curl_is_snap ]]; then mv "$out" "$2"; fi
 }
 
@@ -480,7 +525,21 @@ http_head_wget() {
 }
 
 http_get_wget() {
-  wget -nv ${WGET_OPTS} -O "${2:--}" "$1"
+  local wget_progress="-nv"
+  if [ -t 3 ]; then
+    wget_progress="-nv --show-progress --progress=bar:force:noscroll"
+  fi
+  local cmd
+  cmd=(wget ${WGET_OPTS} ${wget_progress} -O "${2:--}" "$1")
+  if [ -t 3 ]; then
+    exec 5>&1
+    "${cmd[@]}" 3>&1 1>&5 2>&3 3>&- | interactive_output
+    ret=${PIPESTATUS[0]}
+    exec 5>&-
+    return "$ret"
+  else
+    "${cmd[@]}"
+  fi
 }
 
 fetch_tarball() {
@@ -2777,7 +2836,8 @@ else
   BUILD_PATH="$PYTHON_BUILD_BUILD_PATH"
 fi
 
-exec 4<> "$LOG_PATH" # open the log file at fd 4
+# Append mode needed to be able to `tee` to it at the same time
+exec 4>> "$LOG_PATH"
 if [ -n "$VERBOSE" ]; then
   tail -f "$LOG_PATH" &
   TAIL_PID=$!

plugins/python-build/test/fetch.bats

@@ -8,6 +8,21 @@ _setup() {
   mkdir -p "${PYTHON_BUILD_BUILD_PATH}"
 }
 
+check_script_available() {
+  command -v script >/dev/null || skip "'script' not installed"
+}
+
+run_with_script() {
+  local command
+  # `script' outputs CRLF because ttys do so under the hood
+  # https://unix.stackexchange.com/questions/343324/why-in-the-output-of-script-1-the-newline-is-cr-lf-dos-style
+  case "$(uname -s)" in
+    Linux) printf -v command '%q ' "$@"; script -qec "$command" /dev/stdout | tr -d $'\r' ;;
+    *) script -q /dev/stdout "$@" | tr -d $'\r' ;;
+  esac </dev/null
+  return ${PIPESTATUS[0]}
+}
+
 @test "failed download displays error message" {
   stub curl false
 
@@ -17,6 +32,22 @@ _setup() {
   assert_output_contains "error: failed to download package-1.0.0.tar.gz"
 }
 
+@test "interactive download progress is both shown and logged" {
+  check_script_available
+  export TMPDIR="$BATS_TEST_TMPDIR"
+  stub curl '** -o * ** --no-silent ** http://example.com/* : "'"$BASH"'" -ec '\''echo download-progress >&2; "$FIXTURE_ROOT/fake_downloader" "$@"'\'' "$@"'
+
+  run run_with_script python-build "$FIXTURE_ROOT/definitions/without-checksum" "$INSTALL_ROOT"
+  assert_success
+  unstub curl
+
+  assert_line download-progress
+
+  run cat "$BATS_TEST_TMPDIR"/python-build.*.log
+  assert_success
+  assert_line download-progress
+}
+
 @test "using aria2c if available" {
   export PYTHON_BUILD_ARIA2_OPTS=
   export -n PYTHON_BUILD_HTTP_CLIENT
@@ -33,6 +64,23 @@ OUT
   unstub aria2c
 }
 
+@test "interactive aria2c output is emulated by printing summaries" {
+  check_script_available
+  export TMPDIR="$BATS_TEST_TMPDIR"
+  export PYTHON_BUILD_ARIA2_OPTS=
+  export -n PYTHON_BUILD_HTTP_CLIENT
+  stub aria2c '** --summary-interval=* ** http://example.com/* : "'"$BASH"'" -ec '\''echo download-progress; "$FIXTURE_ROOT/fake_downloader" aria2c "$@"'\'' "$@"'
+
+  run run_with_script python-build "$FIXTURE_ROOT/definitions/without-checksum" "$INSTALL_ROOT"
+  assert_success
+  unstub aria2c
+  assert_line download-progress
+
+  run cat "$BATS_TEST_TMPDIR"/python-build.*.log
+  assert_success
+  assert_line download-progress
+}
+
 @test "fetching from git repository" {
   stub git "clone --depth 1 --branch master http://example.com/packages/package.git package-dev : mkdir package-dev"
 

plugins/python-build/test/fixtures/fake_downloader

@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+set -euo pipefail
+aria2c=
+wget=
+src=
+dest=
+dir_=
+
+case "$1" in
+  aria2c)
+    aria2c=1;;
+  wget)
+    wget=1;;
+esac
+while (($#)); do
+  case "$1" in
+    -o)
+      if [[ -z $wget ]]; then
+        dest="$2"
+        shift
+      fi
+      ;;
+    -O)
+      if [[ -n $wget ]]; then
+        dest="$2"
+        shift
+      fi
+      ;;
+    -d)
+      if [[ -n $aria2c ]]; then
+        dir_="$2/"
+        shift
+      fi
+      ;;
+    -*)
+      ;;
+    *)
+      src="$FIXTURE_ROOT/${1##*/}"
+      ;;
+  esac
+  shift
+done
+cp "$src" "${dir_}${dest}"
+

plugins/python-build/test/stubs/stub

@@ -25,6 +25,15 @@ eval "${_STUB_RESULT}"=0
 if test -e "${!_STUB_RUN}"; then source "${!_STUB_RUN}"; fi
 
 
+match_pattern() {
+  local argument="${1?}"
+  local pattern="${2:?}"
+  case "$argument" in
+    $pattern ) return 0;;
+    * ) return 1;;
+  esac
+}
+
 # Loop over each line in the plan.
 index=0
 while IFS= read -r line; do
@@ -53,14 +62,41 @@ while IFS= read -r line; do
 
     # Match the expected argument patterns to actual
     # arguments.
-    for (( i=0; i<${#patterns[@]}; i++ )); do
+    for (( i=0, j=0; i<${#patterns[@]}; i++, j++ )); do
       pattern="${patterns[$i]}"
-      argument="${arguments[$i]}"
 
-      case "$argument" in
+      # ** never backtracks to keep things simple -- so a command line
-        $pattern ) ;;
+      # may fail to match if the next pattern can match multiple times.
-        * ) result=1 ;;
+      # Assuming that we won't have such cases
-      esac
+      # since it's mostly intended to locate specific options
+      # and we do not have repeatable options as of this writing.
+      if [[ $pattern == "**" ]]; then
+        if ((i==${#patterns[@]}-1)); then
+          # no next pattern: ** and all patterns success
+          break;
+        fi
+        next_pattern="${patterns[i+1]}"
+        for (( fj=j; fj<${#arguments[@]}; fj++ )); do
+          if match_pattern "${arguments[fj]}" "$next_pattern"; then
+            # ** matched, the next pattern matched, too
+            # and they consumed arguments up to fj
+            i=$((i+1))
+            j=$fj
+            break
+          fi
+        done
+        if ((fj==${#arguments[@]})); then
+          # next pattern never matched
+          result=1; break
+        fi
+      else
+        if (( j<${#arguments[@]} )) && \
+            match_pattern "${arguments[j]}" "$pattern"; then
+          continue
+        else
+          result=1; break
+        fi
+      fi
     done
 
     # If the arguments matched, evaluate the command

plugins/python-build/test/test_helper.bash

@@ -138,6 +138,19 @@ assert_output_contains() {
   }
 }
 
+assert_line() {
+  if [ "$1" -ge 0 ] 2>/dev/null; then
+    assert_equal "$2" "${lines[$1]}"
+  else
+    local line
+    for line in "${lines[@]}"; do
+      if [ "$line" = "$1" ]; then return 0; fi
+    done
+    flunk "expected line \`$1'" $'\n'\
+    "output: $output"
+  fi
+}
+
 # Output a modified PATH that ensures that the given executable is not present,
 # but in which system utils necessary for pyenv operation are still available.
 path_without() {