Plugins installation improvements (#8503)

* 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 02783503513673574255bd1a809ce17c6f0cee6c.

* Remove PLUGIN_ON_STARTUP support

* Better error catching for broken packages

* Cleanup

* Revert "Cleanup"

This reverts commit a40c85d47d9446cf4181b9865d2fce6a63efba92.

* 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 bf84c8155e6036da0a529acba44386fed982ae8e.

* Add note for future selves

---------

Co-authored-by: Matthias Mair <code@mjmair.com>

src/backend/InvenTree/InvenTree/settings.py

@@ -217,7 +217,8 @@ PLUGIN_RETRY = get_setting(
     '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 = []
 

src/backend/InvenTree/InvenTree/tests.py

@@ -1184,18 +1184,8 @@ class TestSettings(InvenTreeTestCase):
         """Test if install of plugins on startup works."""
         from plugin import registry
 
-        if not settings.DOCKER:
+        registry.reload_plugins(full_reload=True, collect=True)
-            # Check an install run
+        self.assertGreater(len(settings.PLUGIN_FILE_HASH), 0)
-            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)
 
     def test_helpers_cfg_file(self):
         """Test get_config_file."""

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
 

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):

src/backend/InvenTree/plugin/installer.py

@@ -19,12 +19,15 @@ logger = logging.getLogger('inventree')
 def pip_command(*args):
     """Build and run a pip command using using the current python executable.
 
-    returns: subprocess.check_output
+    Returns: The output of the pip command
-    throws: subprocess.CalledProcessError
+
+    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):
+def get_install_info(packagename: str) -> dict:
-    """Determine the install path of a particular package.
+    """Determine the install information for a particular package.
 
-    - If installed, return the installation path
+    - Uses 'pip show' to determine the install location of a package.
-    - If not installed, return False
     """
-    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: ..."
+            parts = line.split(':')
-            match = re.match(r'^Location:\s+(.+)$', line.strip())
 
-            if match:
+            if len(parts) >= 2:
-                return match.group(1)
+                key = str(parts[0].strip().lower().replace('-', '_'))
+                value = str(parts[1].strip())
+
+                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 info
-    return False
+
+
+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 update_plugins_file(install_name, remove=False):
             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)):
+        if packagename and (info := get_install_info(packagename)):
-            # Override result information
+            if path := info.get('location'):
-            ret['result'] = _(f'Installed plugin into {path}')
+                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
+    if version := ret.get('version'):
-    update_plugins_file(full_pkg)
+        # Save plugin to plugins file
+        update_plugins_file(packagename, full_package=full_pkg, version=version)
 
-    # Reload the plugin registry, to discover the new plugin
+        # Reload the plugin registry, to discover the new plugin
-    from plugin.registry import registry
+        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
+        # Update static files
-    plugin.staticfiles.collect_plugins_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]
+    pkg_info = get_install_info(package_name)
 
-    try:
+    if path := pkg_info.get('location'):
-        result = pip_command(*cmd)
+        # Uninstall the plugin using pip
-
+        logger.info('Uninstalling plugin: %s from %s', package_name, path)
-        ret = {
+        try:
-            'result': _('Uninstalled plugin successfully'),
+            pip_command('uninstall', '-y', package_name)
-            'success': True,
+        except subprocess.CalledProcessError as error:
-            'output': str(result, 'utf-8'),
+            handle_pip_error(error, 'plugin_uninstall')
-        }
+    else:
-
+        # No matching install target found
-    except subprocess.CalledProcessError as error:
+        raise ValidationError(_('Plugin installation not found'))
-        handle_pip_error(error, 'plugin_uninstall')
 
     # 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}

src/backend/InvenTree/plugin/models.py

@@ -70,7 +70,7 @@ class PluginConfig(InvenTree.models.MetadataMixin, models.Model):
         """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

src/backend/InvenTree/plugin/plugin.py

@@ -239,9 +239,10 @@ class InvenTreePlugin(VersionMixin, MixinBase, MetaBase):
         """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.

src/backend/InvenTree/plugin/registry.py

@@ -287,6 +287,7 @@ class PluginsRegistry:
 
             if collect:
                 logger.info('Collecting plugins')
+                self.install_plugin_file()
                 self.plugin_modules = self.collect_plugins()
 
             self.plugins_loaded = False
@@ -365,31 +366,32 @@ class PluginsRegistry:
         collected_plugins = []
 
         # Collect plugins from paths
-        for plugin in self.plugin_dirs():
+        for plugin_dir in self.plugin_dirs():
-            logger.debug("Loading plugins from directory '%s'", plugin)
+            logger.debug("Loading plugins from directory '%s'", plugin_dir)
 
             parent_path = None
-            parent_obj = Path(plugin)
+            parent_obj = Path(plugin_dir)
 
             # If a "path" is provided, some special handling is required
-            if parent_obj.name is not plugin and len(parent_obj.parts) > 1:
+            if parent_obj.name is not plugin_dir and len(parent_obj.parts) > 1:
                 # Ensure PosixPath object is converted to a string, before passing to get_plugins
                 parent_path = str(parent_obj.parent)
-                plugin = parent_obj.name
+                plugin_dir = parent_obj.name
 
             # Gather Modules
             if parent_path:
                 # On python 3.12 use new loader method
                 if sys.version_info < (3, 12):
                     raw_module = _load_source(
-                        plugin, str(parent_obj.joinpath('__init__.py'))
+                        plugin_dir, str(parent_obj.joinpath('__init__.py'))
                     )
                 else:
                     raw_module = SourceFileLoader(
-                        plugin, str(parent_obj.joinpath('__init__.py'))
+                        plugin_dir, str(parent_obj.joinpath('__init__.py'))
                     ).load_module()
             else:
-                raw_module = importlib.import_module(plugin)
+                raw_module = importlib.import_module(plugin_dir)
+
             modules = get_plugins(raw_module, InvenTreePlugin, path=parent_path)
 
             for item in modules or []:
@@ -429,16 +431,13 @@ class PluginsRegistry:
 
     def install_plugin_file(self):
         """Make sure all plugins are installed in the current environment."""
-        if settings.PLUGIN_FILE_CHECKED:
+        from plugin.installer import install_plugins_file, plugins_file_hash
-            logger.info('Plugin file was already checked')
-            return True
 
-        from plugin.installer import install_plugins_file
+        file_hash = plugins_file_hash()
 
-        if install_plugins_file():
+        if file_hash != settings.PLUGIN_FILE_HASH:
-            settings.PLUGIN_FILE_CHECKED = True
+            install_plugins_file()
-            return 'first_run'
+            settings.PLUGIN_FILE_HASH = file_hash
-        return False
 
     # endregion