fix(install-status): Correctly handle failed installation of apps (Sc…

…oopInstaller#4676)
CyberFlameGO · Jan 25, 2022 · 158c0fd · 158c0fd
1 parent e09127f
commit 158c0fd
Show file tree

Hide file tree

Showing 10 changed files with 115 additions and 116 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@
 - **decompress:** Fix nested Zstd archive extraction ([#4608](https://github.com/ScoopInstaller/Scoop/issues/4608))
   - **decompress:** Fix `Split-Path -LeafBase` in PS5 ([#4639](https://github.com/ScoopInstaller/Scoop/issues/4639))
 - **installed:** Fix 'core/installed' that mark failed app as 'installed' ([#4650](https://github.com/ScoopInstaller/Scoop/issues/4650))
+  - **status:** Correctly handle failed installation of apps ([#4676](https://github.com/ScoopInstaller/Scoop/issues/4676))
 - **shim:** Fix PS1 shim error when in different drive in PS7 ([#4614](https://github.com/ScoopInstaller/Scoop/issues/4614))
 - **shim:** Fix `sh` shim error in WSL ([#4637](https://github.com/ScoopInstaller/Scoop/issues/4637))
 - **versions:** Fix wrong version number when only one version dir ([#4679](https://github.com/ScoopInstaller/Scoop/issues/4679))

diff --git a/lib/core.ps1 b/lib/core.ps1
@@ -190,20 +190,20 @@ function installed($app, $global) {
     # "dependency". So we need to extract the bucket from the name and only give the app
     # name to is_directory
     $app = ($app -split '/|\\')[-1]
-    if (get_config NO_JUNCTIONS) {
-        $installedVersion = Select-CurrentVersion -AppName $app -Global:$global
-    } else {
-        $installedVersion = 'current'
-    }
-    return Join-Path (versiondir $app $installedVersion $global) 'install.json' | Test-Path
+    return $null -ne (Select-CurrentVersion -AppName $app -Global:$global)
 }
 function installed_apps($global) {
     $dir = appsdir $global
-    if(test-path $dir) {
+    if (Test-Path $dir) {
         Get-ChildItem $dir | Where-Object { $_.psiscontainer -and $_.name -ne 'scoop' } | ForEach-Object { $_.name }
     }
 }
 
+# check whether the app failed to install
+function failed($app, $global) {
+    return (is_directory (appdir $app $global)) -and !(Select-CurrentVersion -AppName $app -Global:$global)
+}
+
 function file_path($app, $file) {
     Show-DeprecatedWarning $MyInvocation 'Get-AppFilePath'
     Get-AppFilePath -App $app -File $file
@@ -222,13 +222,13 @@ function Get-AppFilePath {
 
     # normal path to file
     $Path = "$(versiondir $App 'current' $false)\$File"
-    if(Test-Path $Path) {
+    if (Test-Path $Path) {
         return $Path
     }
 
     # global path to file
     $Path = "$(versiondir $App 'current' $true)\$File"
-    if(Test-Path $Path) {
+    if (Test-Path $Path) {
         return $Path
     }
 
@@ -256,15 +256,15 @@ function Get-HelperPath {
     switch ($Helper) {
         '7zip' {
             $HelperPath = Get-AppFilePath '7zip' '7z.exe'
-            if([String]::IsNullOrEmpty($HelperPath)) {
+            if ([String]::IsNullOrEmpty($HelperPath)) {
                 $HelperPath = Get-AppFilePath '7zip-zstd' '7z.exe'
             }
         }
         'Lessmsi' { $HelperPath = Get-AppFilePath 'lessmsi' 'lessmsi.exe' }
         'Innounp' { $HelperPath = Get-AppFilePath 'innounp' 'innounp.exe' }
         'Dark' {
             $HelperPath = Get-AppFilePath 'dark' 'dark.exe'
-            if([String]::IsNullOrEmpty($HelperPath)) {
+            if ([String]::IsNullOrEmpty($HelperPath)) {
                 $HelperPath = Get-AppFilePath 'wixtoolset' 'dark.exe'
             }
         }
@@ -293,13 +293,13 @@ function Test-Aria2Enabled {
 
 function app_status($app, $global) {
     $status = @{}
-    $status.installed = (installed $app $global)
+    $status.installed = installed $app $global
     $status.version = Select-CurrentVersion -AppName $app -Global:$global
     $status.latest_version = $status.version
 
     $install_info = install_info $app $status.version $global
 
-    $status.failed = (!$install_info -or !$status.version)
+    $status.failed = failed $app $global
     $status.hold = ($install_info.hold -eq $true)
 
     $manifest = manifest $app $install_info.bucket $install_info.url
@@ -319,8 +319,12 @@ function app_status($app, $global) {
 
     $status.missing_deps = @()
     $deps = @($manifest.depends) | Where-Object {
-        $app, $bucket, $null = parse_app $_
-        return !(installed $app)
+        if ($null -eq $_) {
+            return $null
+        } else {
+            $app, $bucket, $null = parse_app $_
+            return !(installed $app)
+        }
     }
     if ($deps) {
         $status.missing_deps += , $deps
@@ -739,6 +743,7 @@ function ensure_architecture($architecture_opt) {
 
 function Confirm-InstallationStatus {
     [CmdletBinding()]
+    [OutputType([Object[]])]
     param(
         [Parameter(Mandatory = $true)]
         [String[]]
@@ -750,26 +755,29 @@ function Confirm-InstallationStatus {
     $Apps | Select-Object -Unique | Where-Object { $_.Name -ne 'scoop' } | ForEach-Object {
         $App, $null, $null = parse_app $_
         if ($Global) {
-            if (installed $App $true) {
-                $Installed += ,@($App, $true)
-            } elseif (installed $App $false) {
-                error "'$App' isn't installed globally, but it is installed for your account."
+            if (Test-Path (appdir $App $true)) {
+                $Installed += , @($App, $true)
+            } elseif (Test-Path (appdir $App $false)) {
+                error "'$App' isn't installed globally, but it may be installed locally."
                 warn "Try again without the --global (or -g) flag instead."
             } else {
                 error "'$App' isn't installed."
             }
         } else {
-            if(installed $App $false) {
-                $Installed += ,@($App, $false)
-            } elseif (installed $App $true) {
-                error "'$App' isn't installed for your account, but it is installed globally."
+            if (Test-Path (appdir $App $false)) {
+                $Installed += , @($App, $false)
+            } elseif (Test-Path (appdir $App $true)) {
+                error "'$App' isn't installed locally, but it may be installed globally."
                 warn "Try again with the --global (or -g) flag instead."
             } else {
                 error "'$App' isn't installed."
             }
         }
+        if (failed $App $Global) {
+            error "'$App' isn't installed correctly."
+        }
     }
-    return ,$Installed
+    return , $Installed
 }
 
 function strip_path($orig_path, $dir) {

diff --git a/lib/install.ps1 b/lib/install.ps1
@@ -1077,17 +1077,9 @@ function prune_installed($apps, $global) {
     return @($uninstalled), @($installed)
 }
 
-# check whether the app failed to install
-function failed($app, $global) {
-    if (is_directory (appdir $app $global)) {
-        return !(install_info $app (Select-CurrentVersion -AppName $app -Global:$global) $global)
-    } else {
-        return $false
-    }
-}
-
 function ensure_none_failed($apps, $global) {
-    foreach($app in $apps) {
+    foreach ($app in $apps) {
+        $app = ($app -split '/|\\')[-1] -replace '\.json$', ''
         if (failed $app $global) {
             warn "Purging previous failed installation of $app."
             & "$PSScriptRoot\..\libexec\scoop-uninstall.ps1" $app$(if ($global) { ' --global' })

diff --git a/lib/versions.ps1 b/lib/versions.ps1
@@ -51,18 +51,18 @@ function Select-CurrentVersion {
     )
     process {
         $appPath = appdir $AppName $Global
-        if (Test-Path "$appPath\current" -PathType Container) {
-            $currentVersion = (installed_manifest $AppName 'current' $Global).version
-            if ($currentVersion -eq 'nightly') {
-                $currentVersion = (Get-Item "$appPath\current").Target | Split-Path -Leaf
-            }
-        } else {
+        if (get_config NO_JUNCTIONS) {
             $installedVersion = Get-InstalledVersion -AppName $AppName -Global:$Global
             if ($installedVersion) {
                 $currentVersion = @($installedVersion)[-1]
             } else {
                 $currentVersion = $null
             }
+        } else {
+            $currentVersion = (installed_manifest $AppName 'current' $Global).version
+            if ($currentVersion -eq 'nightly') {
+                $currentVersion = (Get-Item "$appPath\current").Target | Split-Path -Leaf
+            }
         }
         return $currentVersion
     }

diff --git a/libexec/scoop-cleanup.ps1 b/libexec/scoop-cleanup.ps1
@@ -37,11 +37,7 @@ function cleanup($app, $global, $verbose, $cache) {
     }
     $appDir = appdir $app $global
     $versions = Get-ChildItem $appDir -Name
-    if (!$versions) {
-        Remove-Item $appDir -ErrorAction SilentlyContinue -Force
-        return
-    }
-    $versions = $versions | Where-Object { $_ -ne $current_version -and $_ -ne 'current' }
+    $versions = $versions | Where-Object { $current_version -ne $_ -and $_ -ne 'current' }
     if (!$versions) {
         if ($verbose) { success "$app is already clean" }
         return
@@ -56,8 +52,14 @@ function cleanup($app, $global, $verbose, $cache) {
         unlink_persist_data $dir
         Remove-Item $dir -ErrorAction Stop -Recurse -Force
     }
-    if (!(Get-ChildItem $appDir)) {
-        Remove-Item $appDir -ErrorAction SilentlyContinue -Force
+    $leftVersions = Get-ChildItem $appDir
+    if ($leftVersions.Length -eq 1 -and $leftVersions.Name -eq 'current' -and $leftVersions.LinkType) {
+        attrib $leftVersions.FullName -R /L
+        Remove-Item $leftVersions.FullName -ErrorAction Stop -Force
+        $leftVersions = $null
+    }
+    if (!$leftVersions) {
+        Remove-Item $appDir -ErrorAction Stop -Force
     }
     Write-Host ''
 }

diff --git a/libexec/scoop-hold.ps1 b/libexec/scoop-hold.ps1
@@ -3,6 +3,7 @@
 
 . "$psscriptroot\..\lib\help.ps1"
 . "$psscriptroot\..\lib\manifest.ps1"
+. "$psscriptroot\..\lib\versions.ps1"
 
 reset_aliases
 $apps = $args

diff --git a/libexec/scoop-install.ps1 b/libexec/scoop-install.ps1
@@ -31,25 +31,6 @@
 
 reset_aliases
 
-function is_installed($app, $global) {
-    if ($app.EndsWith('.json')) {
-        $app = [System.IO.Path]::GetFileNameWithoutExtension($app)
-    }
-    if (installed $app $global) {
-        function gf($g) { if ($g) { ' --global' } }
-
-        $version = Select-CurrentVersion -AppName $app -Global:$global
-        if (!(install_info $app $version $global)) {
-            warn "Purging previous failed installation of $app."
-            & "$PSScriptRoot\scoop-uninstall.ps1" $app$(gf $global)
-            return $false
-        }
-        warn "'$app' ($version) is already installed.`nUse 'scoop update $app$(gf $global)' to install a new version."
-        return $true
-    }
-    return $false
-}
-
 $opt, $apps, $err = getopt $args 'gikusa:' 'global', 'independent', 'no-cache', 'no-update-scoop', 'skip', 'arch='
 if ($err) { "scoop install: $err"; exit 1 }
 
@@ -80,8 +61,13 @@ if (is_scoop_outdated) {
 
 if ($apps.length -eq 1) {
     $app, $null, $version = parse_app $apps
-    if ($null -eq $version -and (is_installed $app $global)) {
-        return
+    if ($app.EndsWith('.json')) {
+        $app = [System.IO.Path]::GetFileNameWithoutExtension($app)
+    }
+    $curVersion = Select-CurrentVersion -AppName $app -Global:$global
+    if ($null -eq $version -and $curVersion) {
+        warn "'$app' ($curVersion) is already installed.`nUse 'scoop update $app$(if ($global) { ' --global' })' to install a new version."
+        exit 0
     }
 }
 

diff --git a/libexec/scoop-unhold.ps1 b/libexec/scoop-unhold.ps1
@@ -3,6 +3,7 @@
 
 . "$psscriptroot\..\lib\help.ps1"
 . "$psscriptroot\..\lib\manifest.ps1"
+. "$psscriptroot\..\lib\versions.ps1"
 
 reset_aliases
 $apps = $args