Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 28 additions & 38 deletions source/addonHandler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,6 +73,9 @@
# For more details see appropriate section of the developer guide.
isCLIParamKnown = extensionPoints.AccumulatingDecider(defaultDecision=False)

_failedPendingRemovals: CaseInsensitiveSet[str] = CaseInsensitiveSet()
_failedPendingInstalls: CaseInsensitiveSet[str] = CaseInsensitiveSet()


AddonStateDictT = Dict[AddonStateCategory, CaseInsensitiveSet[str]]

Expand Down Expand Up @@ -203,25 +205,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
Expand Down Expand Up @@ -306,16 +289,23 @@ def initialize():
getAvailableAddons(refresh=True, isFirstLoad=True)
state.cleanupRemovedDisabledAddons()
state._cleanupCompatibleAddonsFromDowngrade()
state._cleanupInstalledAddons()
if NVDAState.shouldWriteToDisk():
state.save()
initializeModulePackagePaths()
if state[AddonStateCategory.PENDING_OVERRIDE_COMPATIBILITY]:
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 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():
Expand Down Expand Up @@ -363,9 +353,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 (
Expand All @@ -376,7 +367,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(
Expand Down Expand Up @@ -528,14 +525,7 @@ def completeInstall(self) -> Optional[str]:
return self.installPath
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)
return None

def requestRemove(self):
"""Marks this addon for removal on NVDA restart."""
Expand Down Expand Up @@ -597,7 +587,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__)
Expand Down
48 changes: 48 additions & 0 deletions source/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,53 @@ def __bool__(self):
_shuttingDownFlagLock = threading.Lock()


def _showAddonsWarnings() -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are displaying an error message:

Suggested change
def _showAddonsWarnings() -> None:
def _showAddonsUpdateErrors() -> None:

or only:

Suggested change
def _showAddonsWarnings() -> None:
def _showAddonsErrors() -> 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: {}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add end of sentence period. It's better when reading the whole dialog (NVDA+B) and various such messages are concatenated.

Suggested change
"The following add-on failed to update: {}",
"The following add-on failed to update: {}.",

"The following add-ons failed to update: {}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add period

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: {}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add period

"The following add-ons failed to uninstall: {}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add period

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: {}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add period

"The following add-ons failed to be installed: {}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add period

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"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the add-on is failing, it's its install/removal process.

A simpler title would be enough IMO.

Suggested change
_("Add-on failures"),
_("Error"),

wx.ICON_ERROR | wx.OK
)


def doStartupDialogs():
import config
import gui
Expand Down Expand Up @@ -138,6 +185,7 @@ def onResult(ID):
pass
# Ask the user if usage stats can be collected.
gui.runScriptModalDialog(gui.startupDialogs.AskAllowUsageStatsDialog(None), onResult)
_showAddonsWarnings()


@dataclass
Expand Down