From c8db50cb20bb1ed9b3180ec87d2f34d293931375 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 10 Jan 2019 13:54:41 -0800 Subject: [PATCH 1/6] start up logger before loading plugins. put a plugin-interface version into the meta data of each plugin. don't load plugins that don't match the current version. --- interface/src/Application.cpp | 6 +++--- interface/src/Application.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 134c375b56..b27403f56c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -724,6 +724,8 @@ const QString TEST_RESULTS_LOCATION_COMMAND{ "--testResultsLocation" }; bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) { const char** constArgv = const_cast(argv); + qInstallMessageHandler(messageHandler); + // HRS: I could not figure out how to move these any earlier in startup, so when using this option, be sure to also supply // --allowMultipleInstances auto reportAndQuit = [&](const char* commandSwitch, std::function report) { @@ -974,6 +976,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo QApplication(argc, argv), _window(new MainWindow(desktop())), _sessionRunTimer(startupTimer), + _logger(new FileLogger(this)), _previousSessionCrashed(setupEssentials(argc, argv, runningMarkerExisted)), _entitySimulation(new PhysicalEntitySimulation()), _physicsEngine(new PhysicsEngine(Vectors::ZERO)), @@ -1063,9 +1066,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo installNativeEventFilter(&MyNativeEventFilter::getInstance()); #endif - _logger = new FileLogger(this); - qInstallMessageHandler(messageHandler); - QFontDatabase::addApplicationFont(PathUtils::resourcesPath() + "styles/Inconsolata.otf"); QFontDatabase::addApplicationFont(PathUtils::resourcesPath() + "fonts/fontawesome-webfont.ttf"); QFontDatabase::addApplicationFont(PathUtils::resourcesPath() + "fonts/hifi-glyphs.ttf"); diff --git a/interface/src/Application.h b/interface/src/Application.h index dc30c3c22c..b1077a523a 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -594,6 +594,8 @@ private: bool _aboutToQuit { false }; + FileLogger* _logger; + bool _previousSessionCrashed; DisplayPluginPointer _displayPlugin; @@ -674,8 +676,6 @@ private: QPointer _entityScriptServerLogDialog; QDir _defaultScriptsLocation; - FileLogger* _logger; - TouchEvent _lastTouchEvent; quint64 _lastNackTime; From 6a8894116613742755330fa731e5832dfa38a516 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 10 Jan 2019 13:59:28 -0800 Subject: [PATCH 2/6] put a plugin-interface version into the meta data of each plugin. don't load plugins that don't match the current version. --- .../plugins/src/plugins/PluginManager.cpp | 24 ++++++++++++++++++- libraries/plugins/src/plugins/PluginManager.h | 2 ++ plugins/hifiCodec/src/plugin.json | 5 +++- plugins/hifiKinect/src/plugin.json | 5 +++- plugins/hifiLeapMotion/src/plugin.json | 5 +++- plugins/hifiNeuron/src/plugin.json | 5 +++- plugins/hifiSdl2/src/plugin.json | 5 +++- plugins/hifiSixense/src/plugin.json | 5 +++- plugins/hifiSpacemouse/src/plugin.json | 5 +++- plugins/oculus/src/oculus.json | 5 +++- plugins/oculusLegacy/src/oculus.json | 5 +++- plugins/openvr/src/plugin.json | 5 +++- plugins/pcmCodec/src/plugin.json | 5 +++- plugins/steamClient/src/plugin.json | 5 +++- 14 files changed, 73 insertions(+), 13 deletions(-) diff --git a/libraries/plugins/src/plugins/PluginManager.cpp b/libraries/plugins/src/plugins/PluginManager.cpp index 32d8486e7a..ece8ccd528 100644 --- a/libraries/plugins/src/plugins/PluginManager.cpp +++ b/libraries/plugins/src/plugins/PluginManager.cpp @@ -71,6 +71,24 @@ QString getPluginIIDFromMetaData(QJsonObject object) { return object[IID_KEY].toString(); } +int getPluginInterfaceVersionFromMetaData(QJsonObject object) { + static const char* METADATA_KEY = "MetaData"; + static const char* NAME_KEY = "version"; + + if (!object.contains(METADATA_KEY) || !object[METADATA_KEY].isObject()) { + return 0; + } + + auto metaDataObject = object[METADATA_KEY].toObject(); + + if (!metaDataObject.contains(NAME_KEY) || !metaDataObject[NAME_KEY].isDouble()) { + return 0; + } + + return (int)(metaDataObject[NAME_KEY].toDouble()); +} + + QStringList preferredDisplayPlugins; QStringList disabledDisplays; QStringList disabledInputs; @@ -122,7 +140,11 @@ const LoaderList& getLoadedPlugins() { continue; } - if (loader->load()) { + if (getPluginInterfaceVersionFromMetaData(loader->metaData()) != HIFI_PLUGIN_INTERFACE_VERSION) { + qCDebug(plugins) << "Plugin" << qPrintable(plugin) << "interface version doesn't match, not loading:" + << getPluginInterfaceVersionFromMetaData(loader->metaData()) + << "doesn't match" << HIFI_PLUGIN_INTERFACE_VERSION; + } else if (loader->load()) { qCDebug(plugins) << "Plugin" << qPrintable(plugin) << "loaded successfully"; loadedPlugins.push_back(loader); } else { diff --git a/libraries/plugins/src/plugins/PluginManager.h b/libraries/plugins/src/plugins/PluginManager.h index c7489fd7e4..a3305d8ee7 100644 --- a/libraries/plugins/src/plugins/PluginManager.h +++ b/libraries/plugins/src/plugins/PluginManager.h @@ -61,3 +61,5 @@ private: DisplayPluginList _displayPlugins; InputPluginList _inputPlugins; }; + +static const int HIFI_PLUGIN_INTERFACE_VERSION = 1; diff --git a/plugins/hifiCodec/src/plugin.json b/plugins/hifiCodec/src/plugin.json index df26a67ea8..27391a484d 100644 --- a/plugins/hifiCodec/src/plugin.json +++ b/plugins/hifiCodec/src/plugin.json @@ -1 +1,4 @@ -{"name":"HiFi 4:1 Audio Codec"} +{ + "name":"HiFi 4:1 Audio Codec", + "version":1 +} diff --git a/plugins/hifiKinect/src/plugin.json b/plugins/hifiKinect/src/plugin.json index daa3a668dd..b401bb8c83 100644 --- a/plugins/hifiKinect/src/plugin.json +++ b/plugins/hifiKinect/src/plugin.json @@ -1 +1,4 @@ -{"name":"Kinect"} +{ + "name":"Kinect", + "version":1 +} diff --git a/plugins/hifiLeapMotion/src/plugin.json b/plugins/hifiLeapMotion/src/plugin.json index 2e867d96e4..92e410ef11 100644 --- a/plugins/hifiLeapMotion/src/plugin.json +++ b/plugins/hifiLeapMotion/src/plugin.json @@ -1 +1,4 @@ -{"name":"Leap Motion"} +{ + "name":"Leap Motion", + "version":1 +} diff --git a/plugins/hifiNeuron/src/plugin.json b/plugins/hifiNeuron/src/plugin.json index d153b5cebd..7059248934 100644 --- a/plugins/hifiNeuron/src/plugin.json +++ b/plugins/hifiNeuron/src/plugin.json @@ -1 +1,4 @@ -{"name":"Neuron"} +{ + "name":"Neuron", + "version":1 +} diff --git a/plugins/hifiSdl2/src/plugin.json b/plugins/hifiSdl2/src/plugin.json index a65846ecab..e1963f9995 100644 --- a/plugins/hifiSdl2/src/plugin.json +++ b/plugins/hifiSdl2/src/plugin.json @@ -1 +1,4 @@ -{"name":"SDL2"} +{ + "name":"SDL2", + "version":1 +} diff --git a/plugins/hifiSixense/src/plugin.json b/plugins/hifiSixense/src/plugin.json index 9e6e15a354..a56a1ba384 100644 --- a/plugins/hifiSixense/src/plugin.json +++ b/plugins/hifiSixense/src/plugin.json @@ -1 +1,4 @@ -{"name":"Sixense"} +{ + "name":"Sixense", + "version":1 +} diff --git a/plugins/hifiSpacemouse/src/plugin.json b/plugins/hifiSpacemouse/src/plugin.json index 294f436039..6eac13ac66 100644 --- a/plugins/hifiSpacemouse/src/plugin.json +++ b/plugins/hifiSpacemouse/src/plugin.json @@ -1 +1,4 @@ -{"name":"Spacemouse"} +{ + "name":"Spacemouse", + "version":1 +} diff --git a/plugins/oculus/src/oculus.json b/plugins/oculus/src/oculus.json index 86546c8dd5..0043a8b50f 100644 --- a/plugins/oculus/src/oculus.json +++ b/plugins/oculus/src/oculus.json @@ -1 +1,4 @@ -{"name":"Oculus Rift"} +{ + "name":"Oculus Rift", + "version":1 +} diff --git a/plugins/oculusLegacy/src/oculus.json b/plugins/oculusLegacy/src/oculus.json index 86546c8dd5..0043a8b50f 100644 --- a/plugins/oculusLegacy/src/oculus.json +++ b/plugins/oculusLegacy/src/oculus.json @@ -1 +1,4 @@ -{"name":"Oculus Rift"} +{ + "name":"Oculus Rift", + "version":1 +} diff --git a/plugins/openvr/src/plugin.json b/plugins/openvr/src/plugin.json index d68c8e68d3..763414cd8b 100644 --- a/plugins/openvr/src/plugin.json +++ b/plugins/openvr/src/plugin.json @@ -1 +1,4 @@ -{"name":"OpenVR (Vive)"} +{ + "name":"OpenVR (Vive)", + "version":1 +} diff --git a/plugins/pcmCodec/src/plugin.json b/plugins/pcmCodec/src/plugin.json index 2d86251845..525124592b 100644 --- a/plugins/pcmCodec/src/plugin.json +++ b/plugins/pcmCodec/src/plugin.json @@ -1 +1,4 @@ -{"name":"PCM Codec"} +{ + "name":"PCM Codec", + "version":1 +} diff --git a/plugins/steamClient/src/plugin.json b/plugins/steamClient/src/plugin.json index dfe37917d2..ce4647188f 100644 --- a/plugins/steamClient/src/plugin.json +++ b/plugins/steamClient/src/plugin.json @@ -1 +1,4 @@ -{"name":"Steam Client"} +{ + "name":"Steam Client", + "version":1 +} From 0603431ac84d9f604d2fe78f5a932266db5e303e Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 10 Jan 2019 15:16:33 -0800 Subject: [PATCH 3/6] initialize _logger with null pointer --- interface/src/Application.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Application.h b/interface/src/Application.h index b1077a523a..4c6d45b8c3 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -594,7 +594,7 @@ private: bool _aboutToQuit { false }; - FileLogger* _logger; + FileLogger* _logger { nullptr }; bool _previousSessionCrashed; From fd8702d5a6b4bfb120c2821b53c283d8bc2b397c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 10 Jan 2019 15:20:53 -0800 Subject: [PATCH 4/6] use const references when querying plugin metadata --- libraries/plugins/src/plugins/PluginManager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/plugins/src/plugins/PluginManager.cpp b/libraries/plugins/src/plugins/PluginManager.cpp index ece8ccd528..854ae62ea1 100644 --- a/libraries/plugins/src/plugins/PluginManager.cpp +++ b/libraries/plugins/src/plugins/PluginManager.cpp @@ -44,7 +44,7 @@ PluginManagerPointer PluginManager::getInstance() { return DependencyManager::get(); } -QString getPluginNameFromMetaData(QJsonObject object) { +QString getPluginNameFromMetaData(const QJsonObject& object) { static const char* METADATA_KEY = "MetaData"; static const char* NAME_KEY = "name"; @@ -61,7 +61,7 @@ QString getPluginNameFromMetaData(QJsonObject object) { return metaDataObject[NAME_KEY].toString(); } -QString getPluginIIDFromMetaData(QJsonObject object) { +QString getPluginIIDFromMetaData(const QJsonObject& object) { static const char* IID_KEY = "IID"; if (!object.contains(IID_KEY) || !object[IID_KEY].isString()) { @@ -71,7 +71,7 @@ QString getPluginIIDFromMetaData(QJsonObject object) { return object[IID_KEY].toString(); } -int getPluginInterfaceVersionFromMetaData(QJsonObject object) { +int getPluginInterfaceVersionFromMetaData(const QJsonObject& object) { static const char* METADATA_KEY = "MetaData"; static const char* NAME_KEY = "version"; From e941dbb4a9971ae830aa226f944d0a425cb5056a Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 10 Jan 2019 15:33:34 -0800 Subject: [PATCH 5/6] review --- .../plugins/src/plugins/PluginManager.cpp | 40 ++++++------------- libraries/plugins/src/plugins/PluginManager.h | 7 ++++ 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/libraries/plugins/src/plugins/PluginManager.cpp b/libraries/plugins/src/plugins/PluginManager.cpp index 854ae62ea1..9f481d04f1 100644 --- a/libraries/plugins/src/plugins/PluginManager.cpp +++ b/libraries/plugins/src/plugins/PluginManager.cpp @@ -47,45 +47,27 @@ PluginManagerPointer PluginManager::getInstance() { QString getPluginNameFromMetaData(const QJsonObject& object) { static const char* METADATA_KEY = "MetaData"; static const char* NAME_KEY = "name"; - if (!object.contains(METADATA_KEY) || !object[METADATA_KEY].isObject()) { return QString(); } - - auto metaDataObject = object[METADATA_KEY].toObject(); - - if (!metaDataObject.contains(NAME_KEY) || !metaDataObject[NAME_KEY].isString()) { - return QString(); - } - - return metaDataObject[NAME_KEY].toString(); + return object[METADATA_KEY][NAME_KEY].toString(""); } QString getPluginIIDFromMetaData(const QJsonObject& object) { static const char* IID_KEY = "IID"; - if (!object.contains(IID_KEY) || !object[IID_KEY].isString()) { return QString(); } - return object[IID_KEY].toString(); } int getPluginInterfaceVersionFromMetaData(const QJsonObject& object) { - static const char* METADATA_KEY = "MetaData"; - static const char* NAME_KEY = "version"; - + static const QString METADATA_KEY = "MetaData"; + static const QString NAME_KEY = "version"; if (!object.contains(METADATA_KEY) || !object[METADATA_KEY].isObject()) { return 0; } - - auto metaDataObject = object[METADATA_KEY].toObject(); - - if (!metaDataObject.contains(NAME_KEY) || !metaDataObject[NAME_KEY].isDouble()) { - return 0; - } - - return (int)(metaDataObject[NAME_KEY].toDouble()); + return object[METADATA_KEY][NAME_KEY].toInt(0); } @@ -135,16 +117,18 @@ const LoaderList& getLoadedPlugins() { QSharedPointer loader(new QPluginLoader(pluginPath + plugin)); if (isDisabled(loader->metaData())) { - qWarning() << "Plugin" << qPrintable(plugin) << "is disabled"; + qCWarning(plugins) << "Plugin" << qPrintable(plugin) << "is disabled"; // Skip this one, it's disabled continue; } - if (getPluginInterfaceVersionFromMetaData(loader->metaData()) != HIFI_PLUGIN_INTERFACE_VERSION) { - qCDebug(plugins) << "Plugin" << qPrintable(plugin) << "interface version doesn't match, not loading:" - << getPluginInterfaceVersionFromMetaData(loader->metaData()) - << "doesn't match" << HIFI_PLUGIN_INTERFACE_VERSION; - } else if (loader->load()) { + qCWarning(plugins) << "Plugin" << qPrintable(plugin) << "interface version doesn't match, not loading:" + << getPluginInterfaceVersionFromMetaData(loader->metaData()) + << "doesn't match" << HIFI_PLUGIN_INTERFACE_VERSION; + continue; + } + + if (loader->load()) { qCDebug(plugins) << "Plugin" << qPrintable(plugin) << "loaded successfully"; loadedPlugins.push_back(loader); } else { diff --git a/libraries/plugins/src/plugins/PluginManager.h b/libraries/plugins/src/plugins/PluginManager.h index a3305d8ee7..2a002577a4 100644 --- a/libraries/plugins/src/plugins/PluginManager.h +++ b/libraries/plugins/src/plugins/PluginManager.h @@ -62,4 +62,11 @@ private: InputPluginList _inputPlugins; }; +// TODO: we should define this value in CMake, and then use CMake +// templating to generate the individual plugin.json files, so that we +// don't have to update every plugin.json file whenever we update this +// value. The value should match "version" in +// plugins/*/src/plugin.json +// plugins/oculus/src/oculus.json +// etc static const int HIFI_PLUGIN_INTERFACE_VERSION = 1; From d31cdd3779c6f8071667f616ccd02cfca0841a5e Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 10 Jan 2019 15:35:40 -0800 Subject: [PATCH 6/6] review --- libraries/plugins/src/plugins/PluginManager.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/libraries/plugins/src/plugins/PluginManager.cpp b/libraries/plugins/src/plugins/PluginManager.cpp index 9f481d04f1..a75ede3f03 100644 --- a/libraries/plugins/src/plugins/PluginManager.cpp +++ b/libraries/plugins/src/plugins/PluginManager.cpp @@ -47,26 +47,17 @@ PluginManagerPointer PluginManager::getInstance() { QString getPluginNameFromMetaData(const QJsonObject& object) { static const char* METADATA_KEY = "MetaData"; static const char* NAME_KEY = "name"; - if (!object.contains(METADATA_KEY) || !object[METADATA_KEY].isObject()) { - return QString(); - } return object[METADATA_KEY][NAME_KEY].toString(""); } QString getPluginIIDFromMetaData(const QJsonObject& object) { static const char* IID_KEY = "IID"; - if (!object.contains(IID_KEY) || !object[IID_KEY].isString()) { - return QString(); - } - return object[IID_KEY].toString(); + return object[IID_KEY].toString(""); } int getPluginInterfaceVersionFromMetaData(const QJsonObject& object) { static const QString METADATA_KEY = "MetaData"; static const QString NAME_KEY = "version"; - if (!object.contains(METADATA_KEY) || !object[METADATA_KEY].isObject()) { - return 0; - } return object[METADATA_KEY][NAME_KEY].toInt(0); }