Plugins installation improvements (inventree#8503)

SchrodingersGat · matmair · web-flow · commit 01aa8bb2ba6f · 2024-11-21T07:38:26.000+11:00
* Append plugins dir to pythonpath * Error handling in plugin helpers * Install plugin into "plugins" directory * Use plugins dir when installing from plugins.txt * Implement removal of plugin from plugins dir * Remove the dist-info dirs too * Cleanup * Catch errors * Specify plugin location for CI * Remove plugins.txt support * Improve regex for plugin matching * Revert "Remove plugins.txt support" This reverts commit 0278350. * Remove PLUGIN_ON_STARTUP support * Better error catching for broken packages * Cleanup * Revert "Cleanup" This reverts commit a40c85d. * Improved exception handling for plugin loading * More logging * Revert uninstall behaviour * Revert python path update * Improve check for plugins file * Revert check on startup * Better management of plugins file - Use file hash to determine if it should be reloaded * Fix docstring * Update unit tests * revert gh env * No cache * Update src/backend/InvenTree/plugin/installer.py Co-authored-by: Matthias Mair <code@mjmair.com> * Use hashlib.file_digest * Remove --no-cache-dir * Revert "Use hashlib.file_digest" This reverts commit bf84c81. * Add note for future selves --------- Co-authored-by: Matthias Mair <code@mjmair.com>
diff --git a/src/backend/InvenTree/InvenTree/settings.py b/src/backend/InvenTree/InvenTree/settings.py
@@ -217,7 +217,8 @@
     'INVENTREE_PLUGIN_RETRY', 'PLUGIN_RETRY', 3, typecast=int
 )  # How often should plugin loading be tried?
 
-PLUGIN_FILE_CHECKED = False  # Was the plugin file checked?
+# Hash of the plugin file (will be updated on each change)
+PLUGIN_FILE_HASH = ''
 
 STATICFILES_DIRS = []
 
diff --git a/src/backend/InvenTree/InvenTree/tests.py b/src/backend/InvenTree/InvenTree/tests.py
@@ -1184,18 +1184,8 @@ def test_initial_install(self):
         """Test if install of plugins on startup works."""
         from plugin import registry
 
-        if not settings.DOCKER:
-            # Check an install run
-            response = registry.install_plugin_file()
-            self.assertEqual(response, 'first_run')
-
-            # Set dynamic setting to True and rerun to launch install
-            InvenTreeSetting.set_setting('PLUGIN_ON_STARTUP', True, self.user)
-            registry.reload_plugins(full_reload=True)
-
-        # Check that there was another run
-        response = registry.install_plugin_file()
-        self.assertEqual(response, True)
+        registry.reload_plugins(full_reload=True, collect=True)
+        self.assertGreater(len(settings.PLUGIN_FILE_HASH), 0)
 
     def test_helpers_cfg_file(self):
         """Test get_config_file."""
diff --git a/src/backend/InvenTree/plugin/apps.py b/src/backend/InvenTree/plugin/apps.py
@@ -23,8 +23,6 @@ class PluginAppConfig(AppConfig):
 
     def ready(self):
         """The ready method is extended to initialize plugins."""
-        # skip loading if we run in a background thread
-
         if not isInMainThread() and not isInWorkerThread():
             return
 
diff --git a/src/backend/InvenTree/plugin/helpers.py b/src/backend/InvenTree/plugin/helpers.py
@@ -177,7 +177,17 @@ def get_modules(pkg, path=None):
     elif type(path) is not list:
         path = [path]
 
-    for finder, name, _ in pkgutil.walk_packages(path):
+    packages = pkgutil.walk_packages(path)
+
+    while True:
+        try:
+            finder, name, _ = next(packages)
+        except StopIteration:
+            break
+        except Exception as error:
+            log_error({pkg.__name__: str(error)}, 'discovery')
+            continue
+
         try:
             if sys.version_info < (3, 12):
                 module = finder.find_module(name).load_module(name)
@@ -202,9 +212,13 @@ def get_modules(pkg, path=None):
     return [v for k, v in context.items()]
 
 
-def get_classes(module):
+def get_classes(module) -> list:
     """Get all classes in a given module."""
-    return inspect.getmembers(module, inspect.isclass)
+    try:
+        return inspect.getmembers(module, inspect.isclass)
+    except Exception:
+        log_error({module.__name__: 'Could not get classes'}, 'discovery')
+        return []
 
 
 def get_plugins(pkg, baseclass, path=None):
diff --git a/src/backend/InvenTree/plugin/installer.py b/src/backend/InvenTree/plugin/installer.py
@@ -19,12 +19,15 @@
 def pip_command(*args):
     """Build and run a pip command using using the current python executable.
 
-    returns: subprocess.check_output
-    throws: subprocess.CalledProcessError
+    Returns: The output of the pip command
+
+    Raises:
+        subprocess.CalledProcessError: If the pip command fails
     """
     python = sys.executable
 
     command = [python, '-m', 'pip']
+
     command.extend(args)
 
     command = [str(x) for x in command]
@@ -63,39 +66,55 @@ def handle_pip_error(error, path: str) -> list:
         raise ValidationError(errors[0])
 
 
-def check_package_path(packagename: str):
-    """Determine the install path of a particular package.
+def get_install_info(packagename: str) -> dict:
+    """Determine the install information for a particular package.
 
-    - If installed, return the installation path
-    - If not installed, return False
+    - Uses 'pip show' to determine the install location of a package.
     """
-    logger.debug('check_package_path: %s', packagename)
+    logger.debug('get_install_info: %s', packagename)
 
     # Remove version information
-    for c in '<>=! ':
+    for c in '<>=!@ ':
         packagename = packagename.split(c)[0]
 
+    info = {}
+
     try:
         result = pip_command('show', packagename)
 
         output = result.decode('utf-8').split('\n')
 
         for line in output:
-            # Check if line matches pattern "Location: ..."
-            match = re.match(r'^Location:\s+(.+)$', line.strip())
+            parts = line.split(':')
+
+            if len(parts) >= 2:
+                key = str(parts[0].strip().lower().replace('-', '_'))
+                value = str(parts[1].strip())
 
-            if match:
-                return match.group(1)
+                info[key] = value
 
     except subprocess.CalledProcessError as error:
-        log_error('check_package_path')
+        log_error('get_install_info')
 
         output = error.output.decode('utf-8')
+        info['error'] = output
         logger.exception('Plugin lookup failed: %s', str(output))
-        return False
 
-    # If we get here, the package is not installed
-    return False
+    return info
+
+
+def plugins_file_hash():
+    """Return the file hash for the plugins file."""
+    import hashlib
+
+    pf = settings.PLUGIN_FILE
+
+    if not pf or not pf.exists():
+        return None
+
+    with pf.open('rb') as f:
+        # Note: Once we support 3.11 as a minimum, we can use hashlib.file_digest
+        return hashlib.sha256(f.read()).hexdigest()
 
 
 def install_plugins_file():
@@ -108,8 +127,10 @@ def install_plugins_file():
         logger.warning('Plugin file %s does not exist', str(pf))
         return
 
+    cmd = ['install', '--disable-pip-version-check', '-U', '-r', str(pf)]
+
     try:
-        pip_command('install', '-r', str(pf))
+        pip_command(*cmd)
     except subprocess.CalledProcessError as error:
         output = error.output.decode('utf-8')
         logger.exception('Plugin file installation failed: %s', str(output))
@@ -120,17 +141,25 @@ def install_plugins_file():
         log_error('pip')
         return False
 
-    # Update static files
+    # Collect plugin static files
     plugin.staticfiles.collect_plugins_static_files()
-    plugin.staticfiles.clear_plugins_static_files()
 
     # At this point, the plugins file has been installed
     return True
 
 
-def update_plugins_file(install_name, remove=False):
+def update_plugins_file(install_name, full_package=None, version=None, remove=False):
     """Add a plugin to the plugins file."""
-    logger.info('Adding plugin to plugins file: %s', install_name)
+    if remove:
+        logger.info('Removing plugin from plugins file: %s', install_name)
+    else:
+        logger.info('Adding plugin to plugins file: %s', install_name)
+
+    # If a full package name is provided, use that instead
+    if full_package and full_package != install_name:
+        new_value = full_package
+    else:
+        new_value = f'{install_name}=={version}' if version else install_name
 
     pf = settings.PLUGIN_FILE
 
@@ -140,7 +169,7 @@ def update_plugins_file(install_name, remove=False):
 
     def compare_line(line: str):
         """Check if a line in the file matches the installname."""
-        return line.strip().split('==')[0] == install_name.split('==')[0]
+        return re.match(rf'^{install_name}[\s=@]', line.strip())
 
     # First, read in existing plugin file
     try:
@@ -166,13 +195,13 @@ def compare_line(line: str):
             found = True
             if not remove:
                 # Replace line with new install name
-                output.append(install_name)
+                output.append(new_value)
         else:
             output.append(line)
 
     # Append plugin to file
     if not found and not remove:
-        output.append(install_name)
+        output.append(new_value)
 
     # Write file back to disk
     try:
@@ -203,15 +232,8 @@ def install_plugin(url=None, packagename=None, user=None, version=None):
 
     logger.info('install_plugin: %s, %s', url, packagename)
 
-    # Check if we are running in a virtual environment
-    # For now, just log a warning
-    in_venv = sys.prefix != sys.base_prefix
-
-    if not in_venv:
-        logger.warning('InvenTree is not running in a virtual environment')
-
     # build up the command
-    install_name = ['install', '-U']
+    install_name = ['install', '-U', '--disable-pip-version-check']
 
     full_pkg = ''
 
@@ -246,23 +268,25 @@ def install_plugin(url=None, packagename=None, user=None, version=None):
         ret['result'] = ret['success'] = _('Installed plugin successfully')
         ret['output'] = str(result, 'utf-8')
 
-        if packagename and (path := check_package_path(packagename)):
-            # Override result information
-            ret['result'] = _(f'Installed plugin into {path}')
+        if packagename and (info := get_install_info(packagename)):
+            if path := info.get('location'):
+                ret['result'] = _(f'Installed plugin into {path}')
+                ret['version'] = info.get('version')
 
     except subprocess.CalledProcessError as error:
         handle_pip_error(error, 'plugin_install')
 
-    # Save plugin to plugins file
-    update_plugins_file(full_pkg)
+    if version := ret.get('version'):
+        # Save plugin to plugins file
+        update_plugins_file(packagename, full_package=full_pkg, version=version)
 
-    # Reload the plugin registry, to discover the new plugin
-    from plugin.registry import registry
+        # Reload the plugin registry, to discover the new plugin
+        from plugin.registry import registry
 
-    registry.reload_plugins(full_reload=True, force_reload=True, collect=True)
+        registry.reload_plugins(full_reload=True, force_reload=True, collect=True)
 
-    # Update static files
-    plugin.staticfiles.collect_plugins_static_files()
+        # Update static files
+        plugin.staticfiles.collect_plugins_static_files()
 
     return ret
 
@@ -303,23 +327,24 @@ def uninstall_plugin(cfg: plugin.models.PluginConfig, user=None, delete_config=T
             _('Plugin cannot be uninstalled as it is currently active')
         )
 
+    if not cfg.is_installed():
+        raise ValidationError(_('Plugin is not installed'))
+
     validate_package_plugin(cfg, user)
     package_name = cfg.package_name
-    logger.info('Uninstalling plugin: %s', package_name)
-
-    cmd = ['uninstall', '-y', package_name]
-
-    try:
-        result = pip_command(*cmd)
 
-        ret = {
-            'result': _('Uninstalled plugin successfully'),
-            'success': True,
-            'output': str(result, 'utf-8'),
-        }
+    pkg_info = get_install_info(package_name)
 
-    except subprocess.CalledProcessError as error:
-        handle_pip_error(error, 'plugin_uninstall')
+    if path := pkg_info.get('location'):
+        # Uninstall the plugin using pip
+        logger.info('Uninstalling plugin: %s from %s', package_name, path)
+        try:
+            pip_command('uninstall', '-y', package_name)
+        except subprocess.CalledProcessError as error:
+            handle_pip_error(error, 'plugin_uninstall')
+    else:
+        # No matching install target found
+        raise ValidationError(_('Plugin installation not found'))
 
     # Update the plugins file
     update_plugins_file(package_name, remove=True)
@@ -334,4 +359,4 @@ def uninstall_plugin(cfg: plugin.models.PluginConfig, user=None, delete_config=T
     # Reload the plugin registry
     registry.reload_plugins(full_reload=True, force_reload=True, collect=True)
 
-    return ret
+    return {'result': _('Uninstalled plugin successfully'), 'success': True}
diff --git a/src/backend/InvenTree/plugin/models.py b/src/backend/InvenTree/plugin/models.py
@@ -70,7 +70,7 @@ def __str__(self) -> str:
         """Nice name for printing."""
         name = f'{self.name} - {self.key}'
         if not self.active:
-            name += '(not active)'
+            name += ' (not active)'
         return name
 
     # extra attributes from the registry
diff --git a/src/backend/InvenTree/plugin/plugin.py b/src/backend/InvenTree/plugin/plugin.py
@@ -239,9 +239,10 @@ def file(cls) -> Path:
         """File that contains plugin definition."""
         return Path(inspect.getfile(cls))
 
-    def path(self) -> Path:
+    @classmethod
+    def path(cls) -> Path:
         """Path to plugins base folder."""
-        return self.file().parent
+        return cls.file().parent
 
     def _get_value(self, meta_name: str, package_name: str) -> str:
         """Extract values from class meta or package info.
diff --git a/src/backend/InvenTree/plugin/registry.py b/src/backend/InvenTree/plugin/registry.py
