Skip to content

fix(install-status): Correctly handle failed installation of apps#4676

Merged
niheaven merged 11 commits intoScoopInstaller:developfrom
niheaven:fix-install-status
Jan 25, 2022
Merged

fix(install-status): Correctly handle failed installation of apps#4676
niheaven merged 11 commits intoScoopInstaller:developfrom
niheaven:fix-install-status

Conversation

@niheaven
Copy link
Copy Markdown
Member

@niheaven niheaven commented Jan 20, 2022

Description

  • Let core/Confirm-InstallationStatus() marks failed installation and give a warning
  • Let scoop-cleanup correctly cleanup failed installation
  • Simplify core/installed() and install/failed() (oh xxxx these two paired functions even are not in same file! Move them to core)
  • Remove scoop-install/is_installed() and move its functionality into main file
  • Some minor style fixes

Motivation and Context

Closes #4674

Relates to #4650

How Has This Been Tested?

Just as #4674

# 1. failed installation can be purged via `scoop-cleanup`:
scoop install tests\meson.json
scoop list meson
#  meson  *failed*
scoop cleanup *
# Removing meson: 0.61.1
# Everything is shiny now!
scoop list meson
# Nothing here

# 2. previous failed installation was purged when (re)install:
scoop install tests\meson.json
scoop list meson
#  meson  *failed*
scoop install meson.json
# WARN  Purging previous failed installation of meson.
# WARN  'meson' isn't installed correctly.
# Uninstalling 'meson' ().
# 'meson' was uninstalled.
# 'meson' (0.61.1) was installed successfully!
scoop list meson
#  meson 0.61.1 [main]
ls (scoop prefix meson) | findstr 'FAILED_INSTALLATION'
# Nothing here

Checklist:

  • I have read the Contributing Guide.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly. (I don't know how to test such functions)

@niheaven niheaven changed the title fix(install-status): Mark failed installation as installed and warn fix(install-status): Correctly handle failed installation of apps Jan 20, 2022
@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 20, 2022

screenshot_20220120130802

#1, passed.

#2, as the screenshot above, no previous files were purged, just like fresh install.

@niheaven
Copy link
Copy Markdown
Member Author

niheaven commented Jan 20, 2022

It's very strange here, the following is running result in my PC (HEAD is 64c2333):

image

@niheaven
Copy link
Copy Markdown
Member Author

Okay, found it... It's the main before meson make the issue. I'll check again.

@niheaven
Copy link
Copy Markdown
Member Author

@HUMORCE Please test again using ee2f3b0

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 20, 2022

PS C:\Users\HUMOR\Desktop\test> scoop install .\v1\meson.json
Installing 'meson' (0.61.1) [64bit]
meson-0.61.1-64.msi (9.9 MB) [================================================================================] 100%
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
PS C:\Users\HUMOR\Desktop\test> scoop list meson
Installed apps matching 'meson':

  meson  *failed*

PS C:\Users\HUMOR\Desktop\test> scoop install main/meson
WARN  Purging previous failed installation of main/meson.
WARN  'meson' isn't installed correctly.
Uninstalling 'meson' ().
'meson' was uninstalled.
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
'meson' (0.61.1) was installed successfully!
'meson' suggests installing 'ninja'.
PS C:\Users\HUMOR\Desktop\test> scoop list meson
Installed apps matching 'meson':

  meson 0.61.1 [main]

PS C:\Users\HUMOR\Desktop\test> ls (scoop prefix meson) | findstr 'FAILED_INSTALLATION'
PS C:\Users\HUMOR\Desktop\test> scoop -v
Current Scoop version:
ee2f3b04 (HEAD -> fix-install-status, origin/fix-install-status) fix(failed): Fix 'failed()' that doesn't recognize app name with bucket

works fine now

WARN  Purging previous failed installation of main/meson.          <<< a minor flaw
WARN  'meson' isn't installed correctly.

when re-testing above WARN output msg, reproduced:

PS C:\Users\HUMOR\Desktop\test> scoop install .\v1\meson.json
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
PS C:\Users\HUMOR\Desktop\test> scoop list meson
Installed apps matching 'meson':

  meson  *failed*

PS C:\Users\HUMOR\Desktop\test> scoop install ~\scoop\buckets\main\bucket\meson.json
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
'meson' (0.61.1) was installed successfully!
'meson' suggests installing 'ninja'.
PS C:\Users\HUMOR\Desktop\test> ls (scoop prefix meson) | findstr 'FAILED_INSTALLATION'
-a----         1/20/2022   3:28 PM              0 FAILED_INSTALLATION

@niheaven
Copy link
Copy Markdown
Member Author

niheaven commented Jan 20, 2022

scoop install ~\scoop\buckets\main\bucket\meson.json

That is, when you install a local manifest with the same name, the bug is happened again?

@niheaven
Copy link
Copy Markdown
Member Author

Local manifest or file app is treated incorrectly. Please test again, thanks, I've never met so many error scenarios.

image

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 20, 2022

  1. tested according to the steps in the report: PASSED
PS C:\Users\HUMOR\Desktop\test> scoop install .\v1\meson.json
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
PS C:\Users\HUMOR\Desktop\test> scoop install .\v2\meson.json
WARN  Purging previous failed installation of meson.
WARN  'meson' isn't installed correctly.
Uninstalling 'meson' ().
'meson' was uninstalled.
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
'meson' (0.61.1) was installed successfully!
'meson' suggests installing 'ninja'.
PS C:\Users\HUMOR\Desktop\test> ls (scoop prefix meson) | findstr 'FAILED_INSTALLATION'
PS C:\Users\HUMOR\Desktop\test>
  1. tested with force update: ISSUE STILL
PS C:\Users\HUMOR\Desktop\test> scoop install .\v1\meson.json
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
PS C:\Users\HUMOR\Desktop\test> scoop update meson
WARN  'meson' isn't installed correctly.
meson:  (latest version)
Latest versions for all apps are installed! For more information try 'scoop status'
PS C:\Users\HUMOR\Desktop\test> scoop update meson -f
WARN  'meson' isn't installed correctly.
meson:  -> 0.61.1
Updating one outdated app:
Updating 'meson' ( -> 0.61.1)
Downloading new version
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Uninstalling 'meson' ()
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Extracting meson-0.61.1-64.msi ... done.
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
'meson' (0.61.1) was installed successfully!
PS C:\Users\HUMOR\Desktop\test> ls (scoop prefix meson) | findstr 'FAILED_INSTALLATION'
-a----         1/20/2022   3:58 PM              0 FAILED_INSTALLATION
  1. tested with a correct manifest that added to the bucket: PASSED
PS C:\Users\HUMOR\Desktop\test> scoop install .\v1\meson.json
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
PS C:\Users\HUMOR\Desktop\test> scoop install meson
WARN  Purging previous failed installation of meson.
WARN  'meson' isn't installed correctly.
Uninstalling 'meson' ().
'meson' was uninstalled.
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
'meson' (0.61.1) was installed successfully!
'meson' suggests installing 'ninja'.
PS C:\Users\HUMOR\Desktop\test> ls (scoop prefix meson) | findstr 'FAILED_INSTALLATION'
  1. re)install via url not tested yet

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 20, 2022

the result should be same, if the app(manifest)name are extracted correctly from any sources. IMO

  • app
  • main/app
  • .\dir\app.json
  • dir\app.json
  • $HOME\app.json
  • http|ftp://exam.ple/app.json
  • etc..

or maybe just me did not have a complete view of the source code 😂

thanks for your work!👍

@niheaven
Copy link
Copy Markdown
Member Author

niheaven commented Jan 20, 2022

  1. tested with force update: ISSUE STILL

The update or force update should exit early if the original installation is broken, IMO. That's not 'cos the code I fixed here, but scoop-update/update()... Just add an early return will fix this. Some fix on this and updating progress interruption by a broken installation will be fixed in another PR. <- This is some problem about abort() function, and need some time to investigate.

Except that, this should be done, right? 😄

@niheaven
Copy link
Copy Markdown
Member Author

niheaven commented Jan 20, 2022

the result should be same, if the app(manifest)name are extracted correctly from any sources. IMO

  • app
  • main/app
  • .\dir\app.json
  • dir\app.json
  • $HOME\app.json
  • http|ftp://exam.ple/app.json
  • etc..

$app = ($app -split '/|\\')[-1] -replace '\.json$', '' in ensure_none_failed() should extract correct vanilla app name for all acceptable app name (bucket/app or local/remote json).

@niheaven
Copy link
Copy Markdown
Member Author

The output is a little messy, I'll find a way to simplify it.

image

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 22, 2022

sure, just some other cases considered by the way. ping me, any progress

@niheaven
Copy link
Copy Markdown
Member Author

@HUMORCE I've change the logic of treating failed installation's uninstallation and cleanup and hadn't found more bug. If this one is suitable for you, I'll merge it as soon as possible since it is REALLY a bug that introduced by #4650

image
image

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 22, 2022

the manifests(v1|v2\meson.json) consistent with the original reported.

PASSED:

> scoop install .\v1\meson.json
> scoop list meson
  meson  *failed*
> scoop install .\v2\meson.json // OR main/meson
> ls (scoop prefix meson) | findstr 'FAILED_INSTALLATION'
> scoop list meson
  meson 0.61.1 [C:\Users\HUMOR\Desktop\test\v2\meson.json] // meson 0.61.1 [main]

That app is NOT installed, so the list and uninstall output are fine, IMO. I'll add some logic in scoop-cleanup to cleanup these false installed apps. #4648 (comment)

but scoop-cleanup cannot purge the failed installation properly now, but scoop-uninstall re-works for it:

> scoop install .\v1\meson.json
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
> scoop cleanup *
Removing meson: 0.61.1File not found - current
Remove-Item : Cannot find path 'C:\Users\HUMOR\Desktop\test\current' because it does not exist.
At C:\Users\HUMOR\scoop\apps\scoop\current\libexec\scoop-cleanup.ps1:58 char:9
+         Remove-Item $leftVersions -ErrorAction Stop -Force
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (C:\Users\HUMOR\Desktop\test\current:String) [Remove-Item], ItemNotFound
   Exception
    + FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.RemoveItemCommand
> scoop uninstall meson
ERROR 'meson' isn't installed correctly.
'meson' was uninstalled.

@niheaven
Copy link
Copy Markdown
Member Author

I've force push some commits, so have you pull the latest?

image

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 22, 2022

sure,

> scoop -v
Current Scoop version:
9331eb87 (HEAD -> fix-install-status, origin/fix-install-status) Merge branch 'develop' into fix-install-status
> scoop install .\v1\meson.json
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
> scoop cleanup meson // OR *
ERROR 'meson' isn't installed correctly.
Removing meson: 0.61.1File not found - current
Remove-Item : Cannot find path 'C:\Users\HUMOR\Desktop\test\current' because it does not exist.
At C:\Users\HUMOR\scoop\apps\scoop\current\libexec\scoop-cleanup.ps1:58 char:9
+         Remove-Item $leftVersions -ErrorAction Stop -Force
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (C:\Users\HUMOR\Desktop\test\current:String) [Remove-Item], ItemNotFound
   Exception
    + FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.RemoveItemCommand
> (pwd).path
C:\Users\HUMOR\Desktop\test

i noticed that you put meson.json into the tests bucket, and i have did so, problem still:

> scoop install tests/meson
Installing 'meson' (0.61.1) [64bit]
Loading meson-0.61.1-64.msi from cache
Checking hash of meson-0.61.1-64.msi ... ok.
Extracting meson-0.61.1-64.msi ... done.
Running installer script...
Linking ~\scoop\apps\meson\current => ~\scoop\apps\meson\0.61.1
Creating shim for 'meson'.
Can't shim 'meson.exe': File doesn't exist.
> scoop cleanup meson
ERROR 'meson' isn't installed correctly.
Removing meson: 0.61.1File not found - current
Remove-Item : Cannot find path 'C:\Users\HUMOR\current' because it does not exist.
At C:\Users\HUMOR\scoop\apps\scoop\current\libexec\scoop-cleanup.ps1:58 char:9
+         Remove-Item $leftVersions -ErrorAction Stop -Force
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (C:\Users\HUMOR\current:String) [Remove-Item], ItemNotFoundException
    + FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.RemoveItemCommand
> (pwd).path
C:\Users\HUMOR

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 22, 2022

testing in VM:

screenshot_20220122181649

niheaven added a commit to ScoopInstaller/Tests that referenced this pull request Jan 23, 2022
@niheaven
Copy link
Copy Markdown
Member Author

niheaven commented Jan 23, 2022

@HUMORCE @rashil2000 The bug is caused by different treatment of [DirectoryInfo] between PS5 and PS7, and is fixed by 3e49852.

A simple test: run (Get-ChildItem "$(scoop prefix 7zip)\..")[-1].ToString() in PS5 and PS7, and PS5 gives Name while PS7 gives FullName.

@rashil2000
Copy link
Copy Markdown
Member

Wow yes, indeed

@rashil2000
Copy link
Copy Markdown
Member

This is also visible in the CI tests btw, where PS 5.1 shows only the name of files being tested, while PS 7.1 shows the full path of files.

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 23, 2022

works.

but one more thing here, the failed installations can purge(uninstall) via scoop-uninstall now.
related: #4648 (comment)

@niheaven
Copy link
Copy Markdown
Member Author

works.

but one more thing here, the failed installations can purge(uninstall) via scoop-uninstall now. related: #4648 (comment)

Yes, sorry, I eat my words again... Who could understand the guy's thought in #4648 (comment)? lol

scoop cleanup, scoop uninstall and scoop update are all using same logic (core/Confirm-InstallationStatus) to check apps' status, so they may behave similar for a failed installation. If this is not suitable, maybe fixed in the future?

@niheaven
Copy link
Copy Markdown
Member Author

If everything is fine, I'll merge this PR @ScoopInstaller/maintainers

Comment thread libexec/scoop-uninstall.ps1 Outdated
Comment thread lib/core.ps1 Outdated
Comment thread lib/core.ps1 Outdated
@niheaven
Copy link
Copy Markdown
Member Author

@Calinou Why the 'bug' label?

@Calinou
Copy link
Copy Markdown
Member

Calinou commented Jan 23, 2022

@Calinou Why the 'bug' label?

The original issue has a bug label, which is why I also added the bug label here (although it can also be considered an enhancement) 🙂

@niheaven
Copy link
Copy Markdown
Member Author

@Calinou Why the 'bug' label?

The original issue has a bug label, which is why I also added the bug label here (although it can also be considered an enhancement) 🙂

Think if there is some bug that I've not found😅

We need a bug-fix tag...

@HUMORCE
Copy link
Copy Markdown
Member

HUMORCE commented Jan 23, 2022

in fact, i do not understand that a failed installation cannot be uninstall by uninstall command. but for tthe consistency(?or what), i am agree to your says.

@niheaven niheaven requested review from chawyehsu and removed request for chawyehsu January 24, 2022 03:41
@niheaven niheaven merged commit 158c0fd into ScoopInstaller:develop Jan 25, 2022
@niheaven niheaven deleted the fix-install-status branch January 25, 2022 13:58
@niheaven niheaven added bugfix and removed bug labels Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants