From a2bdc12f2574e2a129d1a931f8bda173f02491fa Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 16 Jan 2018 10:42:46 +0100 Subject: [PATCH 1/8] [libcalamares] Drop unused plugin defines - Remove some superfluous intermediate defines - baseFactory was not used (always Calamares::PluginFactory) - Move DECLARATION and DEFINITIONS apart - CALAMARES_PLUGIN_FACTORY_DEFINITION was redefined (identically) - CALAMARES_PLUGIN_FACTORY_DECLARATION was redefined (identically) - __VA_ARGS__ was constant --- src/libcalamares/utils/PluginFactory.h | 85 ++++++-------------------- 1 file changed, 18 insertions(+), 67 deletions(-) diff --git a/src/libcalamares/utils/PluginFactory.h b/src/libcalamares/utils/PluginFactory.h index 8e6704a20..456802555 100644 --- a/src/libcalamares/utils/PluginFactory.h +++ b/src/libcalamares/utils/PluginFactory.h @@ -37,71 +37,6 @@ class PluginFactoryPrivate; #define CalamaresPluginFactory_iid "io.calamares.PluginFactory" -#define CALAMARES_PLUGIN_FACTORY_DECLARATION_WITH_BASEFACTORY_SKEL(name, baseFactory, ...) \ - class name : public Calamares::PluginFactory \ - { \ - Q_OBJECT \ - Q_INTERFACES(Calamares::PluginFactory) \ - __VA_ARGS__ \ - public: \ - explicit name(); \ - ~name(); \ - private: \ - void init(); \ - }; - -#define CALAMARES_PLUGIN_FACTORY_DECLARATION_WITH_BASEFACTORY(name, baseFactory) \ - CALAMARES_PLUGIN_FACTORY_DECLARATION_WITH_BASEFACTORY_SKEL(name, baseFactory, Q_PLUGIN_METADATA(IID CalamaresPluginFactory_iid)) - -#define CALAMARES_PLUGIN_FACTORY_DEFINITION_WITH_BASEFACTORY(name, baseFactory, pluginRegistrations) \ - name::name() \ - { \ - pluginRegistrations \ - } \ - name::~name() {} - -#define CALAMARES_PLUGIN_FACTORY_WITH_BASEFACTORY(name, baseFactory, pluginRegistrations) \ - CALAMARES_PLUGIN_FACTORY_DECLARATION_WITH_BASEFACTORY(name, baseFactory) \ - CALAMARES_PLUGIN_FACTORY_DEFINITION_WITH_BASEFACTORY(name, baseFactory, pluginRegistrations) - -#define CALAMARES_PLUGIN_FACTORY_DECLARATION(name) CALAMARES_PLUGIN_FACTORY_DECLARATION_WITH_BASEFACTORY(name, Calamares::PluginFactory) -#define CALAMARES_PLUGIN_FACTORY_DEFINITION(name, pluginRegistrations) CALAMARES_PLUGIN_FACTORY_DEFINITION_WITH_BASEFACTORY(name, Calamares::PluginFactory, pluginRegistrations) - -/** - * \relates PluginFactory - * - * Create a PluginFactory subclass and export it as the root plugin object. - * - * \param name The name of the PluginFactory derived class. - * - * \param pluginRegistrations Code to be inserted into the constructor of the - * class. Usually a series of registerPlugin() calls. - * - * Example: - * \code - * #include - * #include - * - * class MyPlugin : public PluginInterface - * { - * public: - * MyPlugin(QObject *parent, const QVariantList &args) - * : PluginInterface(parent) - * {} - * }; - * - * CALAMARES_PLUGIN_FACTORY(MyPluginFactory, - * registerPlugin(); - * ) - * - * #include - * \endcode - * - * \see CALAMARES_PLUGIN_FACTORY_DECLARATION - * \see CALAMARES_PLUGIN_FACTORY_DEFINITION - */ -#define CALAMARES_PLUGIN_FACTORY(name, pluginRegistrations) CALAMARES_PLUGIN_FACTORY_WITH_BASEFACTORY(name, Calamares::PluginFactory, pluginRegistrations) - /** * \relates PluginFactory * @@ -113,7 +48,18 @@ class PluginFactoryPrivate; * \see CALAMARES_PLUGIN_FACTORY * \see CALAMARES_PLUGIN_FACTORY_DEFINITION */ -#define CALAMARES_PLUGIN_FACTORY_DECLARATION(name) CALAMARES_PLUGIN_FACTORY_DECLARATION_WITH_BASEFACTORY(name, Calamares::PluginFactory) +#define CALAMARES_PLUGIN_FACTORY_DECLARATION(name) \ + class name : public Calamares::PluginFactory \ + { \ + Q_OBJECT \ + Q_INTERFACES(Calamares::PluginFactory) \ + Q_PLUGIN_METADATA(IID CalamaresPluginFactory_iid) \ + public: \ + explicit name(); \ + ~name(); \ + private: \ + void init(); \ + }; /** * \relates PluginFactory @@ -128,7 +74,12 @@ class PluginFactoryPrivate; * \see CALAMARES_PLUGIN_FACTORY * \see CALAMARES_PLUGIN_FACTORY_DECLARATION */ -#define CALAMARES_PLUGIN_FACTORY_DEFINITION(name, pluginRegistrations) CALAMARES_PLUGIN_FACTORY_DEFINITION_WITH_BASEFACTORY(name, Calamares::PluginFactory, pluginRegistrations) +#define CALAMARES_PLUGIN_FACTORY_DEFINITION(name, pluginRegistrations) \ + name::name() \ + { \ + pluginRegistrations \ + } \ + name::~name() {} namespace Calamares { From 0020fd885c0391498adfa0665d357c5f5ee38744 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 16 Jan 2018 11:00:39 +0100 Subject: [PATCH 2/8] [libcalamares] Remove unused extern declaration --- src/libcalamares/utils/PluginFactory.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libcalamares/utils/PluginFactory.cpp b/src/libcalamares/utils/PluginFactory.cpp index 1096ed343..26db804b0 100644 --- a/src/libcalamares/utils/PluginFactory.cpp +++ b/src/libcalamares/utils/PluginFactory.cpp @@ -29,8 +29,6 @@ Q_GLOBAL_STATIC( QObjectCleanupHandler, factorycleanup ) -extern int kLibraryDebugArea(); - namespace Calamares { From 510af704d8e2f0c65b7d5895148ed397320e39a6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 16 Jan 2018 11:05:30 +0100 Subject: [PATCH 3/8] [libcalamares] Improve documentation - reference to _WITH_JSON is bogus copy-replace from other code - fix style of sample code. --- src/libcalamares/utils/PluginFactory.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/utils/PluginFactory.h b/src/libcalamares/utils/PluginFactory.h index 456802555..7a55cc711 100644 --- a/src/libcalamares/utils/PluginFactory.h +++ b/src/libcalamares/utils/PluginFactory.h @@ -111,13 +111,11 @@ namespace Calamares * T(QObject *parent, const QVariantList &args) * \endcode * - * You should typically use either CALAMARES_PLUGIN_FACTORY() or - * CALAMARES_PLUGIN_FACTORY_WITH_JSON() in your plugin code to create the factory. The - * typical pattern is + * You should typically use CALAMARES_PLUGIN_FACTORY_DEFINITION() in your plugin code to + * create the factory. The pattern is * * \code - * #include - * #include + * #include "utils/PluginFactory.h" * * class MyPlugin : public PluginInterface * { @@ -127,10 +125,9 @@ namespace Calamares * {} * }; * - * CALAMARES_PLUGIN_FACTORY(MyPluginFactory, + * CALAMARES_PLUGIN_FACTORY_DEFINITION(MyPluginFactory, * registerPlugin(); * ) - * #include * \endcode * * If you want to load a library use KPluginLoader. From 86b899566e1c51fe7d618d5da2de606e521ac46d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 16 Jan 2018 12:00:58 +0100 Subject: [PATCH 4/8] [libcalamares] Silence compiler warnings about PluginFactories - d_ptr shadows QObject d_ptr, which clang complains about - rename, and don't use Q_D and similar because it messes with internals. --- src/libcalamares/utils/PluginFactory.cpp | 23 +++++++++-------------- src/libcalamares/utils/PluginFactory.h | 4 ++-- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/libcalamares/utils/PluginFactory.cpp b/src/libcalamares/utils/PluginFactory.cpp index 26db804b0..d53b6474f 100644 --- a/src/libcalamares/utils/PluginFactory.cpp +++ b/src/libcalamares/utils/PluginFactory.cpp @@ -33,41 +33,38 @@ namespace Calamares { PluginFactory::PluginFactory() - : d_ptr( new PluginFactoryPrivate ) + : pd_ptr( new PluginFactoryPrivate ) { - Q_D( PluginFactory ); - d->q_ptr = this; + pd_ptr->q_ptr = this; factorycleanup()->add( this ); } PluginFactory::PluginFactory( PluginFactoryPrivate& d ) - : d_ptr( &d ) + : pd_ptr( &d ) { factorycleanup()->add( this ); } PluginFactory::~PluginFactory() { - delete d_ptr; + delete pd_ptr; } void PluginFactory::doRegisterPlugin( const QString& keyword, const QMetaObject* metaObject, CreateInstanceFunction instanceFunction ) { - Q_D( PluginFactory ); - Q_ASSERT( metaObject ); // we allow different interfaces to be registered without keyword if ( !keyword.isEmpty() ) { - if ( d->createInstanceHash.contains( keyword ) ) + if ( pd_ptr->createInstanceHash.contains( keyword ) ) qWarning() << "A plugin with the keyword" << keyword << "was already registered. A keyword must be unique!"; - d->createInstanceHash.insert( keyword, PluginFactoryPrivate::Plugin( metaObject, instanceFunction ) ); + pd_ptr->createInstanceHash.insert( keyword, PluginFactoryPrivate::Plugin( metaObject, instanceFunction ) ); } else { - const QList clashes( d->createInstanceHash.values( keyword ) ); + const QList clashes( pd_ptr->createInstanceHash.values( keyword ) ); const QMetaObject* superClass = metaObject->superClass(); if ( superClass ) { @@ -94,17 +91,15 @@ void PluginFactory::doRegisterPlugin( const QString& keyword, const QMetaObject* } } } - d->createInstanceHash.insertMulti( keyword, PluginFactoryPrivate::Plugin( metaObject, instanceFunction ) ); + pd_ptr->createInstanceHash.insertMulti( keyword, PluginFactoryPrivate::Plugin( metaObject, instanceFunction ) ); } } QObject* PluginFactory::create( const char* iface, QWidget* parentWidget, QObject* parent, const QString& keyword ) { - Q_D( PluginFactory ); - QObject* obj = nullptr; - const QList candidates( d->createInstanceHash.values( keyword ) ); + const QList candidates( pd_ptr->createInstanceHash.values( keyword ) ); // for !keyword.isEmpty() candidates.count() is 0 or 1 for ( const PluginFactoryPrivate::Plugin& plugin : candidates ) diff --git a/src/libcalamares/utils/PluginFactory.h b/src/libcalamares/utils/PluginFactory.h index 7a55cc711..0ca7917c4 100644 --- a/src/libcalamares/utils/PluginFactory.h +++ b/src/libcalamares/utils/PluginFactory.h @@ -148,7 +148,7 @@ namespace Calamares class DLLEXPORT PluginFactory : public QObject { Q_OBJECT - Q_DECLARE_PRIVATE( PluginFactory ) + friend class PluginFactoryPrivate; public: /** * This constructor creates a factory for a plugin. @@ -249,7 +249,7 @@ protected: doRegisterPlugin( keyword, &T::staticMetaObject, instanceFunction ); } - PluginFactoryPrivate* const d_ptr; + PluginFactoryPrivate* const pd_ptr; /** * This function is called when the factory asked to create an Object. From abc69145287ca5395545c4d11f86ac01ef656b6b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 17 Jan 2018 06:18:43 -0500 Subject: [PATCH 5/8] [libcalamares] Enforce singleton-ness of CalamaresPython::Helper - unset instance pointer on destruction - make constructor private, and the instance accessor should create an instance if there isn't one. --- src/libcalamares/JobQueue.cpp | 3 --- src/libcalamares/PythonHelper.cpp | 4 +++- src/libcalamares/PythonHelper.h | 2 +- src/libcalamares/PythonJob.cpp | 6 ++++-- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/JobQueue.cpp b/src/libcalamares/JobQueue.cpp index 3a2e9ff03..b5bdf0543 100644 --- a/src/libcalamares/JobQueue.cpp +++ b/src/libcalamares/JobQueue.cpp @@ -41,9 +41,6 @@ public: , m_queue( queue ) , m_jobIndex( 0 ) { -#ifdef WITH_PYTHON - new CalamaresPython::Helper( this ); -#endif } void setJobs( const JobList& jobs ) diff --git a/src/libcalamares/PythonHelper.cpp b/src/libcalamares/PythonHelper.cpp index 4f2f30e76..f274ac276 100644 --- a/src/libcalamares/PythonHelper.cpp +++ b/src/libcalamares/PythonHelper.cpp @@ -231,7 +231,9 @@ Helper::Helper( QObject* parent ) } Helper::~Helper() -{} +{ + s_instance = nullptr; +} boost::python::dict diff --git a/src/libcalamares/PythonHelper.h b/src/libcalamares/PythonHelper.h index e552a869d..d48e5eaab 100644 --- a/src/libcalamares/PythonHelper.h +++ b/src/libcalamares/PythonHelper.h @@ -48,7 +48,6 @@ class Helper : public QObject { Q_OBJECT public: - explicit Helper( QObject* parent = nullptr ); virtual ~Helper(); boost::python::dict createCleanNamespace(); @@ -57,6 +56,7 @@ public: private: friend Helper* Calamares::PythonJob::helper(); + explicit Helper( QObject* parent = nullptr ); static Helper* s_instance; boost::python::object m_mainModule; diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index faf2fa72d..a9fe4463e 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -381,8 +381,10 @@ PythonJob::emitProgress( qreal progressValue ) CalamaresPython::Helper* PythonJob::helper() { - return CalamaresPython::Helper::s_instance; - + auto ptr = CalamaresPython::Helper::s_instance; + if (!ptr) + ptr = new CalamaresPython::Helper; + return ptr; } From 97fb83c74307fe0b0bffaa94be3310e0cfc2a007 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 17 Jan 2018 06:20:55 -0500 Subject: [PATCH 6/8] [libcalamares] Change debug logging of how job name is derived --- src/libcalamares/PythonJob.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index a9fe4463e..48682dbad 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -322,7 +322,7 @@ PythonJob::exec() } if ( !m_description.isEmpty() ) { - cDebug() << "Job" << prettyName() << "(func) ->" << m_description; + cDebug() << "Job description from pretty_name" << prettyName() << "=" << m_description; emit progress( 0 ); } } @@ -337,7 +337,7 @@ PythonJob::exec() auto i_newline = m_description.indexOf('\n'); if ( i_newline > 0 ) m_description.truncate( i_newline ); - cDebug() << "Job" << prettyName() << "(doc) ->" << m_description; + cDebug() << "Job description from __doc__" << prettyName() << "=" << m_description; emit progress( 0 ); } } From 845986d48fcfb5c95a67267c648040496ada4f4d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 17 Jan 2018 09:09:46 -0500 Subject: [PATCH 7/8] [libcalamaresui] Mark virtual QObject destructors override --- src/libcalamaresui/ViewManager.h | 2 +- src/libcalamaresui/modulesystem/ModuleManager.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcalamaresui/ViewManager.h b/src/libcalamaresui/ViewManager.h index fa80698ce..2e7e4df84 100644 --- a/src/libcalamaresui/ViewManager.h +++ b/src/libcalamaresui/ViewManager.h @@ -123,7 +123,7 @@ signals: private: explicit ViewManager( QObject* parent = nullptr ); - virtual ~ViewManager(); + virtual ~ViewManager() override; void insertViewStep( int before, ViewStep* step ); diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index 6621bdaf3..ed7700c40 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -42,7 +42,7 @@ class ModuleManager : public QObject Q_OBJECT public: explicit ModuleManager( const QStringList& paths, QObject* parent = nullptr ); - virtual ~ModuleManager(); + virtual ~ModuleManager() override; static ModuleManager* instance(); From 9c9486bb783a698375a306ba997ca8276be32ca4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 17 Jan 2018 09:16:47 -0500 Subject: [PATCH 8/8] [libcalamares] When ViewManager is destroyed, reset instance pointer --- src/libcalamaresui/ViewManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index 2807cf9f4..645be4371 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -100,6 +100,7 @@ ViewManager::ViewManager( QObject* parent ) ViewManager::~ViewManager() { m_widget->deleteLater(); + s_instance = nullptr; }