From defd06b4b208ec3cf71908f2cc999bf8c880da04 Mon Sep 17 00:00:00 2001
From: Erica Portnoy <ebportnoy@gmail.com>
Date: Mon, 18 Dec 2017 14:51:41 -0800
Subject: [PATCH] address style, documentation, nits

---
 letsencrypt-auto-source/letsencrypt-auto.template     | 11 +++++------
 .../pieces/bootstrappers/rpm_common.sh                |  7 +++----
 .../pieces/bootstrappers/rpm_common_base.sh           |  8 +++++---
 .../pieces/bootstrappers/rpm_python3.sh               |  7 +++----
 4 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template
index 46ed5a1a4..4777213a7 100755
--- a/letsencrypt-auto-source/letsencrypt-auto.template
+++ b/letsencrypt-auto-source/letsencrypt-auto.template
@@ -249,15 +249,16 @@ DeterminePython2Version() {
   DeterminePythonVersionBase $1 "$LE_PYTHON python2.7 python27 python2 python"
 }
 
-# TODO: use this to not upgrade if we already have python3 installed
 DeterminePythonVersion() {
   # TODO: check that this is the only executable we might have installed
-  if [ "$USE_PYTHON_3" = true ] ; then
+  if [ "$USE_PYTHON_3" -eq 1 ] ; then
     DeterminePythonVersionBase $1 "$LE_PYTHON python3"
   else
     DeterminePython2Version $1
   fi
 
+# Sets LE_PYTHON to Python version string and PYVER to the first two
+# digits of the python version
 DeterminePythonVersionBase() {
   for LE_PYTHON in $2; do
     # Break (while keeping the LE_PYTHON value) if found.
@@ -279,7 +280,6 @@ DeterminePythonVersionBase() {
     error "This isn't going to work; you'll need at least version 2.6."
     exit 1
   fi
-  export PYVER
 }
 
 {{ bootstrappers/deb_common.sh }}
@@ -321,7 +321,7 @@ elif [ -f /etc/redhat-release ]; then
       BootstrapMessage "RedHat-based OSes that will use Python3"
       BootstrapRpmPython3
     }
-    USE_PYTHON_3=true
+    USE_PYTHON_3=1
     BOOTSTRAP_VERSION="BootstrapRpmPython3 $BOOTSTRAP_RPM_PYTHON3_VERSION"
   else
     Bootstrap() {
@@ -330,8 +330,7 @@ elif [ -f /etc/redhat-release ]; then
     }
     BOOTSTRAP_VERSION="BootstrapRpmCommon $BOOTSTRAP_RPM_COMMON_VERSION"
   fi
-  LE_PYTHON="$PREV_LE_PYTHON"
-  export LE_PYTHON
+  export LE_PYTHON="$PREV_LE_PYTHON"
 elif [ -f /etc/os-release ] && `grep -q openSUSE /etc/os-release` ; then
   Bootstrap() {
     BootstrapMessage "openSUSE-based OSes"
diff --git a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh
index 9e3bc85dc..68dafaa81 100755
--- a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh
+++ b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh
@@ -10,10 +10,9 @@ BootstrapRpmCommon() {
   #   - CentOS 6 (EPEL must be installed manually)
 
   FindInstallTool
-  tool=$?
 
   # Most RPM distros use the "python" or "python-" naming convention.  Let's try that first.
-  if $tool list python >/dev/null 2>&1; then
+  if $TOOL list python >/dev/null 2>&1; then
     python_pkgs="$python
       python-devel
       python-virtualenv
@@ -22,7 +21,7 @@ BootstrapRpmCommon() {
     "
   # Fedora 26 starts to use the prefix python2 for python2 based packages.
   # this elseif is theoretically for any Fedora over version 26:
-  elif $tool list python2 >/dev/null 2>&1; then
+  elif $TOOL list python2 >/dev/null 2>&1; then
     python_pkgs="$python2
       python2-libs
       python2-setuptools
@@ -42,5 +41,5 @@ BootstrapRpmCommon() {
     "
   fi
 
-  BootstrapRpmCommonBase $python_pkgs
+  BootstrapRpmCommonBase "$python_pkgs"
 }
diff --git a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common_base.sh b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common_base.sh
index 227d6a10f..465229dfe 100644
--- a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common_base.sh
+++ b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common_base.sh
@@ -1,13 +1,14 @@
 # If new packages are installed by BootstrapRpmCommonBase below, version
 # numbers in rpm_common.sh and rpm_python3.sh must be increased.
 
+# Sets TOOL to the name of the package manager
 FindInstallTool() {
   if type dnf 2>/dev/null
   then
-    tool=dnf
+    TOOL=dnf
   elif type yum 2>/dev/null
   then
-    tool=yum
+    TOOL=yum
 
   else
     error "Neither yum nor dnf found. Aborting bootstrap!"
@@ -16,8 +17,9 @@ FindInstallTool() {
 }
 
 BootstrapRpmCommonBase() {
+  # Arguments: whitespace-delimited python packages to install
+
   FindInstallTool
-  tool=$?
 
   if [ "$ASSUME_YES" = 1 ]; then
     yes_flag="-y"
diff --git a/letsencrypt-auto-source/pieces/bootstrappers/rpm_python3.sh b/letsencrypt-auto-source/pieces/bootstrappers/rpm_python3.sh
index e918450b4..40698c9d6 100644
--- a/letsencrypt-auto-source/pieces/bootstrappers/rpm_python3.sh
+++ b/letsencrypt-auto-source/pieces/bootstrappers/rpm_python3.sh
@@ -7,10 +7,9 @@ BootstrapRpmPython3() {
   #   - CentOS 6 (EPEL must be installed manually)
 
   FindInstallTool
-  tool=$?
 
   # EPEL uses python34
-  if $tool list python34 >/dev/null 2>&1; then
+  if $TOOL list python34 >/dev/null 2>&1; then
     python_pkgs="python34
       python34-devel
       python-virtualenv
@@ -21,5 +20,5 @@ BootstrapRpmPython3() {
   # TODO: add some elifs and elses for other distros that
   # might have 2.6 or no python installed to get here
 
-  BootstrapRpmCommonBase $python_pkgs
-}
\ No newline at end of file
+  BootstrapRpmCommonBase "$python_pkgs"
+}