From fdef458f0e2418b90ae9e1083692d32b5441d41f Mon Sep 17 00:00:00 2001 From: Ryan Inch Date: Wed, 31 Jul 2024 01:22:26 -0400 Subject: [PATCH] Fix dependency installation error reporting and handling. False positives in the error reporting when installing dependencies caused confusion to users and sometimes led to the dependency not getting installed at all when false errors were detected and the install halted. Switch to relying solely on is_module_available to determine whether an install was successful and suppress errors unless they are 100% confirmed to be an issue. Modify is_module_available to be able to check both system dependencies (i.e. pip) and locally installed dependencies. Modify is_module_available to explicitly return a bool. --- addons/io_hubs_addon/preferences.py | 43 +++++++++++++++++------------ addons/io_hubs_addon/utils.py | 17 +++++++----- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/addons/io_hubs_addon/preferences.py b/addons/io_hubs_addon/preferences.py index 31e7342e..25d4c9e5 100644 --- a/addons/io_hubs_addon/preferences.py +++ b/addons/io_hubs_addon/preferences.py @@ -45,23 +45,24 @@ def execute(self, context): import sys result = subprocess.run([sys.executable, '-m', 'ensurepip'], - capture_output=False, text=True, input="y") - if result.returncode < 0: + capture_output=True, text=True, input="y") + if not is_module_available('pip', local_dependency=False): print(result.stderr) + report_messages = [ + f"Installing {self.dep_config.name} has failed. Pip is not present and can't be installed", + "See the terminal for more details. Windows users can access their terminal by going to Window ‣ Toggle System Console" + ] bpy.ops.wm.hubs_report_viewer('INVOKE_DEFAULT', title="Hubs scene debugger report", - report_string='\n\n'.join(["Dependencies install has failed installing pip", - f'{result.stderr}'])) + report_string='\n\n'.join(report_messages)) return {'CANCELLED'} + if result.returncode != 0: + print("ensurepip failed but pip appears to be present anyway. Continuing on.") result = subprocess.run( [sys.executable, '-m', 'pip', 'install', '--upgrade', 'pip'], - capture_output=False, text=True, input="y") - if result.returncode < 0: - print(result.stderr) - bpy.ops.wm.hubs_report_viewer('INVOKE_DEFAULT', title="Hubs scene debugger report", - report_string='\n\n'.join(["Dependencies install has failed upgrading pip", - f'{result.stderr}'])) - return {'CANCELLED'} + capture_output=True, text=True, input="y") + if result.returncode != 0: + print("Upgrading pip has failed. Continuing on.") from .utils import get_or_create_deps_path dep = self.dep_config.name @@ -72,18 +73,24 @@ def execute(self, context): [sys.executable, '-m', 'pip', 'install', '--upgrade', dep, '-t', get_or_create_deps_path(self.dep_config.name)], capture_output=True, text=True, input="y") - failed = False if not is_module_available(self.dep_config.name): - failed = True - if result.returncode != 0 or failed: - print(result.stderr) + report_messages = [ + f"Installing {self.dep_config.name} has failed.", + "See the terminal for more details. Windows users can access their terminal by going to Window ‣ Toggle System Console" + ] bpy.ops.wm.hubs_report_viewer('INVOKE_DEFAULT', title="Hubs scene debugger report", - report_string='\n\n'.join(["Dependencies install has failed", - f'{result.stderr}'])) + report_string='\n\n'.join(report_messages)) return {'CANCELLED'} else: + # Installing packages with pip always seems to have a successful return code, + # regardless of whether any errors were encountered. + # So don't bother trying to notify the user of any potential errors. + # We assume that since the module is there, everything is fine. + # If needed, change capture_output to False and check the terminal for any errors. + print(f"{self.dep_config.name} has been installed successfully.") + report_messages = [f"{self.dep_config.name} has been installed successfully."] bpy.ops.wm.hubs_report_viewer('INVOKE_DEFAULT', title="Hubs scene debugger report", - report_string="Dependencies installed successfully") + report_string='\n\n'.join(report_messages)) return {'FINISHED'} diff --git a/addons/io_hubs_addon/utils.py b/addons/io_hubs_addon/utils.py index f8a13dd5..8ad34da9 100644 --- a/addons/io_hubs_addon/utils.py +++ b/addons/io_hubs_addon/utils.py @@ -35,25 +35,28 @@ def get_or_create_deps_path(name): return deps_path -def is_module_available(name): +def is_module_available(name, local_dependency=True): import sys old_syspath = sys.path[:] old_sysmod = sys.modules.copy() try: - path = get_or_create_deps_path(name) + if local_dependency: + path = get_or_create_deps_path(name) + sys.path.insert(0, str(path)) import importlib - sys.path.insert(0, str(path)) - try: loader = importlib.util.find_spec(name) except ImportError as ex: print(f'{name} not found') - import os - path = os.path.join(path, name) - return loader and os.path.exists(path) + if local_dependency: + import os + path = os.path.join(path, name) + return bool(loader and os.path.exists(path)) + else: + return bool(loader) finally: # Restore without assigning a new list instance. That way references