From 33a5e7d80441d993dac497f9ed16680037e6129d Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Tue, 31 Mar 2015 17:42:14 +0200 Subject: [PATCH] PluginManager: Behave more like a package manager Instead of implicitly disabling plugins if their dependencies are disabled, implicitly enable plugins if some enabled plugin needs it. That will avoid issues if people disable plugins (e.g. QmlJSTools et al) and we later add one of these as a dependency to another plugin (e.g. make QmakeProjectManager depend on QmlJSTools), which resulted in the previously enabled plugin being implicitly disabled. Enabling a plugin in About Plugins now asks for all required dependencies to be enabled as well. Disabling a plugin in About Plugins now asks for disabling all plugins that require it. Using the -noload command line option now disables all plugins that require it in addition. Using the -load command line option now implicitly enables all plugins that are required. Multiple -noload and -load options are handled in the order given on the command line. Task-number: QTCREATORBUG-9131 Change-Id: I0956106105060a7898a8992e0629009d5ec3ea4d Reviewed-by: Daniel Teske --- src/libs/extensionsystem/optionsparser.cpp | 5 + src/libs/extensionsystem/optionsparser.h | 4 +- .../extensionsystem/pluginerroroverview.cpp | 2 +- src/libs/extensionsystem/pluginmanager.cpp | 64 ++++++++- src/libs/extensionsystem/pluginmanager.h | 2 + src/libs/extensionsystem/pluginspec.cpp | 38 +++--- src/libs/extensionsystem/pluginspec.h | 6 +- src/libs/extensionsystem/pluginspec_p.h | 4 +- src/libs/extensionsystem/pluginview.cpp | 123 ++++++++++++------ src/libs/extensionsystem/pluginview.h | 2 +- 10 files changed, 176 insertions(+), 74 deletions(-) diff --git a/src/libs/extensionsystem/optionsparser.cpp b/src/libs/extensionsystem/optionsparser.cpp index c77edc88646..16b96f2e5c0 100644 --- a/src/libs/extensionsystem/optionsparser.cpp +++ b/src/libs/extensionsystem/optionsparser.cpp @@ -30,6 +30,8 @@ #include "optionsparser.h" +#include "pluginmanager.h" +#include "pluginmanager_p.h" #include "pluginspec_p.h" #include @@ -173,6 +175,9 @@ bool OptionsParser::checkForNoLoadOption() m_hasError = true; } else { spec->d->setForceDisabled(true); + // recursively disable all plugins that require this plugin + foreach (PluginSpec *dependantSpec, PluginManager::pluginsRequiringPlugin(spec)) + dependantSpec->d->setForceDisabled(true); m_isDependencyRefreshNeeded = true; } } diff --git a/src/libs/extensionsystem/optionsparser.h b/src/libs/extensionsystem/optionsparser.h index e864f889601..8edbac317ac 100644 --- a/src/libs/extensionsystem/optionsparser.h +++ b/src/libs/extensionsystem/optionsparser.h @@ -31,14 +31,14 @@ #ifndef OPTIONSPARSER_H #define OPTIONSPARSER_H -#include "pluginmanager_p.h" - #include #include namespace ExtensionSystem { namespace Internal { +class PluginManagerPrivate; + class OptionsParser { public: diff --git a/src/libs/extensionsystem/pluginerroroverview.cpp b/src/libs/extensionsystem/pluginerroroverview.cpp index 39f69cbfbaa..c6a3852362b 100644 --- a/src/libs/extensionsystem/pluginerroroverview.cpp +++ b/src/libs/extensionsystem/pluginerroroverview.cpp @@ -48,7 +48,7 @@ PluginErrorOverview::PluginErrorOverview(QWidget *parent) : foreach (PluginSpec *spec, PluginManager::plugins()) { // only show errors on startup if plugin is enabled. - if (spec->hasError() && spec->isEnabledBySettings() && !spec->isDisabledIndirectly()) { + if (spec->hasError() && spec->isEffectivelyEnabled()) { QListWidgetItem *item = new QListWidgetItem(spec->name()); item->setData(Qt::UserRole, qVariantFromValue(spec)); m_ui->pluginList->addItem(item); diff --git a/src/libs/extensionsystem/pluginmanager.cpp b/src/libs/extensionsystem/pluginmanager.cpp index 8341be84ea6..459141eca2a 100644 --- a/src/libs/extensionsystem/pluginmanager.cpp +++ b/src/libs/extensionsystem/pluginmanager.cpp @@ -376,12 +376,62 @@ bool PluginManager::hasError() { foreach (PluginSpec *spec, plugins()) { // only show errors on startup if plugin is enabled. - if (spec->hasError() && spec->isEnabledBySettings() && !spec->isDisabledIndirectly()) + if (spec->hasError() && spec->isEffectivelyEnabled()) return true; } return false; } +/*! + Returns all plugins that require \a spec to be loaded. Recurses into dependencies. + */ +QSet PluginManager::pluginsRequiringPlugin(PluginSpec *spec) +{ + QSet dependingPlugins; + dependingPlugins.insert(spec); + foreach (PluginSpec *checkSpec, d->loadQueue()) { + QHashIterator depIt(checkSpec->dependencySpecs()); + while (depIt.hasNext()) { + depIt.next(); + if (depIt.key().type != PluginDependency::Required) + continue; + if (dependingPlugins.contains(depIt.value())) { + dependingPlugins.insert(checkSpec); + break; // no use to check other dependencies, continue with load queue + } + } + } + dependingPlugins.remove(spec); + return dependingPlugins; +} + +/*! + Returns all plugins that \a spec requires to be loaded. Recurses into dependencies. + */ +QSet PluginManager::pluginsRequiredByPlugin(PluginSpec *spec) +{ + QSet recursiveDependencies; + recursiveDependencies.insert(spec); + QList queue; + queue.append(spec); + while (!queue.isEmpty()) { + PluginSpec *checkSpec = queue.takeFirst(); + QHashIterator depIt(checkSpec->dependencySpecs()); + while (depIt.hasNext()) { + depIt.next(); + if (depIt.key().type != PluginDependency::Required) + continue; + PluginSpec *depSpec = depIt.value(); + if (!recursiveDependencies.contains(depSpec)) { + recursiveDependencies.insert(depSpec); + queue.append(depSpec); + } + } + } + recursiveDependencies.remove(spec); + return recursiveDependencies; +} + /*! Shuts down and deletes all plugins. */ @@ -647,10 +697,10 @@ static inline void formatOption(QTextStream &str, void PluginManager::formatOptions(QTextStream &str, int optionIndentation, int descriptionIndentation) { formatOption(str, QLatin1String(OptionsParser::LOAD_OPTION), - QLatin1String("plugin"), QLatin1String("Load "), + QLatin1String("plugin"), QLatin1String("Load and all plugins that it requires"), optionIndentation, descriptionIndentation); formatOption(str, QLatin1String(OptionsParser::NO_LOAD_OPTION), - QLatin1String("plugin"), QLatin1String("Do not load "), + QLatin1String("plugin"), QLatin1String("Do not load and all plugins that require it"), optionIndentation, descriptionIndentation); formatOption(str, QLatin1String(OptionsParser::PROFILE_OPTION), QString(), QLatin1String("Profile plugin loading"), @@ -1422,11 +1472,15 @@ void PluginManagerPrivate::readPluginPaths() void PluginManagerPrivate::resolveDependencies() { foreach (PluginSpec *spec, pluginSpecs) { + spec->d->enabledIndirectly = false; // reset, is recalculated below spec->d->resolveDependencies(pluginSpecs); } - foreach (PluginSpec *spec, loadQueue()) { - spec->d->disableIndirectlyIfDependencyDisabled(); + QListIterator it(loadQueue()); + it.toBack(); + while (it.hasPrevious()) { + PluginSpec *spec = it.previous(); + spec->d->enableDependenciesIndirectly(); } } diff --git a/src/libs/extensionsystem/pluginmanager.h b/src/libs/extensionsystem/pluginmanager.h index 72b4402da83..7b0ee066f4b 100644 --- a/src/libs/extensionsystem/pluginmanager.h +++ b/src/libs/extensionsystem/pluginmanager.h @@ -124,6 +124,8 @@ public: static QList plugins(); static QHash pluginCollections(); static bool hasError(); + static QSet pluginsRequiringPlugin(PluginSpec *spec); + static QSet pluginsRequiredByPlugin(PluginSpec *spec); // Settings static void setSettings(QSettings *settings); diff --git a/src/libs/extensionsystem/pluginspec.cpp b/src/libs/extensionsystem/pluginspec.cpp index fe996372ff0..efb83fe42f0 100644 --- a/src/libs/extensionsystem/pluginspec.cpp +++ b/src/libs/extensionsystem/pluginspec.cpp @@ -284,8 +284,9 @@ bool PluginSpec::isEnabledByDefault() const Returns whether the plugin should be loaded at startup, taking into account the default enabled state, and the user's settings. - \note This function returns true even if a plugin is disabled because its - dependencies were not loaded, or an error occurred during loading it. + \note This function might return false even if the plugin is loaded as a requirement of another + enabled plugin. + \sa PluginSpec::isEffectivelyEnabled */ bool PluginSpec::isEnabledBySettings() const { @@ -294,24 +295,25 @@ bool PluginSpec::isEnabledBySettings() const /*! Returns whether the plugin is loaded at startup. - \see PluginSpec::isEnabled + \see PluginSpec::isEnabledBySettings */ bool PluginSpec::isEffectivelyEnabled() const { - if (d->disabledIndirectly - || (!d->enabledBySettings && !d->forceEnabled) - || d->forceDisabled) { + if (!isAvailableForHostPlatform()) return false; - } - return isAvailableForHostPlatform(); + if (isForceEnabled() || isEnabledIndirectly()) + return true; + if (isForceDisabled()) + return false; + return isEnabledBySettings(); } /*! Returns true if loading was not done due to user unselecting this plugin or its dependencies. */ -bool PluginSpec::isDisabledIndirectly() const +bool PluginSpec::isEnabledIndirectly() const { - return d->disabledIndirectly; + return d->enabledIndirectly; } /*! @@ -486,7 +488,7 @@ PluginSpecPrivate::PluginSpecPrivate(PluginSpec *spec) experimental(false), enabledByDefault(true), enabledBySettings(true), - disabledIndirectly(false), + enabledIndirectly(false), forceEnabled(false), forceDisabled(false), plugin(0), @@ -910,24 +912,18 @@ bool PluginSpecPrivate::resolveDependencies(const QList &specs) return true; } -void PluginSpecPrivate::disableIndirectlyIfDependencyDisabled() +void PluginSpecPrivate::enableDependenciesIndirectly() { - if (!enabledBySettings) + if (!q->isEffectivelyEnabled()) // plugin not enabled, nothing to do return; - - if (disabledIndirectly) - return; - QHashIterator it(dependencySpecs); while (it.hasNext()) { it.next(); if (it.key().type != PluginDependency::Required) continue; PluginSpec *dependencySpec = it.value(); - if (!dependencySpec->isEffectivelyEnabled()) { - disabledIndirectly = true; - break; - } + if (!dependencySpec->isEffectivelyEnabled()) + dependencySpec->d->enabledIndirectly = true; } } diff --git a/src/libs/extensionsystem/pluginspec.h b/src/libs/extensionsystem/pluginspec.h index 9c9d49328f4..70403807c4f 100644 --- a/src/libs/extensionsystem/pluginspec.h +++ b/src/libs/extensionsystem/pluginspec.h @@ -53,7 +53,7 @@ class PluginManagerPrivate; } // Internal class IPlugin; -class PluginItem; +class PluginView; struct EXTENSIONSYSTEM_EXPORT PluginDependency { @@ -104,7 +104,7 @@ public: bool isEnabledByDefault() const; bool isEnabledBySettings() const; bool isEffectivelyEnabled() const; - bool isDisabledIndirectly() const; + bool isEnabledIndirectly() const; bool isForceEnabled() const; bool isForceDisabled() const; QVector dependencies() const; @@ -137,7 +137,7 @@ private: PluginSpec(); Internal::PluginSpecPrivate *d; - friend class PluginItem; + friend class PluginView; friend class Internal::OptionsParser; friend class Internal::PluginManagerPrivate; friend class Internal::PluginSpecPrivate; diff --git a/src/libs/extensionsystem/pluginspec_p.h b/src/libs/extensionsystem/pluginspec_p.h index 19ed4835b71..56787597059 100644 --- a/src/libs/extensionsystem/pluginspec_p.h +++ b/src/libs/extensionsystem/pluginspec_p.h @@ -88,7 +88,7 @@ public: QRegExp platformSpecification; QVector dependencies; bool enabledBySettings; - bool disabledIndirectly; + bool enabledIndirectly; bool forceEnabled; bool forceDisabled; @@ -107,7 +107,7 @@ public: static bool isValidVersion(const QString &version); static int versionCompare(const QString &version1, const QString &version2); - void disableIndirectlyIfDependencyDisabled(); + void enableDependenciesIndirectly(); bool readMetaData(const QJsonObject &metaData); diff --git a/src/libs/extensionsystem/pluginview.cpp b/src/libs/extensionsystem/pluginview.cpp index 1e1476bdbc7..a918c5facb7 100644 --- a/src/libs/extensionsystem/pluginview.cpp +++ b/src/libs/extensionsystem/pluginview.cpp @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -107,8 +108,20 @@ public: case NameColumn: if (role == Qt::DisplayRole) return m_spec->name(); - if (role == Qt::ToolTipRole) - return QDir::toNativeSeparators(m_spec->filePath()); + if (role == Qt::ToolTipRole) { + QString toolTip; + if (!m_spec->isAvailableForHostPlatform()) + toolTip = PluginView::tr("Path: %1\nPlugin is not available on this platform."); + else if (m_spec->isEnabledIndirectly()) + toolTip = PluginView::tr("Path: %1\nPlugin is enabled as dependency of an enabled plugin."); + else if (m_spec->isForceEnabled()) + toolTip = PluginView::tr("Path: %1\nPlugin is enabled by command line argument."); + else if (m_spec->isForceDisabled()) + toolTip = PluginView::tr("Path: %1\nPlugin is disabled by command line argument."); + else + toolTip = PluginView::tr("Path: %1"); + return toolTip.arg(QDir::toNativeSeparators(m_spec->filePath())); + } if (role == Qt::DecorationRole) { bool ok = !m_spec->hasError(); QIcon i = icon(ok ? OkIcon : ErrorIcon); @@ -153,24 +166,14 @@ public: bool setData(int column, const QVariant &data, int role) { - if (column == LoadedColumn && role == Qt::CheckStateRole) { - m_spec->d->setEnabledBySettings(data.toBool()); - updateColumn(column); - parent()->updateColumn(column); - emit m_view->pluginSettingsChanged(m_spec); - return true; - } + if (column == LoadedColumn && role == Qt::CheckStateRole) + return m_view->setPluginsEnabled(QSet() << m_spec, data.toBool()); return false; } bool isEnabled() const { - if (m_spec->isRequired() || !m_spec->isAvailableForHostPlatform()) - return false; - foreach (PluginSpec *spec, m_view->m_pluginDependencies.value(m_spec)) - if (!spec->isEnabledBySettings()) - return false; - return true; + return m_spec->isAvailableForHostPlatform() && !m_spec->isRequired(); } Qt::ItemFlags flags(int column) const @@ -245,10 +248,13 @@ public: bool setData(int column, const QVariant &data, int role) { if (column == LoadedColumn && role == Qt::CheckStateRole) { + QSet affectedPlugins; foreach (TreeItem *item, children()) - static_cast(item)->setData(column, data, role); - update(); - return true; + affectedPlugins.insert(static_cast(item)->m_spec); + if (m_view->setPluginsEnabled(affectedPlugins, data.toBool())) { + update(); + return true; + } } return false; } @@ -336,29 +342,8 @@ PluginSpec *PluginView::pluginForIndex(const QModelIndex &index) const return item ? item->m_spec: 0; } -static void queryDependendPlugins(PluginSpec *spec, QSet *dependencies) -{ - QHashIterator it(spec->dependencySpecs()); - while (it.hasNext()) { - it.next(); - PluginSpec *dep = it.value(); - if (!dependencies->contains(dep)) { - dependencies->insert(dep); - queryDependendPlugins(dep, dependencies); - } - } -} - void PluginView::updatePlugins() { - // Dependencies. - m_pluginDependencies.clear(); - foreach (PluginSpec *spec, PluginManager::loadQueue()) { - QSet deps; - queryDependendPlugins(spec, &deps); - m_pluginDependencies[spec] = deps; - } - // Model. m_model->removeItems(); @@ -392,4 +377,64 @@ void PluginView::updatePlugins() m_categoryView->expandAll(); } +static QString pluginListString(const QSet &plugins) +{ + QStringList names = Utils::transform(plugins, &PluginSpec::name); + names.sort(); + return names.join(QLatin1Char('\n')); +} + +bool PluginView::setPluginsEnabled(const QSet &plugins, bool enable) +{ + QSet additionalPlugins; + if (enable) { + foreach (PluginSpec *spec, plugins) { + foreach (PluginSpec *other, PluginManager::pluginsRequiredByPlugin(spec)) { + if (!other->isEnabledBySettings()) + additionalPlugins.insert(other); + } + } + additionalPlugins.subtract(plugins); + if (!additionalPlugins.isEmpty()) { + if (QMessageBox::question(this, tr("Enabling Plugins"), + tr("Enabling\n%1\nwill also enable the following plugins:\n\n%2") + .arg(pluginListString(plugins)) + .arg(pluginListString(additionalPlugins)), + QMessageBox::Ok | QMessageBox::Cancel, + QMessageBox::Ok) != QMessageBox::Ok) + return false; + } + } else { + foreach (PluginSpec *spec, plugins) { + foreach (PluginSpec *other, PluginManager::pluginsRequiringPlugin(spec)) { + if (other->isEnabledBySettings()) + additionalPlugins.insert(other); + } + } + additionalPlugins.subtract(plugins); + if (!additionalPlugins.isEmpty()) { + if (QMessageBox::question(this, tr("Disabling Plugins"), + tr("Disabling\n%1\nwill also disable the following plugins:\n\n%2") + .arg(pluginListString(plugins)) + .arg(pluginListString(additionalPlugins)), + QMessageBox::Ok | QMessageBox::Cancel, + QMessageBox::Ok) != QMessageBox::Ok) + return false; + } + } + + QSet affectedPlugins = plugins + additionalPlugins; + foreach (PluginSpec *spec, affectedPlugins) { + PluginItem *item = m_model->findItemAtLevel(2, [spec](PluginItem *item) { + return item->m_spec == spec; + }); + QTC_ASSERT(item, continue); + spec->d->setEnabledBySettings(enable); + item->updateColumn(LoadedColumn); + item->parent()->updateColumn(LoadedColumn); + emit pluginSettingsChanged(spec); + } + return true; +} + } // namespace ExtensionSystem diff --git a/src/libs/extensionsystem/pluginview.h b/src/libs/extensionsystem/pluginview.h index 4cf31c9176c..64d2c595e94 100644 --- a/src/libs/extensionsystem/pluginview.h +++ b/src/libs/extensionsystem/pluginview.h @@ -72,6 +72,7 @@ signals: private: PluginSpec *pluginForIndex(const QModelIndex &index) const; void updatePlugins(); + bool setPluginsEnabled(const QSet &plugins, bool enable); Utils::TreeView *m_categoryView; Utils::TreeModel *m_model; @@ -79,7 +80,6 @@ private: friend class CollectionItem; friend class PluginItem; - QHash> m_pluginDependencies; }; } // namespae ExtensionSystem