From 5f344e886f4866e6d6a845d2d7dd1ab2edca13da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Golonka?= Date: Fri, 15 Dec 2023 17:25:44 +0100 Subject: [PATCH 1/6] Revert "Add-on store: Clean up failed installs (#15921)" This reverts commit 81e17be07bbebe02f682fbc82c23c698b865c50f. --- source/addonHandler/__init__.py | 40 ++++----------------------------- 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/source/addonHandler/__init__.py b/source/addonHandler/__init__.py index c2817cce7e8..beeedbcfb22 100644 --- a/source/addonHandler/__init__.py +++ b/source/addonHandler/__init__.py @@ -5,7 +5,6 @@ # See the file COPYING for more details. from abc import abstractmethod, ABC -import glob import sys import os.path import gettext @@ -20,6 +19,7 @@ from typing import ( Callable, Dict, + List, Optional, Set, TYPE_CHECKING, @@ -203,25 +203,6 @@ def cleanupRemovedDisabledAddons(self) -> None: log.debug(f"Discarding {disabledAddonName} from disabled add-ons as it has been uninstalled.") self[AddonStateCategory.DISABLED].discard(disabledAddonName) - def _cleanupInstalledAddons(self) -> None: - # There should be no pending installs after add-ons have been loaded during initialization. - for path in _getDefaultAddonPaths(): - pendingInstallPaths = glob.glob(f"{path}/*.{ADDON_PENDINGINSTALL_SUFFIX}") - for pendingInstallPath in pendingInstallPaths: - if os.path.exists(pendingInstallPath): - try: - log.error(f"Removing failed install of {pendingInstallPath}") - shutil.rmtree(pendingInstallPath, ignore_errors=True) - except OSError: - log.error(f"Failed to remove {pendingInstallPath}", exc_info=True) - - if self[AddonStateCategory.PENDING_INSTALL]: - log.error( - f"Discarding {self[AddonStateCategory.PENDING_INSTALL]} from pending install add-ons " - "as their install failed." - ) - self[AddonStateCategory.PENDING_INSTALL].clear() - def _cleanupCompatibleAddonsFromDowngrade(self) -> None: from addonStore.dataManager import addonDataManager installedAddons = addonDataManager._installedAddonsCache.installedAddons @@ -306,7 +287,6 @@ def initialize(): getAvailableAddons(refresh=True, isFirstLoad=True) state.cleanupRemovedDisabledAddons() state._cleanupCompatibleAddonsFromDowngrade() - state._cleanupInstalledAddons() if NVDAState.shouldWriteToDisk(): state.save() initializeModulePackagePaths() @@ -323,8 +303,8 @@ def terminate(): pass -def _getDefaultAddonPaths() -> list[str]: - r""" Returns paths where addons can be found. +def _getDefaultAddonPaths() -> List[str]: + """ Returns paths where addons can be found. For now, only \addons is supported. """ addon_paths = [] @@ -517,11 +497,7 @@ def __init__(self, path: str): _report_manifest_errors(self.manifest) raise AddonError("Manifest file has errors.") - def completeInstall(self) -> Optional[str]: - if not os.path.exists(self.pendingInstallPath): - log.error(f"Pending install path {self.pendingInstallPath} does not exist") - return None - + def completeInstall(self) -> str: try: os.rename(self.pendingInstallPath, self.installPath) state[AddonStateCategory.PENDING_INSTALL].discard(self.name) @@ -529,14 +505,6 @@ def completeInstall(self) -> Optional[str]: except OSError: log.error(f"Failed to complete addon installation for {self.name}", exc_info=True) - # Remove pending install folder - try: - log.error(f"Removing failed install of {self.pendingInstallPath}") - shutil.rmtree(self.pendingInstallPath, ignore_errors=True) - state[AddonStateCategory.PENDING_INSTALL].discard(self.name) - except OSError: - log.error(f"Failed to remove {self.pendingInstallPath}", exc_info=True) - def requestRemove(self): """Marks this addon for removal on NVDA restart.""" if self.isPendingInstall and not self.isInstalled: From 0c896c44045dfcad372075ae2606489f1970854d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Golonka?= Date: Sat, 16 Dec 2023 13:39:39 +0100 Subject: [PATCH 2/6] Show warnings when add-ons failed to install / uninstall and clean up no longer existing pending installation entries. --- source/addonHandler/__init__.py | 22 +++++++++++++++++--- source/core.py | 36 +++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/source/addonHandler/__init__.py b/source/addonHandler/__init__.py index beeedbcfb22..1aa106b5a42 100644 --- a/source/addonHandler/__init__.py +++ b/source/addonHandler/__init__.py @@ -74,6 +74,9 @@ # For more details see appropriate section of the developer guide. isCLIParamKnown = extensionPoints.AccumulatingDecider(defaultDecision=False) +_failedPendingRemovals: set[str] = set() +_failedPendingInstalls: set[str] = set() + AddonStateDictT = Dict[AddonStateCategory, CaseInsensitiveSet[str]] @@ -287,6 +290,12 @@ def initialize(): getAvailableAddons(refresh=True, isFirstLoad=True) state.cleanupRemovedDisabledAddons() state._cleanupCompatibleAddonsFromDowngrade() + if missingPendingInstalls := state[AddonStateCategory.PENDING_INSTALL] - _failedPendingInstalls: + log.error( + "The following add-ons should be installed, " + f"but are no longer present on disk: {', '.join(missingPendingInstalls)}" + ) + state[AddonStateCategory.PENDING_INSTALL] -= missingPendingInstalls if NVDAState.shouldWriteToDisk(): state.save() initializeModulePackagePaths() @@ -343,9 +352,10 @@ def _getAvailableAddonsFromPath( ): try: a.completeRemove() + continue except RuntimeError: log.exception(f"Failed to remove {name} add-on") - continue + _failedPendingRemovals.add(name) if( isFirstLoad and ( @@ -356,7 +366,13 @@ def _getAvailableAddonsFromPath( newPath = a.completeInstall() if newPath: a = Addon(newPath) - if isFirstLoad and name in state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY]: + else: # installation failed + _failedPendingInstalls.add(name) + if ( + isFirstLoad + and name in state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY] + and name not in _failedPendingInstalls + ): state[AddonStateCategory.OVERRIDE_COMPATIBILITY].add(name) state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY].remove(name) log.debug( @@ -565,7 +581,7 @@ def addToPackagePath(self, package): """ # #3090: Ensure that we don't add disabled / blocked add-ons to package path. # By returning here the addon does not "run"/ become active / registered. - if self.isDisabled or self.isBlocked or self.isPendingInstall: + if self.isDisabled or self.isBlocked or self.isPendingInstall or self.name in _failedPendingRemovals: return extension_path = os.path.join(self.path, package.__name__) diff --git a/source/core.py b/source/core.py index 60de2163263..7ce1665f006 100644 --- a/source/core.py +++ b/source/core.py @@ -70,6 +70,8 @@ def __bool__(self): def doStartupDialogs(): + import wx + import config import gui @@ -88,7 +90,6 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: if not isParamKnown: unknownCLIParams.append(param) if unknownCLIParams: - import wx gui.messageBox( # Translators: Shown when NVDA has been started with unknown command line parameters. _("The following command line parameters are unknown to NVDA: {params}").format( @@ -100,7 +101,6 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: wx.OK | wx.ICON_ERROR ) if config.conf.baseConfigError: - import wx gui.messageBox( # Translators: A message informing the user that there are errors in the configuration file. _("Your configuration file contains errors. " @@ -118,7 +118,6 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: gui.mainFrame.onToggleSpeechViewerCommand(evt=None) import inputCore if inputCore.manager.userGestureMap.lastUpdateContainedError: - import wx gui.messageBox(_("Your gesture map file contains errors.\n" "More details about the errors can be found in the log file."), _("gesture map File Error"), wx.OK|wx.ICON_EXCLAMATION) @@ -130,7 +129,6 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: if updateCheck and not config.conf['update']['askedAllowUsageStats']: # a callback to save config after the usage stats question dialog has been answered. def onResult(ID): - import wx if ID in (wx.ID_YES,wx.ID_NO): try: config.conf.save() @@ -138,6 +136,36 @@ def onResult(ID): pass # Ask the user if usage stats can be collected. gui.runScriptModalDialog(gui.startupDialogs.AskAllowUsageStatsDialog(None), onResult) + addonFailureMessages: list[str] = [] + failedUpdates = addonHandler._failedPendingInstalls.intersection(addonHandler._failedPendingRemovals) + failedInstalls = addonHandler._failedPendingInstalls - failedUpdates + failedRemovals = addonHandler._failedPendingRemovals - failedUpdates + if failedUpdates: + addonFailureMessages.append( + # Translators: Shown when one or more add-ons failed to update. + _("Following add-ons failed to update: {}").format(", ".join(failedUpdates)) + ) + if failedRemovals: + addonFailureMessages.append( + # Translators: Shown when one or more add-ons failed to be uninstalled. + _("Following add-ons failed to uninstall: {}").format(", ".join(failedRemovals)) + ) + if failedInstalls: + addonFailureMessages.append( + # Translators: Shown when one or more add-ons failed to be installed. + _("Following add-ons failed to be installed: {}").format(", ".join(failedInstalls)) + ) + + if addonFailureMessages: + gui.messageBox( + _( + # Translators: Shown when one or more actions on add-ons failed. + "Some operations on add-ons failed. See the log file for more details.\n{}" + ).format("\n".join(addonFailureMessages)), + # Translators: Title of message shown when requested action on add-ons failed. + _("Add-on failures"), + wx.ICON_ERROR | wx.OK + ) @dataclass From 122ad56d29e1f1c3478f89e1e7f47d43237624c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Golonka?= Date: Mon, 18 Dec 2023 16:23:54 +0100 Subject: [PATCH 3/6] Fix-up for pending override compat --- source/addonHandler/__init__.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/source/addonHandler/__init__.py b/source/addonHandler/__init__.py index 1aa106b5a42..7f4f8ed46ff 100644 --- a/source/addonHandler/__init__.py +++ b/source/addonHandler/__init__.py @@ -74,8 +74,8 @@ # For more details see appropriate section of the developer guide. isCLIParamKnown = extensionPoints.AccumulatingDecider(defaultDecision=False) -_failedPendingRemovals: set[str] = set() -_failedPendingInstalls: set[str] = set() +_failedPendingRemovals: CaseInsensitiveSet[str] = CaseInsensitiveSet() +_failedPendingInstalls: CaseInsensitiveSet[str] = CaseInsensitiveSet() AddonStateDictT = Dict[AddonStateCategory, CaseInsensitiveSet[str]] @@ -296,15 +296,17 @@ def initialize(): f"but are no longer present on disk: {', '.join(missingPendingInstalls)}" ) state[AddonStateCategory.PENDING_INSTALL] -= missingPendingInstalls - if NVDAState.shouldWriteToDisk(): - state.save() - initializeModulePackagePaths() - if state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY]: + if missingPendingOverrideCompat := ( + state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY] - _failedPendingInstalls + ): log.error( "The following add-ons which were marked as compatible are no longer installed: " - f"{', '.join(state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY])}" + f"{', '.join(missingPendingOverrideCompat)}" ) - state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY].clear() + state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY] -= missingPendingOverrideCompat + if NVDAState.shouldWriteToDisk(): + state.save() + initializeModulePackagePaths() def terminate(): From 85a2a4fa01461a69e571dbccce9139aad66a9479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Golonka?= Date: Mon, 18 Dec 2023 17:54:03 +0100 Subject: [PATCH 4/6] Bump for CI From 376a7abb3d44ea8cac00c7435574b3fb8086ef6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Golonka?= Date: Tue, 19 Dec 2023 12:13:28 +0100 Subject: [PATCH 5/6] Review actions --- source/addonHandler/__init__.py | 12 +++-- source/core.py | 84 ++++++++++++++++++++------------- 2 files changed, 60 insertions(+), 36 deletions(-) diff --git a/source/addonHandler/__init__.py b/source/addonHandler/__init__.py index 7f4f8ed46ff..2d404f3c184 100644 --- a/source/addonHandler/__init__.py +++ b/source/addonHandler/__init__.py @@ -19,7 +19,6 @@ from typing import ( Callable, Dict, - List, Optional, Set, TYPE_CHECKING, @@ -314,8 +313,8 @@ def terminate(): pass -def _getDefaultAddonPaths() -> List[str]: - """ Returns paths where addons can be found. +def _getDefaultAddonPaths() -> list[str]: + r""" Returns paths where addons can be found. For now, only \addons is supported. """ addon_paths = [] @@ -515,13 +514,18 @@ def __init__(self, path: str): _report_manifest_errors(self.manifest) raise AddonError("Manifest file has errors.") - def completeInstall(self) -> str: + def completeInstall(self) -> Optional[str]: + if not os.path.exists(self.pendingInstallPath): + log.error(f"Pending install path {self.pendingInstallPath} does not exist") + return None + try: os.rename(self.pendingInstallPath, self.installPath) state[AddonStateCategory.PENDING_INSTALL].discard(self.name) return self.installPath except OSError: log.error(f"Failed to complete addon installation for {self.name}", exc_info=True) + return None def requestRemove(self): """Marks this addon for removal on NVDA restart.""" diff --git a/source/core.py b/source/core.py index 7ce1665f006..4107cacfa83 100644 --- a/source/core.py +++ b/source/core.py @@ -69,9 +69,54 @@ def __bool__(self): _shuttingDownFlagLock = threading.Lock() -def doStartupDialogs(): - import wx +def _showAddonsWarnings() -> None: + addonFailureMessages: list[str] = [] + failedUpdates = addonHandler._failedPendingInstalls.intersection(addonHandler._failedPendingRemovals) + failedInstalls = addonHandler._failedPendingInstalls - failedUpdates + failedRemovals = addonHandler._failedPendingRemovals - failedUpdates + if failedUpdates: + addonFailureMessages.append( + ngettext( + # Translators: Shown when one or more add-ons failed to update. + "The following add-on failed to update: {}", + "The following add-ons failed to update: {}", + len(failedUpdates) + ).format(", ".join(failedUpdates)) + ) + if failedRemovals: + addonFailureMessages.append( + ngettext( + # Translators: Shown when one or more add-ons failed to be uninstalled. + "The following add-on failed to uninstall: {}", + "The following add-ons failed to uninstall: {}", + len(failedRemovals) + ).format(", ".join(failedRemovals)) + ) + if failedInstalls: + addonFailureMessages.append( + ngettext( + # Translators: Shown when one or more add-ons failed to be installed. + "The following add-on failed to be installed: {}", + "The following add-ons failed to be installed: {}", + len(failedInstalls) + ).format(", ".join(failedInstalls)) + ) + + if addonFailureMessages: + import wx + import gui + gui.messageBox( + _( + # Translators: Shown when one or more actions on add-ons failed. + "Some operations on add-ons failed. See the log file for more details.\n{}" + ).format("\n".join(addonFailureMessages)), + # Translators: Title of message shown when requested action on add-ons failed. + _("Add-on failures"), + wx.ICON_ERROR | wx.OK + ) + +def doStartupDialogs(): import config import gui @@ -90,6 +135,7 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: if not isParamKnown: unknownCLIParams.append(param) if unknownCLIParams: + import wx gui.messageBox( # Translators: Shown when NVDA has been started with unknown command line parameters. _("The following command line parameters are unknown to NVDA: {params}").format( @@ -101,6 +147,7 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: wx.OK | wx.ICON_ERROR ) if config.conf.baseConfigError: + import wx gui.messageBox( # Translators: A message informing the user that there are errors in the configuration file. _("Your configuration file contains errors. " @@ -118,6 +165,7 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: gui.mainFrame.onToggleSpeechViewerCommand(evt=None) import inputCore if inputCore.manager.userGestureMap.lastUpdateContainedError: + import wx gui.messageBox(_("Your gesture map file contains errors.\n" "More details about the errors can be found in the log file."), _("gesture map File Error"), wx.OK|wx.ICON_EXCLAMATION) @@ -129,6 +177,7 @@ def handleReplaceCLIArg(cliArgument: str) -> bool: if updateCheck and not config.conf['update']['askedAllowUsageStats']: # a callback to save config after the usage stats question dialog has been answered. def onResult(ID): + import wx if ID in (wx.ID_YES,wx.ID_NO): try: config.conf.save() @@ -136,36 +185,7 @@ def onResult(ID): pass # Ask the user if usage stats can be collected. gui.runScriptModalDialog(gui.startupDialogs.AskAllowUsageStatsDialog(None), onResult) - addonFailureMessages: list[str] = [] - failedUpdates = addonHandler._failedPendingInstalls.intersection(addonHandler._failedPendingRemovals) - failedInstalls = addonHandler._failedPendingInstalls - failedUpdates - failedRemovals = addonHandler._failedPendingRemovals - failedUpdates - if failedUpdates: - addonFailureMessages.append( - # Translators: Shown when one or more add-ons failed to update. - _("Following add-ons failed to update: {}").format(", ".join(failedUpdates)) - ) - if failedRemovals: - addonFailureMessages.append( - # Translators: Shown when one or more add-ons failed to be uninstalled. - _("Following add-ons failed to uninstall: {}").format(", ".join(failedRemovals)) - ) - if failedInstalls: - addonFailureMessages.append( - # Translators: Shown when one or more add-ons failed to be installed. - _("Following add-ons failed to be installed: {}").format(", ".join(failedInstalls)) - ) - - if addonFailureMessages: - gui.messageBox( - _( - # Translators: Shown when one or more actions on add-ons failed. - "Some operations on add-ons failed. See the log file for more details.\n{}" - ).format("\n".join(addonFailureMessages)), - # Translators: Title of message shown when requested action on add-ons failed. - _("Add-on failures"), - wx.ICON_ERROR | wx.OK - ) + _showAddonsWarnings() @dataclass From d5d29428adfdb3b8fca87c737dafe2bad28f5db1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Golonka?= Date: Tue, 19 Dec 2023 16:38:25 +0100 Subject: [PATCH 6/6] Second wave of review actions --- source/core.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/source/core.py b/source/core.py index 4107cacfa83..13f7df7f783 100644 --- a/source/core.py +++ b/source/core.py @@ -69,7 +69,7 @@ def __bool__(self): _shuttingDownFlagLock = threading.Lock() -def _showAddonsWarnings() -> None: +def _showAddonsErrors() -> None: addonFailureMessages: list[str] = [] failedUpdates = addonHandler._failedPendingInstalls.intersection(addonHandler._failedPendingRemovals) failedInstalls = addonHandler._failedPendingInstalls - failedUpdates @@ -78,8 +78,8 @@ def _showAddonsWarnings() -> None: addonFailureMessages.append( ngettext( # Translators: Shown when one or more add-ons failed to update. - "The following add-on failed to update: {}", - "The following add-ons failed to update: {}", + "The following add-on failed to update: {}.", + "The following add-ons failed to update: {}.", len(failedUpdates) ).format(", ".join(failedUpdates)) ) @@ -87,8 +87,8 @@ def _showAddonsWarnings() -> None: addonFailureMessages.append( ngettext( # Translators: Shown when one or more add-ons failed to be uninstalled. - "The following add-on failed to uninstall: {}", - "The following add-ons failed to uninstall: {}", + "The following add-on failed to uninstall: {}.", + "The following add-ons failed to uninstall: {}.", len(failedRemovals) ).format(", ".join(failedRemovals)) ) @@ -96,8 +96,8 @@ def _showAddonsWarnings() -> None: addonFailureMessages.append( ngettext( # Translators: Shown when one or more add-ons failed to be installed. - "The following add-on failed to be installed: {}", - "The following add-ons failed to be installed: {}", + "The following add-on failed to be installed: {}.", + "The following add-ons failed to be installed: {}.", len(failedInstalls) ).format(", ".join(failedInstalls)) ) @@ -111,7 +111,7 @@ def _showAddonsWarnings() -> None: "Some operations on add-ons failed. See the log file for more details.\n{}" ).format("\n".join(addonFailureMessages)), # Translators: Title of message shown when requested action on add-ons failed. - _("Add-on failures"), + _("Error"), wx.ICON_ERROR | wx.OK ) @@ -185,7 +185,7 @@ def onResult(ID): pass # Ask the user if usage stats can be collected. gui.runScriptModalDialog(gui.startupDialogs.AskAllowUsageStatsDialog(None), onResult) - _showAddonsWarnings() + _showAddonsErrors() @dataclass