linuxdcpp-team team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 3303
committer: poy <poy@xxxxxxxxxx>
branch nick: trunk
timestamp: Thu 2013-05-30 20:51:05 +0200
message:
  Improve plugin management
modified:
  changelog.txt
  dcpp/PluginApiImpl.cpp
  dcpp/PluginDefs.h
  dcpp/PluginManager.cpp
  dcpp/PluginManager.h


--
lp:dcplusplus
https://code.launchpad.net/~dcplusplus-team/dcplusplus/trunk

Your team Dcplusplus-team is subscribed to branch lp:dcplusplus.
To unsubscribe from this branch go to https://code.launchpad.net/~dcplusplus-team/dcplusplus/trunk/+edit-subscription

=== modified file 'changelog.txt'
--- changelog.txt	2013-05-21 17:51:38 +0000
+++ changelog.txt	2013-05-30 18:51:05 +0000
@@ -1,3 +1,5 @@
+* Improve plugin management
+
 -- 0.820 2013-05-21 --
 * Rotate the icon while DC++ is loading (poy)
 * [L#243727] Allow expanding merged search results (poy)

=== modified file 'dcpp/PluginApiImpl.cpp'
--- dcpp/PluginApiImpl.cpp	2013-05-15 17:17:56 +0000
+++ dcpp/PluginApiImpl.cpp	2013-05-30 18:51:05 +0000
@@ -232,7 +232,7 @@
 DCInterfacePtr PluginApiImpl::queryInterface(const char* guid, uint32_t version) {
 	// we only return the registered interface if it is same or newer than requested
 	DCInterface* dci = (DCInterface*)PluginManager::getInstance()->queryInterface(guid);
-	return (!dci || dci->apiVersion >= version) ? dci : NULL;
+	return (!dci || dci->apiVersion >= version) ? dci : nullptr;
 }
 
 Bool PluginApiImpl::releaseInterface(intfHandle hInterface) {

=== modified file 'dcpp/PluginDefs.h'
--- dcpp/PluginDefs.h	2013-05-15 17:17:56 +0000
+++ dcpp/PluginDefs.h	2013-05-30 18:51:05 +0000
@@ -32,7 +32,7 @@
 #endif
 
 /* Version of the plugin api (must change if old plugins simply can't be seen as viably working) */
-#define DCAPI_CORE_VER				7
+#define DCAPI_CORE_VER				8
 
 #ifdef _WIN32
 # define DCAPI __stdcall
@@ -117,10 +117,11 @@
 /* Main hook events (returned by pluginInit) */
 typedef enum tagPluginState {
 	ON_INSTALL = 0,												/* Replaces ON_LOAD for the very first loading of the plugin */
-	ON_UNINSTALL,												/* Replaces ON_UNLOAD when plugin is being uninstalled */
-	ON_LOAD,													/* Sent after successful call to pluginInit (obj: DCCore) */
-	ON_UNLOAD,													/* Sent right before plugin is unloaded (no params) */
-	ON_CONFIGURE												/* Sent when user wants to configure the plugin (obj: DCCore, data: impl. dependant) */
+	ON_LOAD,													/* Loading the plugin at program start; sent after the pluginInit call (obj: DCCore) */
+	ON_LOAD_RUNTIME,											/* Replaces ON_LOAD when loading the plugin at runtime */
+	ON_CONFIGURE,												/* The user wants to configure the plugin (obj: DCCore, data: impl. dependant) */
+	ON_UNLOAD,													/* The plugin is going to be unloaded (no params) */
+	ON_UNINSTALL												/* Replaces ON_UNLOAD when the plugin is being uninstalled */
 } PluginState;
 
 /* Argument types */

=== modified file 'dcpp/PluginManager.cpp'
--- dcpp/PluginManager.cpp	2013-05-15 17:17:56 +0000
+++ dcpp/PluginManager.cpp	2013-05-30 18:51:05 +0000
@@ -103,8 +103,8 @@
 
 		string version;
 		parse("ApiVersion", version);
-		if(Util::toInt(version) < DCAPI_CORE_VER) {
-			throw Exception(str(F_("%1% is too old, contact the plugin author for an update") % Util::getFileName(path)));
+		if(Util::toInt(version) != DCAPI_CORE_VER) {
+			throw Exception(str(F_("%1% is not compatible with %2%") % Util::getFileName(path) % APPNAME));
 		}
 
 		parse("UUID", info.uuid);
@@ -147,13 +147,12 @@
 	{
 		Lock l(cs);
 
-		auto it = findPluginIter(info.uuid);
-		if(it != plugins.end()) {
-			auto& plugin = *it;
-			if(plugin.version < info.version) {
+		auto dupe = findPlugin(info.uuid);
+		if(dupe) {
+			if(dupe->version < info.version) {
 				info.updating = true;
 			} else {
-				throw Exception(str(F_("%1% is already installed") % plugin.name));
+				throw Exception(str(F_("%1% is already installed") % dupe->name));
 			}
 		}
 	}
@@ -162,6 +161,22 @@
 }
 
 void PluginManager::install(const DcextInfo& info) {
+	{
+		Lock l(cs);
+
+		auto dupe = findPlugin(info.uuid);
+		if(dupe) {
+			if(dupe->version < info.version) {
+				// disable when updating or the file copy will fail...
+				if(dupe->handle) {
+					disable(*dupe, false);
+				}
+			} else {
+				throw Exception(str(F_("%1% is already installed") % dupe->name));
+			}
+		}
+	}
+
 	const auto source = Util::getTempPath() + "dcext" PATH_SEPARATOR_STR;
 	const auto target = getInstallPath(info.uuid);
 	const auto lib = target + Util::getFileName(info.plugin);
@@ -188,7 +203,7 @@
 	for(auto& plugin: plugins) {
 		if(!plugin.dcMain) { continue; } // a little trick to avoid an additonal "bool enabled"
 		if(f) { f(plugin.name); }
-		try { enable(plugin, false); }
+		try { enable(plugin, false, false); }
 		catch(const Exception& e) { LogManager::getInstance()->message(e.getError()); }
 	}
 }
@@ -227,9 +242,7 @@
 	plugin.name = Util::getFileName(path);
 	plugin.path = path;
 
-	enable(plugin, true);
-
-	plugins.push_back(move(plugin));
+	enable(plugin, true, true);
 }
 
 bool PluginManager::configPlugin(const string& guid, dcptr_t data) {
@@ -245,7 +258,7 @@
 	Lock l(cs);
 	auto p = findPlugin(guid);
 	if(p && !p->handle) {
-		enable(*p, false);
+		enable(*p, false, true);
 	}
 }
 
@@ -509,7 +522,7 @@
 	return Util::getPath(Util::PATH_USER_LOCAL) + "Plugins" PATH_SEPARATOR_STR + uuid + PATH_SEPARATOR_STR;
 }
 
-void PluginManager::enable(Plugin& plugin, bool install) {
+void PluginManager::enable(Plugin& plugin, bool install, bool runtime) {
 	Lock l(cs);
 
 	dcassert(dcCore.apiVersion != 0);
@@ -518,8 +531,8 @@
 	if(!plugin.handle) {
 		throw Exception(str(F_("Error loading %1%: %2%") % plugin.name % GET_ERROR()));
 	}
-	bool success = false;
-	ScopedFunctor(([&plugin, &success] { if(!success) { FREE_LIBRARY(plugin.handle); plugin.handle = nullptr; plugin.dcMain = nullptr; } }));
+	bool unload = true;
+	ScopedFunctor(([&plugin, &unload] { if(unload) { FREE_LIBRARY(plugin.handle); plugin.handle = nullptr; plugin.dcMain = nullptr; } }));
 
 	auto pluginInfo = reinterpret_cast<DCMAIN (DCAPI *)(MetaDataPtr)>(GET_ADDRESS(plugin.handle, "pluginInit"));
 	MetaData info { };
@@ -527,23 +540,76 @@
 		throw Exception(str(F_("%1% is not a valid plugin") % plugin.name));
 	}
 
-	if(checkPlugin(info, install)) {
-		install = true;
-	}
-
-	if(plugin.dcMain((install ? ON_INSTALL : ON_LOAD), &dcCore, nullptr) == False) {
-		throw Exception(str(F_("Error loading %1%") % plugin.name));
-	}
-
-	success = true;
-
-	// refresh the info we have on the plugin.
-	plugin.guid = info.guid;
-	plugin.name = info.name;
-	plugin.version = info.version;
-	plugin.author = info.author;
-	plugin.description = info.description;
-	plugin.website = info.web;
+	// Check API compatibility (this should only block on absolutely wrecking api changes, which generally should not happen)
+	if(info.apiVersion != DCAPI_CORE_VER) {
+		throw Exception(str(F_("%1% is not compatible with %2%") % info.name % APPNAME));
+	}
+
+	// Check that all dependencies are loaded
+	for(decltype(info.numDependencies) i = 0; i < info.numDependencies; ++i) {
+		if(!isLoaded(info.dependencies[i])) {
+			throw Exception(str(F_("Missing dependencies for %1%") % info.name));
+		}
+	}
+
+	auto pluginPtr = &plugin;
+
+	// When installing, check for duplicates and possible updates
+	if(install) {
+		auto dupe = findPlugin(info.guid);
+		if(dupe) {
+
+			if(dupe->version < info.version) {
+				// in-place updating to keep settings etc.
+
+				LogManager::getInstance()->message(str(F_("Updating the %1% plugin from version %2% to version %3%") %
+					string(dupe->name) % dupe->version % info.version));
+
+				if(dupe->handle) {
+					disable(*dupe, false);
+				}
+				dupe->handle = plugin.handle;
+				dupe->dcMain = plugin.dcMain;
+				pluginPtr = dupe;
+
+				install = false;
+
+			} else {
+				throw Exception(str(F_("%1% is already installed") % info.name));
+			}
+
+		} else {
+			// add to the list before executing dcMain, to guarantee a correct state (eg for settings).
+			plugins.push_back(move(plugin));
+			pluginPtr = &plugins.back();
+		}
+	}
+
+	{ // scope to change the plugin ref
+		auto& plugin = *pluginPtr;
+
+		if(plugin.dcMain(install ? ON_INSTALL : runtime ? ON_LOAD_RUNTIME : ON_LOAD, &dcCore, nullptr) == False) {
+
+			if(install) {
+				// remove from the list.
+				FREE_LIBRARY(plugin.handle);
+				plugins.pop_back();
+				unload = false;
+			}
+
+			throw Exception(str(F_("Error loading %1%") % plugin.name));
+		}
+
+		unload = false;
+
+		// refresh the info we have on the plugin.
+		plugin.guid = info.guid;
+		plugin.name = info.name;
+		plugin.version = info.version;
+		plugin.author = info.author;
+		plugin.description = info.description;
+		plugin.website = info.web;
+	}
 }
 
 void PluginManager::disable(Plugin& plugin, bool uninstall) {
@@ -642,42 +708,6 @@
 	}
 }
 
-bool PluginManager::checkPlugin(const MetaData& info, bool install) {
-	auto updating = false;
-
-	// When installing, check for duplicates and possible updates
-	if(install) {
-		auto it = findPluginIter(info.guid);
-		if(it != plugins.end()) {
-			auto& plugin = *it;
-			if(plugin.version < info.version) {
-				LogManager::getInstance()->message(str(F_("Updating the %1% plugin from version %2% to version %3%") %
-					string(plugin.name) % plugin.version % info.version));
-				plugins.erase(it);
-				updating = true;
-			} else {
-				throw Exception(str(F_("%1% is already installed") % info.name));
-			}
-		}
-	}
-
-	// Check API compatibility (this should only block on absolutely wrecking api changes, which generally should not happen)
-	if(info.apiVersion < DCAPI_CORE_VER) {
-		throw Exception(str(F_("%1% is too old, contact the plugin author for an update") % info.name));
-	}
-
-	// Check that all dependencies are loaded
-	if(info.numDependencies != 0) {
-		for(size_t i = 0; i < info.numDependencies; ++i) {
-			if(!isLoaded(info.dependencies[i])) {
-				throw Exception(str(F_("Missing dependencies for %1%") % info.name));
-			}
-		}
-	}
-
-	return updating;
-}
-
 const Plugin* PluginManager::findPlugin(const string& guid) const {
 	auto it = findPluginIter(guid);
 	return it != plugins.end() ? &*it : nullptr;

=== modified file 'dcpp/PluginManager.h'
--- dcpp/PluginManager.h	2013-05-15 17:17:56 +0000
+++ dcpp/PluginManager.h	2013-05-30 18:51:05 +0000
@@ -174,16 +174,12 @@
 	static string getInstallPath(const string& uuid);
 
 private:
-	void enable(Plugin& plugin, bool install);
+	void enable(Plugin& plugin, bool install, bool runtime);
 	void disable(Plugin& plugin, bool uninstall);
 
 	void loadSettings() noexcept;
 	void saveSettings() noexcept;
 
-	/** Check if the plugin can be loaded; throws if it can't.
-	@return Whether the plugin is being updated. */
-	bool checkPlugin(const MetaData& info, bool install);
-
 	const Plugin* findPlugin(const string& guid) const;
 	Plugin* findPlugin(const string& guid);
 	vector<Plugin>::const_iterator findPluginIter(const string& guid) const;