Allowing Plugin Interdependency and Improving Plugin Management

[ollydbg]

On my test, install a bundle(such as a file named ProjectsImporter-1.1.cbplugin which I exported from C::B) failed in function bool PluginManager::InstallPlugin(const wxString& pluginName, bool forAllUsers, bool askForConfirmation)

Code

    // extract plugin from bundle
    if (!ExtractFile(actualName,
                    localName,
                    pluginFilename))
        return false;

Not sure the reason.

EDIT: uninstall an unenabled plugin also failed without any dialog or message box.

[ollydbg]

On my test, install a bundle(such as a file named ProjectsImporter-1.1.cbplugin which I exported from C::B) failed in function bool PluginManager::InstallPlugin(const wxString& pluginName, bool forAllUsers, bool askForConfirmation)

Code

    // extract plugin from bundle
    if (!ExtractFile(actualName,
                    localName,
                    pluginFilename))
        return false;

Not sure the reason.

EDIT: uninstall an unenabled plugin also failed without any dialog or message box.

For an disabled plugin.

Code

void PluginsConfigurationDlg::OnUninstall(cb_unused wxCommandEvent& event)
{
    wxListCtrl* list = XRCCTRL(*this, "lstPlugins", wxListCtrl);
    if (list->GetSelectedItemCount() == 0)
        return;

    wxBusyCursor busy;

    long sel = -1;
    wxString failure;
    while (true)
    {
        sel = list->GetNextItem(sel, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED);
        if (sel == -1)
            break;

        PluginElement* elem = (PluginElement*)list->GetItemData(sel);
        if (elem && elem->plugin)
        {
            if (!Manager::Get()->GetPluginManager()->UninstallPlugin(elem))
                failure << elem->info.title << _T('\n');
        }
    }

    FillList();
    if (!failure.IsEmpty())
        cbMessageBox(_("One or more plugins were not un-installed successfully:\n\n") + failure, _("Warning"), wxICON_WARNING, this);
}

elem->plugin is NULL, but we still need to uninstall it, right?

[ollydbg]

Patch updated to handle safemode

, you have version 2 of your patch here, but I'm testing your version 1 patch...

[ollydbg]

My new patch is out, which is based on dmoore's version 2 patch, it can uninstall an unloaded plugin.
Code::Blocks / Tickets / #300 Only load plugin shared library objects for plugins that are enabled

But I still have an issue that I can't install an exported plugin bundle, reported in my previous posts.

[ollydbg]

But I still have an issue that I can't install an exported plugin bundle, reported in my previous posts.

OK, I found the reason, I see that in my exported plugin bundle, the dll and zip file name are all in lowercase, but the bundle name is "ProjectsImporter-1.1.cbplugin", so that the name are not matched. I'm bulding C::B against wx trunk(wx31)

This does not happens in our 16.01 release(it will generated ProjectsImporter.dll and ProjectsImporter.zip file)

[ollydbg]

Return to the original issue when install a new plugin

Code

    ScanForPlugins(pluginDir);
    LoadAllPlugins();
    cbPlugin* plugin = FindPluginByName(basename); //THIS DOESN'T WORK RIGHT...
    const PluginInfo* info = GetPluginInfo(plugin);

Here, I see the plugin is NULL. Because I think LoadAllPlugins() only load the manifest(xml), but the dll is not loaded yet.

EDIT:
Maybe, we need to just Load the plugin as in the code:

Code

void PluginsConfigurationDlg::OnToggle(wxCommandEvent& event)
{
    wxListCtrl* list = XRCCTRL(*this, "lstPlugins", wxListCtrl);
    if (list->GetSelectedItemCount() == 0)
        return;
    bool isEnable = event.GetId() == XRCID("btnEnable");

    wxBusyCursor busy;

    wxProgressDialog pd(wxString::Format(_("%s plugin(s)"), isEnable ? _("Enabling") : _("Disabling")),
                        _T("A description wide enough for the dialog ;)"),
                        list->GetSelectedItemCount(),
                        this,
                        wxPD_AUTO_HIDE | wxPD_APP_MODAL | wxPD_CAN_ABORT);

    int count = 0;
    long sel = -1;
    bool skip = false;
    while (true)
    {
        sel = list->GetNextItem(sel, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED);
        if (sel == -1)
            break;

        PluginElement* elem = (PluginElement*)list->GetItemData(sel);
        if (elem)
        {
            pd.Update(++count,
                        wxString::Format(_("%s \"%s\"..."), isEnable ? _("Enabling") : _("Disabling"), elem->info.title.c_str()),
                        &skip);
            if (skip)
                break;

            if (!isEnable && elem->plugin && elem->plugin->IsAttached())
            {
                Manager::Get()->GetPluginManager()->DetachPlugin(elem->plugin);
                Manager::Get()->GetPluginManager()->UnloadPlugin(elem->plugin);
            }
            else if (isEnable && !(elem->plugin && elem->plugin->IsAttached()))
            {
                if (!elem->plugin)
                    Manager::Get()->GetPluginManager()->LoadPlugin(elem->fileName, elem);
                Manager::Get()->GetPluginManager()->AttachPlugin(elem->plugin, true); // ignore safe-mode here
            }
            else
                continue;

See the above code:

Code

                if (!elem->plugin)
                    Manager::Get()->GetPluginManager()->LoadPlugin(elem->fileName, elem);

[ollydbg]

I have find the solution to solve the "THIS DOESN'T WORK RIGHT" problem in dmoore's original v2 patch.

Please have a look and test the v4.patch in Code::Blocks / Tickets / #300 Only load plugin shared library objects for plugins that are enabled.

[blauzahn]

just glanced over the patch.

Code

void PluginManager::AddUnloadedPluginElem(const wxString &path, const PluginInfo &info)
{
    PluginElement *plugElem  = new PluginElement;
    plugElem->fileName = path;
    plugElem->info = info;
    plugElem->library = 0;
    plugElem->freeProc = 0;
    plugElem->plugin = 0;
    m_Plugins.Add(plugElem);
}

I would prefer nullptr instead of 0. At other places in the patch as well.

The method name AddUnloadedPluginElement reveals, that it is doing actually 2 things:
Creating a PluginElement and adding it. To me, the creation details belong to PluginElement.
I'd convert it into a ctor or maybe into a static method to give it a name like:

Code

static PluginElement PluginElement::unloaded(const wxString &path, const PluginInfo &info);

That way, the void method wil evaporate into a mere call :

Code

m_Plugins.Add( new PluginElement::unloaded(path, info));

where at least the creation is more exception safe and the newly created element is given
directly to the array. I always shiver when I see naked new and delete.

[oBFusCATed]

I always shiver when I see naked new and delete.

Welcome to the real world outside of the committee...
Except for the nullptrs everything else is not that bad in this function.

[frithjofh]

@obfuscated

is there a special reason, you don't implement and use a constructor for PluginElement taking a String and a PluginInfo and setting the other members to nullptr in the initializer list?

[oBFusCATed]

Because it doesn't matter that much.