Re: [Evolution-hackers] Install E-D-S backends into separate directories?

2011-01-06 Thread Matthew Barnes
On Tue, 2011-01-04 at 15:00 -0500, Matthew Barnes wrote:
> The cleanest and most obvious solution is to install the backends into
> separate address book and calendar directories, then have each factory
> process load from the appropriate directory.
> 
> This will require a few API changes to the backend libraries (if you
> count pkg-config files as part of the API):

This is committed now for 2.91.5.

I changed the pkg-config variable name to "backenddir" since "backend"
is the term used throughout evolution-data-server for dynamically loaded
library modules.

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Install E-D-S backends into separate directories?

2011-01-06 Thread Milan Crha
On Wed, 2011-01-05 at 12:13 -0500, Matthew Barnes wrote:
> Yes, but I'll write them.  They're nothing more than a bunch of
> GObject properties with simple get/set functions.  The marshalling of
> those values to and from key files is all handled transparently by
> ESource.
> 
> Writing those classes is monkey work, as you like to call it.  :)

Nobody likes monkey-work, neither code duplication, and as we are
talking about each backend for addressbook and calendar, then one can
imagine what that might mean. For example, will you write these for
evolution-couchdb, evolution-kolab and possibly many other 3rd party
providers, apart to all of included in eds itself? You may understand
why I dislike the idea.

I thought, from one of your first mails about this, that this code
duplication will be unnecessary, that the basic ESource will have a way,
different from the GObject properties, to get/set any property from the
ESource hierarchy. And what I thought was that it'll not be done through
GObject, but through its own API.

What I'm trying to tell is that the approach of GObject properties is
more limiting and adds more monkey work than an approach with special
get/set API on ESource itself.
Bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Install E-D-S backends into separate directories?

2011-01-05 Thread Matthew Barnes
On Wed, 2011-01-05 at 16:21 +0100, Milan Crha wrote:
> Ah, I see, quite complicated. The g_type_module_register_type() says:
>As long as any instances of the type exist, the type plugin
>will not be unloaded.
> Whatever that means. Though if it means what I would expect from it,
> then registering unconditionally all types in eds_module_initialize()
> and "dereferencing" those special types in eds_module_shutdown() may
> cause unload of the module.

It took me awhile to understand how GTypeModule works.  I know you said
you're convinced already, but let me see if I can summarize GTypeModule
and explain the problem in detail just for posterity.

  - GTypes for dynamically loaded classes are registered permanently
regardless of whether the type module stays loaded or is unloaded.
That's the job of GTypeModule.

  - When you reference a dynamically loaded class, whether through
g_object_new(), g_type_create_instance(), or g_type_class_ref(),
the type module is loaded.  In our case, loading a type module
invokes eds_module_initialize().  So it can be called multiple
times, which is usually harmless if all you're doing there is
registering types.

  - When you drop the last reference to a dynamically loaded class,
whether through g_object_unref(), g_type_free_instance(), or
g_type_class_unref(), the type module is unloaded.

  - When a type module is unloaded, all static variables get reset.
That's why you have to use g_type_module_register_type() instead
of the usual pattern of:

static GType type = 0;

if (type == 0)
type = g_type_register_static (...);

return type;


The problem in, say, e-addressbook-factory is as follows:

  - Both address book and calendar type modules are loaded because they
all reside in the same directory.  Address book type modules remain
loaded because we create an instance of each factory backend.
Calendar type modules, however, are immediately unloaded.

  - Calendar type modules link to libedata-cal, but libedata-book does
not link libedata-cal (nor do I think we want to go down that road).
Also, ECalBackendFactory is registered statically in libedata-cal.
Therefore when calendar type modules are unloaded, the static
GTypeInfo data for ECalBackendFactory becomes corrupted.

  - In 2.91 this corruption doesn't cause problems because calendar type
modules are never reloaded in e-addressbook-factory.

  - Now along comes my newfangled ESource API, where extension objects
are created on-demand by asking for them by name.  For example:

  ldap_extension = e_source_get_extension (source, "ldap-backend");

  - ESourceExtensionClass has a "name" field which each subclass must
set to something unique.  ESourceLDAPClass, for example, sets it to
"ldap-backend".

  - In order for ESource to match the requested extension name to the
appropriate class, it has to peek at the "name" field of each class,
like so:

  class = g_type_class_ref (type);

  extension_name = class->name;
  ... do some other stuff ...

  g_type_class_unref (class);

  - That call to g_type_class_ref() causes those calendar type modules
to be reloaded, and the aforementioned ECalBackendFactory corruption
now manifests itself -- sometimes in the form of run time warnings
from GType, other times e-addressbook-factory crashes.

Splitting the address book and calendar type modules into separate
install directories is the simplest way I could think of to solve the
corruption problem.


> Does the proposed change (in ESourceGroup/ESource) also mean that each
> backend should provide its own ESourceExtension when it'll need to set
> some custom attributes on an ESourceGroup/ESource? I hope not.

Yes, but I'll write them.  They're nothing more than a bunch of GObject
properties with simple get/set functions.  The marshalling of those
values to and from key files is all handled transparently by ESource.

Writing those classes is monkey work, as you like to call it.  :)


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Install E-D-S backends into separate directories?

2011-01-05 Thread Milan Crha
On Wed, 2011-01-05 at 08:25 -0500, Matthew Barnes wrote:
> Both the backend factory class and the ESource extension types have to
> be registered with g_type_module_register_type(), and the GTypeModule
> isn't available from the class init function.
>
> Here's the workaround I'm currently having to add to almost every
> backend.  The lengthy comment explains a bit more technical detail. 

Ah, I see, quite complicated. The g_type_module_register_type() says:
   As long as any instances of the type exist, the type plugin
   will not be unloaded.
Whatever that means. Though if it means what I would expect from it,
then registering unconditionally all types in eds_module_initialize()
and "dereferencing" those special types in eds_module_shutdown() may
cause unload of the module.

Nonetheless this is just a theory, and I believe the usage of this
showed you that it doesn't behave this way, thus you've a green from my
side, even I would prefer less intrusive change for providers of
calendar/book backends.

Does the proposed change (in ESourceGroup/ESource) also mean that each
backend should provide its own ESourceExtension when it'll need to set
some custom attributes on an ESourceGroup/ESource? I hope not.
Bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Install E-D-S backends into separate directories?

2011-01-05 Thread Matthew Barnes
On Wed, 2011-01-05 at 13:17 +0100, Milan Crha wrote:
> are you sure they are kept in memory? I see the factory calls

Yes, GTypes are registered permanently whether the class is loaded
statically or dynamically.  This isn't about class instances.


> Do you mean you are returning from the backend module, apart of EBackend
> types, also different types in the eds_module_list_types() call? I
> always thought this function is mean to return only list of descendants
> of EBackend.

No, eds_module_initialize() and eds_module_list_types() are separate
functions.  eds_module_list_types() still only returns backend types.


> There is no need to have a separate module for calendar and book at the
> moment, it is only done this way. Separate folders may mean forcing this
> two-module model. On the other hand, if you share some parts between
> both, where I hope you do with Exchange, where both calendar and book
> backends are using same ESourceBackend type (or how you call it), then
> having one module may be a benefit.

The address book and calendar parts of Evolution-MAPI can share some
kind of ESourceMAPI class in libexchangemapi that holds configuration
details for the MAPI account.  I haven't got that far yet, but that's
basically what I'm doing for other backends.


> By the way, why cannot be ESourceBackend type registered only on the
> backend class creation? It's a similar way as Camel's provider does it,
> isn't it?

Both the backend factory class and the ESource extension types have to
be registered with g_type_module_register_type(), and the GTypeModule
isn't available from the class init function.


Here's the workaround I'm currently having to add to almost every
backend.  The lengthy comment explains a bit more technical detail.


void
eds_module_initialize (GTypeModule *type_module)
{
/* XXX Because address book and calendar extensions are installed
 * to a common "extensions" directory, both factory processes
 * load all the extension modules.
 *
 * That's fine for the factory classes.  Each address book
 * factory class registers its GType in e-calendar-factory,
 * but then the extension is unloaded and the address book
 * factory class is never referenced again.  No harm done.
 *
 * Similarly, each calendar factory class registers its
 * GType in e-addressbook-factory, but then the extension
 * is unloaded and the calendar factory class is never
 * referenced again.
 *
 * ESource, on the other hand, finds extension classes by
 * querying the GType system for registered descendants of
 * E_TYPE_SOURCE_EXTENSION.  Since GType registration is
 * permanent whether the GTypeClass is static or dynamic,
 * we only want to register the ESource extension in the
 * appropriate process.  So we check g_get_prgname()
 * before registering the ESource extension type.
 *
 * The single extension directory design dates back to the
 * Bonobo era when we just had a single evolution-data-server
 * process.  Now that we've split up address book and calendar
 * services into separate processes, the "extensions" directory
 * should be split up as well.  But that will require an API
 * break in libebackend so the directory to load extensions
 * from can be passed in instead of hard-coded.  It requires
 * further public discussion before we do this. */

if (g_strcmp0 (g_get_prgname (), "e-addressbook-factory") == 0)
e_source_ldap_type_register (type_module);

ldap_type = _ldap_factory_get_type (type_module);
}


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Install E-D-S backends into separate directories?

2011-01-05 Thread Milan Crha
On Tue, 2011-01-04 at 15:00 -0500, Matthew Barnes wrote:
> That means e-addressbook-factory is loading calendar backend modules
> and e-calendar-factory is loading address book backend modules.  Until
> now that hasn't been a problem: the foreign backend classes are
> registered but remain dormant. 

Hi,
are you sure they are kept in memory? I see the factory calls
   e_data_server_get_extensions_for_type()
then frees this list with
   e_data_server_extension_list_free()
and finally calls
   e_data_server_module_remove_unused()
which may do the things you are trying to achieve by extension folder
separation, right?

I do not think it worth the change, the overhead on a factory load may
not be so large. Though see below.

> On my account management branch I've started registering other GTypes
> in eds_module_initialize() functions.  For example, the LDAP backend
> now registers both a backend factory class and an ESourceLDAP class
> which holds all the LDAP account details and configuration options.

Do you mean you are returning from the backend module, apart of EBackend
types, also different types in the eds_module_list_types() call? I
always thought this function is mean to return only list of descendants
of EBackend.

There is no need to have a separate module for calendar and book at the
moment, it is only done this way. Separate folders may mean forcing this
two-module model. On the other hand, if you share some parts between
both, where I hope you do with Exchange, where both calendar and book
backends are using same ESourceBackend type (or how you call it), then
having one module may be a benefit.

> The cleanest and most obvious solution is to install the backends into
> separate address book and calendar directories, then have each factory
> process load from the appropriate directory.

I suppose the other approach is to have some kind of hierarchical type
tree in e-data-server-module.c, having separated descendants of EBackend
and the rest returned from eds_module_list_types() and unload those
other types only if every EBackend types from that module are unused.
Is it more dirty way than overloading eds_module_list_types()?

By the way, why cannot be ESourceBackend type registered only on the
backend class creation? It's a similar way as Camel's provider does it,
isn't it?
Bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers