Merge lp:~7-eric/inkscape/dbus-instances into lp:~inkscape.dev/inkscape/trunk

[Eric Greveson]

Added a "--dbus-name" option to the command-line args, allowing users to specify a D-Bus bus name other than "org.inkscape" if required. This makes it possible to run and control multiple Inkscape instances (command line or GUI) in a single user session, very useful in a multi-processor server scripting environment.

[Martin Owens (doctormo)]

The code is good and looks clean. I'd leave out the comment at dbus-init.cpp:117, but apart from that very minor item. It's ready to be merged.

The functional test went well and it worked as expected.

review: Approve (full review)

=== modified file 'src/extension/dbus/dbus-init.cpp'
--- src/extension/dbus/dbus-init.cpp	2013-07-04 22:51:56 +0000
+++ src/extension/dbus/dbus-init.cpp	2013-07-10 11:00:40 +0000
@@ -36,7 +36,14 @@
 #include <sstream>
 
 
-
+namespace
+{
+    // This stores the bus name to use for this app instance. By default, it
+    // will be set to org.inkscape. However, users may provide other names by
+    // setting command-line parameters when starting Inkscape, so that more
+    // than one instance of Inkscape may be used by external scripts.
+    gchar *instance_bus_name = NULL;
+}
 
 namespace Inkscape {
 namespace Extension {
@@ -120,6 +127,11 @@
 void 
 init (void)
 {
+        if (instance_bus_name == NULL) {
+            // Set the bus name to the default
+            instance_bus_name = strdup("org.inkscape");
+        }
+
         guint   result;
         GError *error = NULL;
         DBusGConnection *connection;
@@ -127,7 +139,7 @@
         connection = dbus_get_connection();
         proxy = dbus_get_proxy(connection);
         org_freedesktop_DBus_request_name (proxy,
-                "org.inkscape",
+                instance_bus_name,
                 DBUS_NAME_FLAG_DO_NOT_QUEUE, &result, &error);
         //create interface for application
         dbus_register_object (connection, 
@@ -198,6 +210,19 @@
     return strdup(name.c_str());
 }
 
+void
+dbus_set_bus_name(gchar * bus_name)
+{
+    g_assert(bus_name != NULL);
+    g_assert(instance_bus_name == NULL);
+    instance_bus_name = strdup(bus_name);
+}
 
+gchar *
+dbus_get_bus_name()
+{
+    g_assert(instance_bus_name != NULL);
+    return instance_bus_name;
+}
 
 } } } /* namespace Inkscape::Extension::Dbus */

=== modified file 'src/extension/dbus/dbus-init.h'
--- src/extension/dbus/dbus-init.h	2013-07-03 19:06:11 +0000
+++ src/extension/dbus/dbus-init.h	2013-07-10 11:00:40 +0000
@@ -28,6 +28,16 @@
 
 gchar * dbus_init_desktop_interface (SPDesktop * dt);
 
+/** Set the bus name to use. Default is "org.inkscape".
+  This function should only be called once, before init(), if a non-default
+  bus name is required. */
+void dbus_set_bus_name(gchar * bus_name);
+
+/** Get the bus name for this instance. Default is "org.inkscape".
+  This function should only be called after init().
+  The returned gchar * is owned by this module and should not be freed. */
+gchar * dbus_get_bus_name();
+
 } } }  /* namespace Dbus, Extension, Inkscape */
 
 #endif /* INKSCAPE_EXTENSION_DBUS_INIT_H__ */

=== modified file 'src/extension/dbus/document-interface.h'
--- src/extension/dbus/document-interface.h	2013-07-04 22:51:56 +0000
+++ src/extension/dbus/document-interface.h	2013-07-10 11:00:40 +0000
@@ -34,9 +34,6 @@
 
 class SPDesktop;
 class SPItem;
-
-// TODO: this define doesn't seem to be used... although the path itself is also hardcoded in dbus-init.cpp
-#define DBUS_DOCUMENT_INTERFACE_PATH  "/org/inkscape/document"
         
 #define TYPE_DOCUMENT_INTERFACE            (document_interface_get_type ())
 #define DOCUMENT_INTERFACE(object)         (G_TYPE_CHECK_INSTANCE_CAST ((object), TYPE_DOCUMENT_INTERFACE, DocumentInterface))

=== modified file 'src/extension/dbus/wrapper/inkscape-dbus-wrapper.h'
--- src/extension/dbus/wrapper/inkscape-dbus-wrapper.h	2013-07-03 22:00:06 +0000
+++ src/extension/dbus/wrapper/inkscape-dbus-wrapper.h	2013-07-10 11:00:40 +0000
@@ -8,8 +8,6 @@
 
 //#include <dbus/dbus-glib-bindings.h>
 //#include <dbus/dbus-glib-lowlevel.h>
-
-#define DBUS_DOCUMENT_INTERFACE_PATH  "/org/inkscape/document"
         
 #define TYPE_DOCUMENT_INTERFACE            (document_interface_get_type ())
 #define DOCUMENT_INTERFACE(object)         (G_TYPE_CHECK_INSTANCE_CAST ((object), TYPE_DOCUMENT_INTERFACE, DocumentInterface))

=== modified file 'src/main.cpp'
--- src/main.cpp	2013-07-04 22:57:15 +0000
+++ src/main.cpp	2013-07-10 11:00:40 +0000
@@ -103,6 +103,11 @@
 #endif // WIN32
 
 #include "extension/init.h"
+// Not ideal, but there doesn't appear to be a nicer system in place for
+// passing command-line parameters to extensions before initialization...
+#ifdef WITH_DBUS
+#include "extension/dbus/dbus-init.h"
+#endif // WITH_DBUS
 
 #include <glibmm/i18n.h>
 #include <glibmm/main.h>
@@ -171,6 +176,7 @@
     SP_ARG_VACUUM_DEFS,
 #ifdef WITH_DBUS
     SP_ARG_DBUS_LISTEN,
+    SP_ARG_DBUS_NAME,
 #endif // WITH_DBUS
     SP_ARG_VERB_LIST,
     SP_ARG_VERB,
@@ -227,6 +233,7 @@
 static gboolean sp_vacuum_defs = FALSE;
 #ifdef WITH_DBUS
 static gboolean sp_dbus_listen = FALSE;
+static gchar *sp_dbus_name = NULL;
 #endif // WITH_DBUS
 static gchar *sp_export_png_utf8 = NULL;
 static gchar *sp_export_svg_utf8 = NULL;
@@ -274,6 +281,7 @@
         sp_vacuum_defs = FALSE;
 #ifdef WITH_DBUS
         sp_dbus_listen = FALSE;
+        sp_dbus_name = NULL;
 #endif // WITH_DBUS
 
         sp_export_png_utf8 = NULL;
@@ -487,6 +495,11 @@
      POPT_ARG_NONE, &sp_dbus_listen, SP_ARG_DBUS_LISTEN,
      N_("Enter a listening loop for D-Bus messages in console mode"),
      NULL},
+
+    {"dbus-name", 0,
+     POPT_ARG_STRING, &sp_dbus_name, SP_ARG_DBUS_NAME,
+     N_("Specify the D-Bus bus name to listen for messages on (default is org.inkscape)"),
+     N_("BUS-NAME")},
 #endif // WITH_DBUS
 
     {"verb-list", 0,
@@ -885,6 +898,13 @@
         if ( sp_global_printer )
             sp_global_printer_utf8 = g_strdup( sp_global_printer );
     }
+    
+#ifdef WITH_DBUS
+    // Before initializing extensions, we must set the DBus bus name if required
+    if (sp_dbus_name != NULL) {
+        Inkscape::Extension::Dbus::dbus_set_bus_name(sp_dbus_name);
+    }
+#endif
 
     // Return the list if wanted, else free it up.
     if ( flDest ) {