Review Request 121775: Correctly load dependencies for plugins

[Sergey Kalinichev]

> On Jan. 3, 2015, 6:23 p.m., Milian Wolff wrote:
> > I wonder why I never saw this issue. Can you explain what you are encountering? Also, adding a unit test would help here to ensure we never run into this issue again.
> > 
> > Generally, the code for checkForDependencies ensures that all dependencies are available and none are disabled. Only then should potential dependencies be loaded. Your code now always tries to load dependencies, no? Even when one of them is disabled or not existing.
> > 
> > so -1 from my side until you give more information on what issue you are encountering and trying to solve.

>Generally, the code for checkForDependencies ensures that all dependencies are available and none are disabled.

Not exactly. It checks all loaded plugins to see if dependencies satisfied. And returns all interfaces that's not yet loaded/not available

>Only then should potential dependencies be loaded. 

Yes.

>Your code now always tries to load dependencies, no?

No. That's what happens now. Run KDevelop with debug channels enabled and search for "Missing dependencies:" messages. You won't find any because checkForDependencies checks all available plugins instead of loaded ones. So at the point after checkForDependencies call there could be some plugins that not yet loaded. And then loadDependencies tries to load all dependendencies (even if some are already loaded), and only because of that everything works.

>I wonder why I never saw this issue.

Because currently everything works

>so -1 from my side until you give more information on what issue you are encountering and trying to solve

Some plugins didn't load for me. So I began investigating and encountered this weird behaviour. But in the end turned out that they didn't load because weren't rebuild properly.

>Also, adding a unit test would help here to ensure we never run into this issue again.

I don't think we need this. If it dosn't work some plugins won't load -> some unit tests won't pass.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121775/#review73014
-----------------------------------------------------------


On Jan. 5, 2015, 4:30 p.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121775/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2015, 4:30 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> It fixes 2 bugs:
> 
> 1) checkForDependencies returns false if there are some dependencies, but the code that calls this method threats false return code as an error, so it doesn't try to load any dependencies at all.
> 2) checkForDependencies must check loaded plugins (instead of all available) for satisfied dependencies. As a result currently it almost always returns true
> 
> So because of these 2 bugs interconnected currently everything works.
> 
> 
> Diffs
> -----
> 
>   shell/plugincontroller.h 89b0b8b 
>   shell/plugincontroller.cpp f70fc64 
> 
> Diff: https://git.reviewboard.kde.org/r/121775/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150105/74fc546f/attachment.html>