Fix registration of the enabled state of plugins
Concern Raised35f3f710c083

Authored by kossebau on Oct 6 2017, 3:57 AM.

Description

Fix registration of the enabled state of plugins

Summary:
The old code was slightly broken:
At start-up plugin controller updated the config storage only for the
enabled state of the global-category plugins (and indirectly their
dependency plugins). Project-category plugins would only get an update
indirectly once loadProjectPlugins() was run, other plugins only once
the were requested by their interface.
Thus in a new session with no project yet loaded the config storage would
not yet cover the enabled state of the project-category plugins and those
other.

PluginPreferences code queried the initial enabled state of each plugin
from the plugin controller, which then has extra code in the isEnabled()
method to deal with that. Though that information is dropped and replaced
with the one fetched from the config storage, multiple times even:

  • the plugins are added to the selector using the flag KPluginSelector::ReadConfigFile, which results in the data manually set to each KPluginInfo instance with kpi.setPluginEnabled(...) being overwritten with the config storage data
  • selector->load() was called after plugins were added, which results in the same update from the config storage data
  • the PluginPreferences instance like each KDevelop::ConfigPage gets an initial call of the reset() method on setup, which again calls selector->load()

Which results in the plugin selector showing wrong enabled state at least
for new sessions, where no project has been loaded yet and not all plugins
at least been tried to load once.
While the first two times of data drop could be fixed, the third would be
more complicted to do.

So instead of keeping the current on-the-fly enabled state calculation for
non-global-category plugins, on start-up the config storage is updated for
any known plugin (and then consecutively updated if failing to load a
plugin when needed). This gives the plugin selector and any other code
consistent data and simplifies things.

Next to this, this patch also restores ShellExtension::defaultPlugins()
power to control which plugins should be loaded by default. Thus any
tests which (try to) make use of the AutoTestShell::init plugin list
feature can (and have to) now explicitly define all userselectable plugins
which should be used (thus also using the correct plugin id :) ).

Test Plan:
Create a new session, open directly the plugin selection settings and see
that all build and projectmanager plugins but also the QML plugin are shown
as enabled, as they should be.

Reviewers: KDevelop, mwolff

Reviewed By: KDevelop, mwolff

Subscribers: mwolff, kdevelop-devel

Differential Revision: https://phabricator.kde.org/D8156

Details

Auditors
rjvbb
Committed
kossebauOct 17 2017, 3:17 AM
Reviewer
KDevelop
Differential Revision
D8156: Fix registration of the enabled state of plugins
Parents
R32:e78ddf2fb9d6: Remove empty init/cleanup methods from unit tests
Branches
Unknown
Tags
Unknown
rjvbb raised a concern with this commit.Oct 17 2017, 8:39 AM
rjvbb added a subscriber: rjvbb.

If this solves my issues with the documentation toolview being restored to an unwanted state, then great. But:

kdevplatform.shell: Not loading plugin named "kdevperforce" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevpdb" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevlldb" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevandroid" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevManPage" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevflatpak" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevkonsoleview" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevdocker" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevgdb" because it has been disabled!
kdevplatform.shell: Not loading plugin named "kdevheaptrack" because it has been disabled!

do those really have to be warnings, once per project for some plugins (see below)? For one, I know they're disabled, I asked for that. Secondly, if these are indeed to be shown to the user every time a session is started, they'd better show the same title as shown in the settings dialog. Here I recognise most at a glance, but not "kdevdocker".

I would suggest to make these debug messages, and extend the "Loaded Plugins" dialog to show which known plugins have not been loaded.
Failure to load a plugin should of course remain a warning - but not if that is because the plugin has been disabled of course.

From a session in which I disable the Ninja, CMake and QMake plugins (plus a bunch of other, to keep its footprint as small as possible). This session contains 3 projects that use the custom make manager project plugin:

kdevplatform.shell: Not loading plugin named "KDevNinjaBuilder" because it has been disabled!
kdevplatform.shell: Can't load plugin "KDevCMakeManager" some of its required dependencies could not be fulfilled: "org.kdevelop.IProjectBuilder@KDevCMakeBuilder"
kdevplatform.shell: Not loading plugin named "KDevCMakeBuilder" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevQMakeManager" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevQMakeBuilder" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevNinjaBuilder" because it has been disabled!
kdevplatform.shell: Can't load plugin "KDevCMakeManager" some of its required dependencies could not be fulfilled: "org.kdevelop.IProjectBuilder@KDevCMakeBuilder"
kdevplatform.shell: Not loading plugin named "KDevCMakeBuilder" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevQMakeManager" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevQMakeBuilder" because it has been disabled!
kdevelop.plugins.git: couldn't find the git root for QUrl("file:///opt/local/portia-site-ports")
kdevplatform.shell: Not loading plugin named "KDevNinjaBuilder" because it has been disabled!
kdevplatform.shell: Can't load plugin "KDevCMakeManager" some of its required dependencies could not be fulfilled: "org.kdevelop.IProjectBuilder@KDevCMakeBuilder"
kdevplatform.shell: Not loading plugin named "KDevCMakeBuilder" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevQMakeManager" because it has been disabled!
kdevplatform.shell: Not loading plugin named "KDevQMakeBuilder" because it has been disabled!
This commit now has outstanding concerns.Oct 17 2017, 8:39 AM

If this solves my issues with the documentation toolview being restored to an unwanted state, then great.

This has nothing to do with tool view state restoring.

I would suggest to make these debug messages, and extend the "Loaded Plugins" dialog to show which known plugins have not been loaded.
Failure to load a plugin should of course remain a warning - but not if that is because the plugin has been disabled of course.

This patch was about keeping the config storage more consistent (as a first approach, more to be sorted) between the plugin selection dialog and the kdevelop plugin loading code.
It did not change anything of what you now talked about (thus will also not reply here on the issues).
The purpose of a review request is to discuss the very patch, not to start new discussion about other things which are only related to this.

Please help and keep things focussed. One thing at a time, in an own thread/report/commit/reviewrequest.

rjvbb added a comment.Oct 17 2017, 4:36 PM
It did not change anything of what you now talked about

I'm pretty certain I did not get the warnings about plugins not being loaded before this commit (except the failure ones). No other commit was made...