Make install scripts more consistent over different operating systems (…

…PowerShell#9071) I noticed a couple of inconsistencies when reading through the install bash scripts. - Make documentation for switches consistent over all files. - Replace all `sed` implementations of `lowercase` with a more maintainable `tr` implementation. - Set the `OS` variable in every install script, making it so previously unused checks are actually used. - Exit with a non-zero exit code when the script reaches an illegal state. ## PR Context A lot of people, including myself, read the install scripts before executing them. While doing so I noticed they contained inconsistencies. For example, certain flags you can pass to the install script were either undocumented or had an incorrect description. This PR fixes some of these inconsistencies, which should make them easier to maintain and easier to read. Co-authored-by: Travis Plunk <github@ez13.net>
todun · Mar 8, 2019 · 7031954 · 7031954
1 parent b23e835
commit 7031954
Show file tree

Hide file tree

Showing 11 changed files with 173 additions and 189 deletions.
diff --git a/.vsts-ci/install-ps.yml b/.vsts-ci/install-ps.yml
@@ -44,6 +44,19 @@ phases:
     jobName: InstallPowerShellUbuntu
     pool: Hosted Ubuntu 1604
 
+# amazonlinux was missing adduser
+
+- template: templates/install-ps-phase.yml
+  parameters:
+    scriptName: ./tools/install-powershell.sh
+    jobName: InstallPowerShellCentOS
+    pool: Hosted Ubuntu 1604
+    container: mcr.microsoft.com/powershell:centos-7
+
+# VSTS could not find pwsh in:
+# mcr.microsoft.com/powershell:opensuse-42.3
+# could not repo locally
+
 - template: templates/install-ps-phase.yml
   parameters:
     scriptName: ./tools/install-powershell.sh

diff --git a/.vsts-ci/templates/install-ps-phase.yml b/.vsts-ci/templates/install-ps-phase.yml
@@ -2,14 +2,18 @@ parameters:
   pool: 'Hosted Ubuntu 1604'
   jobName: 'none'
   scriptName: ''
+  container: ''
 
 jobs:
+
 - job: ${{ parameters.jobName }}
   variables:
     scriptName: ${{ parameters.scriptName }}
 
-  pool:
-    name: ${{ parameters.pool }}
+  ${{ if ne(parameters.container, '') }}:
+    container: ${{ parameters.container }}
+
+    pool: ${{ parameters.pool }}
 
   displayName: ${{ parameters.jobName }}
 

diff --git a/tools/Xml/Xml.psm1 b/tools/Xml/Xml.psm1
@@ -51,7 +51,7 @@ function New-XmlElement
     }
 }
 
-# Removes an XmlElement and it's parent if it is empty
+# Removes an XmlElement and its parent if it is empty
 function Remove-XmlElement
 {
     param(

diff --git a/tools/install-powershell-readme.md b/tools/install-powershell-readme.md
@@ -21,10 +21,11 @@
 
 ## Parameters
 
-* -includeide - installs vscode and vscode powershell extension (only relevant to machines with desktop environment)
-* -interactivetesting - do a quick launch test of vscode - only relevant when used with -includeide
-* -skip-sudo-check - use sudo without verifying it's availability (hard to accurately do on some distros)
-* -preview - installs preview of powershell core side-by-side with any existing production releases.
+* -includeide - installs VSCode and VSCode PowerShell extension (only relevant to machines with a desktop environment)
+* -interactivetesting - do a quick launch test of VSCode (only relevant when used with -includeide)
+* -skip-sudo-check - use sudo without verifying its availability (hard to accurately do on some distros)
+* -preview - installs the latest preview release of PowerShell core side-by-side with any existing production releases
+* -appimage - perform an AppImage install instead of a native install
 
 ## Usage
 

diff --git a/tools/install-powershell.sh b/tools/install-powershell.sh
@@ -18,9 +18,11 @@ install(){
     #Completely automated install requires a root account or sudo with a password requirement
 
     #Switches
-    #  -includeide - the script is being run headless, do not perform actions that require response from the console
-    #  -interactivetesting - requires a human user in front of the machine - loads a script into the ide to test with F5 to ensure the IDE can run scripts
-    #  -appimage - does appimage instead of native install
+    # -includeide         - installs VSCode and VSCode PowerShell extension (only relevant to machines with desktop environment)
+    # -interactivetesting - do a quick launch test of VSCode (only relevant when used with -includeide)
+    # -skip-sudo-check    - use sudo without verifying its availability (hard to accurately do on some distros)
+    # -preview            - installs the latest preview release of PowerShell core side-by-side with any existing production releases
+    # -appimage           - perform an AppImage install instead of a native install
 
     #gitrepo paths are overrideable to run from your own fork or branch for testing or private distribution
 
@@ -31,7 +33,7 @@ install(){
 
     echo "Get-PowerShell Core MASTER Installer Version $VERSION"
     echo "Installs PowerShell Core and Optional The Development Environment"
-    echo "  Original script is at: $gitreposcriptroot\$gitscriptname"
+    echo "  Original script is at: $gitreposcriptroot\\$gitscriptname"
 
     echo "Arguments used: $*"
     echo ""
@@ -44,12 +46,15 @@ install(){
     ' INT
 
     lowercase(){
-        echo "$1" | sed "y/ABCDEFGHIJKLMNOPQRSTUVWXYZ/abcdefghijklmnopqrstuvwxyz/"
+        echo "$1" | tr "[:upper:]" "[:lower:]"
     }
 
-    local OS=`lowercase \`uname\``
-    local KERNEL=`uname -r`
-    local MACH=`uname -m`
+    local OS
+    OS=$(lowercase "$(uname)")
+    local KERNEL
+    KERNEL=$(uname -r)
+    local MACH
+    MACH=$(uname -m)
     local DIST
     local DistroBasedOn
     local PSUEDONAME
@@ -58,57 +63,57 @@ install(){
     if [ "${OS}" == "windowsnt" ]; then
         OS=windows
         DistroBasedOn=windows
-        SCRIPTFOLDER=$(dirname $(readlink -f $0))
+        SCRIPTFOLDER=$(dirname "$(readlink -f "$0")")
     elif [ "${OS}" == "darwin" ]; then
         OS=osx
         DistroBasedOn=osx
         # readlink doesn't work the same on macOS
-        SCRIPTFOLDER=$(dirname $0)
+        SCRIPTFOLDER=$(dirname "$0")
     else
-        SCRIPTFOLDER=$(dirname $(readlink -f $0))
-        OS=`uname`
+        SCRIPTFOLDER=$(dirname "$(readlink -f "$0")")
+        OS=$(uname)
         if [ "${OS}" == "SunOS" ] ; then
             OS=solaris
-            ARCH=`uname -p`
-            OSSTR="${OS} ${REV}(${ARCH} `uname -v`)"
+            ARCH=$(uname -p)
+            OSSTR="${OS} ${REV}(${ARCH} $(uname -v))"
             DistroBasedOn=sunos
         elif [ "${OS}" == "AIX" ] ; then
-            OSSTR="${OS} `oslevel` (`oslevel -r`)"
+            OSSTR="${OS} $(oslevel) ($(oslevel -r))"
             DistroBasedOn=aix
         elif [ "${OS}" == "Linux" ] ; then
             if [ -f /etc/redhat-release ] ; then
                 DistroBasedOn='redhat'
-                DIST=`cat /etc/redhat-release |sed s/\ release.*//`
-                PSUEDONAME=`cat /etc/redhat-release | sed s/.*\(// | sed s/\)//`
-                REV=`cat /etc/redhat-release | sed s/.*release\ // | sed s/\ .*//`
+                DIST=$(sed s/\ release.*// < /etc/redhat-release)
+                PSUEDONAME=$( (sed s/.*\(// | sed s/\)//) < /etc/redhat-release )
+                REV=$( (sed s/.*release\ // | sed s/\ .*//) < /etc/redhat-release )
             elif [ -f /etc/system-release ] ; then
-                DIST=`cat /etc/system-release |sed s/\ release.*//`
-                PSUEDONAME=`cat /etc/system-release | sed s/.*\(// | sed s/\)//`
-                REV=`cat /etc/system-release | sed s/.*release\ // | sed s/\ .*//`
+                DIST=$(sed s/\ release.*// < /etc/system-release)
+                PSUEDONAME=$( (sed s/.*\(// | sed s/\)//) < /etc/system-release )
+                REV=$( (sed s/.*release\ // | sed s/\ .*//) < /etc/system-release )
                 if [[ $DIST == *"Amazon Linux"* ]] ; then
                     DistroBasedOn='amazonlinux'
                 else
                     DistroBasedOn='redhat'
                 fi
             elif [ -f /etc/SuSE-release ] ; then
                 DistroBasedOn='suse'
-                PSUEDONAME=`cat /etc/SuSE-release | tr "\n" ' '| sed s/VERSION.*//`
-                REV=`cat /etc/SuSE-release | grep 'VERSION' | sed s/.*=\ //`
+                PSUEDONAME=$( (tr "\n" ' '| sed s/VERSION.*//) < /etc/SuSE-release )
+                REV=$( (grep 'VERSION' | sed s/.*=\ //) < /etc/SuSE-release )
             elif [ -f /etc/mandrake-release ] ; then
                 DistroBasedOn='mandrake'
-                PSUEDONAME=`cat /etc/mandrake-release | sed s/.*\(// | sed s/\)//`
-                REV=`cat /etc/mandrake-release | sed s/.*release\ // | sed s/\ .*//`
+                PSUEDONAME=$( (sed s/.*\(// | sed s/\)//) < /etc/mandrake-release )
+                REV=$( (sed s/.*release\ // | sed s/\ .*//) < /etc/mandrake-release )
             elif [ -f /etc/debian_version ] ; then
                 DistroBasedOn='debian'
-                DIST=`cat /etc/lsb-release | grep '^DISTRIB_ID' | awk -F=  '{ print $2 }'`
-                PSUEDONAME=`cat /etc/lsb-release | grep '^DISTRIB_CODENAME' | awk -F=  '{ print $2 }'`
-                REV=`cat /etc/lsb-release | grep '^DISTRIB_RELEASE' | awk -F=  '{ print $2 }'`
+                DIST=$( (grep '^DISTRIB_ID' | awk -F=  '{ print $2 }') < /etc/lsb-release )
+                PSUEDONAME=$( (grep '^DISTRIB_CODENAME' | awk -F=  '{ print $2 }') < /etc/lsb-release )
+                REV=$( (grep '^DISTRIB_RELEASE' | awk -F=  '{ print $2 }') < /etc/lsb-release)
             fi
             if [ -f /etc/UnitedLinux-release ] ; then
-                DIST="${DIST}[`cat /etc/UnitedLinux-release | tr "\n" ' ' | sed s/VERSION.*//`]"
+                DIST="${DIST}[$( (tr "\n" ' ' | sed s/VERSION.*//) < /etc/UnitedLinux-release )]"
             fi
-            OS=`lowercase $OS`
-            DistroBasedOn=`lowercase $DistroBasedOn`
+            OS=$(lowercase $OS)
+            DistroBasedOn=$(lowercase $DistroBasedOn)
         fi
     fi
 
@@ -120,42 +125,45 @@ install(){
     echo "  REV: $REV"
     echo "  KERNEL: $KERNEL"
     echo "  MACH: $MACH"
+    echo "  OSSTR: $OSSTR"
 
 
 
     if [[ "'$*'" =~ appimage ]] ; then
-        if [ -f $SCRIPTFOLDER/appimage.sh ]; then
+        if [ -f "$SCRIPTFOLDER/appimage.sh" ]; then
             #Script files were copied local - use them
-            . $SCRIPTFOLDER/appimage.sh
+            # shellcheck source=/dev/null
+            . "$SCRIPTFOLDER/appimage.sh"
         else
             #Script files are not local - pull from remote
             echo "Could not find \"appimage.sh\" next to this script..."
             echo "Pulling and executing it from \"$gitreposcriptroot/appimage.sh\""
             if [ -n "$(command -v curl)" ]; then
                 echo "found and using curl"
-                bash <(curl -s $gitreposcriptroot/appimage.sh) $@
+                bash <(curl -s $gitreposcriptroot/appimage.sh) "$@"
             elif [ -n "$(command -v wget)" ]; then
                 echo "found and using wget"
-                bash <(wget -qO- $gitreposcriptroot/appimage.sh) $@
+                bash <(wget -qO- $gitreposcriptroot/appimage.sh) "$@"
             else
                 echo "Could not find curl or wget, install one of these or manually download \"$gitreposcriptroot/appimage.sh\""
             fi
         fi
     elif [ "$DistroBasedOn" == "redhat" ] || [ "$DistroBasedOn" == "debian" ] || [ "$DistroBasedOn" == "osx" ] || [ "$DistroBasedOn" == "suse" ] || [ "$DistroBasedOn" == "amazonlinux" ]; then
         echo "Configuring PowerShell Core Environment for: $DistroBasedOn $DIST $REV"
-        if [ -f $SCRIPTFOLDER/installpsh-$DistroBasedOn.sh ]; then
+        if [ -f "$SCRIPTFOLDER/installpsh-$DistroBasedOn.sh" ]; then
             #Script files were copied local - use them
-            . $SCRIPTFOLDER/installpsh-$DistroBasedOn.sh
+            # shellcheck source=/dev/null
+            . "$SCRIPTFOLDER/installpsh-$DistroBasedOn.sh"
         else
             #Script files are not local - pull from remote
             echo "Could not find \"installpsh-$DistroBasedOn.sh\" next to this script..."
             echo "Pulling and executing it from \"$gitreposcriptroot/installpsh-$DistroBasedOn.sh\""
             if [ -n "$(command -v curl)" ]; then
                 echo "found and using curl"
-                bash <(curl -s $gitreposcriptroot/installpsh-$DistroBasedOn.sh) $@
+                bash <(curl -s $gitreposcriptroot/installpsh-"$DistroBasedOn".sh) "$@"
             elif [ -n "$(command -v wget)" ]; then
                 echo "found and using wget"
-                bash <(wget -qO- $gitreposcriptroot/installpsh-$DistroBasedOn.sh) $@
+                bash <(wget -qO- $gitreposcriptroot/installpsh-"$DistroBasedOn".sh) "$@"
             else
                 echo "Could not find curl or wget, install one of these or manually download \"$gitreposcriptroot/installpsh-$DistroBasedOn.sh\""
             fi
@@ -166,4 +174,4 @@ install(){
 }
 
 # run the install function
-install;
+install "$@";