Menu
▾
▴
Re: [Opensync-devel] [trunk] OSYNC_START_TYPE_EXTERNAL example / OSyncClient API changes
|
From: Daniel G. <go...@b1...> - 2009-11-01 13:52:43
|
On Sunday 01 November 2009 01:39:24 am Graham Cobb wrote: > I have a first cut of the external plugin config file ready to check in > (http://opensync.pastebin.com/m38d98a95). Thanks, will review your patch ASAP! > But I have a couple of > questions: > > 1) When I run the unittests, 17 of them fail. I don't think any of them > are related to these changes but is that expected at the moment? No, actually they should fail. Could you run: "make Experimental" so the testing resutls get uploaded to the CDash dashboard .. so we can have a look on the logs? http://opensync.org/testing/index.php?project=OpenSync > > The following tests FAILED: [...] JFYI, you can also run those tests with: ctest -V -V makes things more verbose ... If you only want to run specific tests you can use -R $regex to run only specifc tests - e.g.: ctest -V -R sync_ Will run all tests including "sync_" > > If it is not expected I will have to do some more checking. Would be great if you could check whats going wrong there ... Those tests also fail if you revert your changes - right? > > 2) I have created an example external plugin config file in > docs/examples/plugins/src. Can the existing CMake macros be used to get > that file installed into the plugin directory (which is where I am > looking for it -- I look for .xml files first, then .so files)? I haven't > tried it but I assume the macros like OPENSYNC_PLUGIN_INSTALL expect a .so > file. OSYNC_PLUGIN_INSTALL is quite simple ... it doesn't check for *.so -----8<---- ## Install plugin MACRO( OPENSYNC_PLUGIN_INSTALL _pluginName ) INSTALL( TARGETS ${_pluginName} DESTINATION ${OPENSYNC_PLUGINDIR} ) ENDMACRO( OPENSYNC_PLUGIN_INSTALL ) ---->8------ So you could point this to a .xml file ... but maybe we should introduce a new macro for such configuration file. Maybe one day we want to change the location of this kind config file .. so we just need to change the central macro, and don't have to fix all plugins. What do you think? > > I really don't know CMake -- can anyone advise how to get my .xml file > installed into the plugin directory? I'll introduce a cmake macro called "OPENSYNC_EXTERNAL_PLUGIN_INSTALL" ... > > The new code seems to work but there are a few things that still need to > be done: > > 3) The XML format includes <ExternalCommand> but I currently ignore > it. My plan is that this should be saved in the plugin structure and > could be used as a default for the config option that Henrik is adding. Ok, fine with me. > > 4) If the .xml and the .so are both present, they both get loaded and two > plugins exist with the same name. I thought of adding error checking to > osync_plugin_set_name to fail if a plugin of the same name is already > loaded. But I have just noticed that none of the osync_plugin_set_... > calls have OSyncError parameters. Should they? This would be an API > change (to a widely used API). Actually the file-name of a plugin doesn't have to be the name of the plugin in the Plugin Enviroment. e.g.: syncml.so spawns 4 plugins, non called "syncml". But i see your point with osync_plugin_set_name() .. and having multiple plugins with the same name. This problem is already present ... even without your modification. What we could do, without breaking the api to handle this: We could let osync_plugin_env_load() fail if there are multiple plugins with the same name in the env->plugins list. Or we could let osync_plugin_env_register_plugin() fail ... but this function get called by the plugins any maybe not get handled by the plugin. We could do checks in both function to be sure ... Anyway, this is a different issue - independent of your planned changes .. which should get fixed at some point. I'll create for that a ticket ... in my opionoen it's not a blocker, since this could be avoided by distribution with sane packaging ;) > > 5) There is no unittest for this code yet. > > I'm not sure if I will get time to work on this further on Sunday -- I > will check it in by the end of Sunday. > Ok, cool. Thanks for your contribution! Best Regards, Daniel -- Daniel Gollub Geschaeftsfuehrer: Ralph Dehner FOSS Developer Unternehmenssitz: Vohburg B1 Systems GmbH Amtsgericht: Ingolstadt Mobil: +49-(0)-160 47 73 970 Handelsregister: HRB 3537 EMail: go...@b1... http://www.b1-systems.de Adresse: B1 Systems GmbH, Osterfeldstraße 7, 85088 Vohburg http://pgpkeys.pca.dfn.de/pks/lookup?op=get&search=0xED14B95C2F8CA78D |
