Ensure that all plugins implement Plugin or one of its sub-interfaces

[sschuberth]

ORT currently supports the following ServiceLoader-based plugins, grouped by whether they need to support plugin-specific configuration or not.

These are plugins that may require configuration and should implement the org.ossreviewtoolkit.utils.common.TypedConfigurablePluginFactory interface (ticked ones already do):

• org.ossreviewtoolkit.advisor.AdviceProviderFactory (currently implements org.ossreviewtoolkit.utils.common.Plugin only)
• org.ossreviewtoolkit.analyzer.PackageConfigurationProviderFactory
• org.ossreviewtoolkit.analyzer.PackageCurationProviderFactory
• org.ossreviewtoolkit.analyzer.PackageManagerFactory (currently implements org.ossreviewtoolkit.utils.common.Plugin only)
• org.ossreviewtoolkit.reporter.Reporter (currently implements org.ossreviewtoolkit.utils.common.Plugin only)
  • This implies to migrate reporter-specific (CLI) options to config.yml.
  • The conversion to typed plugins was done by migrating to the new org.ossreviewtoolkit.plugins.api.Plugin API.
• org.ossreviewtoolkit.scanner.ScannerWrapperFactory (currently implements org.ossreviewtoolkit.utils.common.Plugin only)

These are plugins that don't need to support configuration and should just implement the org.ossreviewtoolkit.utils.common.Plugin interface (ticked ones already do):

• org.ossreviewtoolkit.cli.OrtCommand
• org.ossreviewtoolkit.downloader.VersionControlSystem
  • There's currently a conflict with the type properties that needs to be resolved first.

[sschuberth]

@mnonnenmacher, am I correct to assume that this issue is now obsolete in favor of implementing the new KSP-based plugin API?

[sschuberth]

This implies to migrate reporter-specific (CLI) options to config.yml.

Also, now that reporters are migrated to the new KSP-based plugin API, should we remove the code

ort/plugins/commands/reporter/src/main/kotlin/ReporterCommand.kt

Lines 280 to 285 in 4596888

	// Allow overwriting reporter-specific options via the command line.
	reportOptions.forEach { (reporterName, option) ->
	val reportSpecificConfig = reportConfigMap.getOrPut(reporterName) { PluginConfiguration.EMPTY }
	val updatedConfig = reportSpecificConfig.copy(options = reportSpecificConfig.options + option)
	reportConfigMap[reporterName] = updatedConfig
	}

and require users to leverage the generic -P option to specify reporter-specific options? This would align with package manager plugins.

[mnonnenmacher]

@mnonnenmacher, am I correct to assume that this issue is now obsolete in favor of implementing the new KSP-based plugin API?

With #9403 this issue is not required anymore.

... require users to leverage the generic -P option to specify reporter-specific options? This would align with package manager plugins.

Does this work? I thought with -P you cannot set map entries.

[sschuberth]

Does this work? I thought with -P you cannot set map entries.

That's true (see sksamuel/hoplite#358), but I thought that with typed plugin options they would not be configured as a map anyway.

[mnonnenmacher]

Does this work? I thought with -P you cannot set map entries.

I thought that with typed plugin options they would not be configured as a map anyway.

In the ORT config model it still has to be a map, because the model cannot know the config classes of the plugins.

[sschuberth]

because the model cannot know the config classes of the plugins.

Could we solve that by making the plugin config classes of the plugin themselves also discoverable via the ServiceLoader mechanism?

[mnonnenmacher]

because the model cannot know the config classes of the plugins.

Could we solve that by making the plugin config classes of the plugin themselves also discoverable via the ServiceLoader mechanism?

I don't think so. The config class would need a property of the config class type for the mapping to work, like webAppReporterConfig: WebAppReporterConfig, so it needs to be known at compile time. Or you need some interface like ReporterPluginConfig to determine the type during serialization, but then I don't know how this could work with -P. Anyway, I think it makes sense to have a separate issue for this topic if we don't have one already.