Created attachment 38133 [details] [review]
patchtastic

Looks nice - but why can't we keep the old API as a trivial wrapper of the
existing (better) glib API ? - surely that'd solve the problem in more
compatible fashion ? - then again, cleaning that evilness up can only be good :-)

The problem is that the old API takes a catagory string, so theoretically you
could pass it "LC_COLLATE" (or anything else).  The glib function only supports
"LC_MESSAGES".

As it turns out, people only ever use it for "LC_MESSAGES" (or "LANG" when they
actually mean "LC_MESSAGES").

The two other things that use this function (illegally, by prototyping it
themselves)

gnome-vfs uses it for LC_MESSAGES. (we can fix gnome-vfs to use glib directly)
libgnome uses it and reexports its functionality (in a public header) ((ow))

So now the only problem is libgnome.

gnome_i18n_get_language_list (in libgnome) is used by at least:
    libbonoboui, libgal, libgnome itself, libgnome-desktop
    bug-buddy, evolution, gdm, gnome-about, gnome-help, gnumeric
    yelp, gweather applet

gnome_i18n_get_language_list is deprecated.

I was talking with Ben and apparently Owen thinks that there are no
significant/useful/etc uses of gnome_i18n_get_language_list for non-LC_MESSAGES
categories.

Ben's idea of how to fix this is to replace gnome_i18n_get_language_list with a
small function that ignores its 'category' parameter and always returns the
LC_MESSAGES list (since this is the only functionality supported by
g_get_language_names() and since this is the only thing anyone uses it for anyway).

This breaks API/ABI, but supposedly in a way that nobody will care about or even
notice.  Assuming this breakage is kosher, I also believe this is the correct
approach.

Sane?

After talking this over with a couple of people I think the correct solution is
to leave bonobo_activation_i18n_get_language_list() in place.

However, any code that we have control over (including bonobo-activation) should
stop using it, with the exception of libgnome.

Additionally, all code that we have control over should stop using
gnome_i18n_get_language_list() and use g_get_language_names().  This is a
no-brainer since gnome_i18n_get_language_list is deprecated anyway.

The theory here is that if bonobo_activation_i18n_get_language_list never gets
called, then it will never use the 100k.  If an old application calls it then it
will use the 100k but at least we won't be breaking ABI compatibility. 
Meanwhile, everything in the desktop will benefit from the memory reduction.

The part of my patch that causes bonobo-activation to use the glib function
should still be applied after the freeze ends.

Additionally, the functionality itself could be moved to libgnome to live out
its final days, but this isn't really a pressing issue.

Why can't we provide the i18n_get_language_list functionality as a wrapper of
g_get_language_names - or is their function radically disimilar ? either way -
we have to maintain bincompat - so the symbol can't go, or get broken or
whatever forever :-)

gnome_i18n_get_language_list( const char * )
g_get_language_names( void )

the gnome function allows you to query any category (messages, paper, collate,
numeric, monetary, ...).  g_get_language_names is only for messages.

Why does bonobo-activation need a language list?

Owen says that the research they did when adding the g_ function basically came 
to the conclusion that the non-messages versions were useless. I think we 
should just replace the text of bonobo_activation_i18n_get_language_list with a 
call to the g_ function and make it absolutely clear that this function (as 
well as the gnome_blahblah function) are obsolete.

Should we make this a duplicate of bug 169414 now that we have a patch from Alex
in there?

No.  This is a bad idea.  This change can't just be made as such since it will
break other modules (see the big ALEX comment that alex's patch patch removes.)

What really needs to be done is to have this function removed from bonobo and
moved into libgnome.

I've already written and commited patches to gnome-vfs and gweather to remove
their dependancy on this function, but libgnome exports this as a public
interface (and therefore this code absolutely can not die without breaking ABI
compatibility  -- but definitely should be moved).

Here's the interface that we'd be breaking:

http://developer.gnome.org/doc/API/2.0/libgnome/libgnome-gnome-i18n.html

The code that implements this function is just a wrapper that makes a call to
'bonobo_activation_i18n_get_language_list'.  If we kill this function (which we
should) then the functionality should move into libgnome.

The reason that we can't just use g_get_language_list() is that it is hard-wired
to only support LC_MESSAGES, whereas the more generic
gnome_i18n_get_language_list() takes a parameter that lets you choose which
category you want the list for (messages, character set, time and date format,
paper size, sort orders, etc...)

Nothing uses the category field, and the few places that do do it wrongly
(passing LANG, which is not a category and will break the function!). My change
is perfectly safe for all sane uses of the API (since the extra parameter, if
passed, will be ignored due to C calling semantics).

I mean, why would you want to get the language list for the current default
paper size?

We could change the API docs for gnome_i18n_get_language_list to note that the
argument is ignored. And recommend direct use of g_get_language_list().

API/ABI compatibility is promised for the GNOME 2.x series.  My argument is
simply that we shouldn't break this promise.

I understand that it _probably_ doesn't matter (ie: we know of no apps that use
this).

That being said, gnome_i18n_get_language_list() is a documented function in a
LGPL library.  Anything (open, closed, whatever) could be using it.

I think we ought to move the code to libgnome and mark
gnome_i18n_get_language_list() deprecated (recommending g_get_language_list()).
 This means that the user only pays for it if they use it.

In any case, the symbol named 
'bonobo_activation_i18n_get_language_list' ought to die.  It's an awful hack
that people are manually prototyping/importing this function.  Everything that
calls this ought to just use g_get_language_list() directly.  This is something
that is fixable (and is partially fixed already -- see bug 169329).

It'd be really nice to kill the code outright, but I don't really think that we can.

The bonobo_activation_i18n_get_language_list symbol absolutely can't go away,
since that would break ABI for real. There are libgnome versions out there that
reference that symbol, and removing it will cause apps to stop running.

I'm all for replacing all use of gnome_i18n_get_language_list with
g_get_language_list() in all gnome code, and its clear we should deprecate
gnome_i18n_get_language_list. However, code can still be using
gnome_i18n_get_language_list.

So, the possible decisions are:
1) keep gnome_i18n_get_language_list() and b_a_g_l_l() as is
pro: 
  Keeps the old API
con:
  Old apps that use this will still use 100K of memory extra
  Need to continue maintaining the code (for instance, fix bug 169414).

2) base gnome_i18n_get_language_list on g_get_language_list()
pro:
  Less code
  Less maintainance
  Don't use 100k extra
  Can untangle the bonobo-activation dependency for this 
  (We can put the glib based version in both b-a and libgnome since
   its so small.)
con:
  Technically a change of behaviour compared to the docs.

Option 2 has a lot of nice pros, and the con is very limited. I can't think of
any reason at all to get a language list for anything other than used for
translations. And its not like things break, we still always return a valid
language list.

Your ideas, as you present them are pretty clear.  Breaking API/ABI
compatibility is a fairly small price to pay for getting rid of all of this
badness.  Additionally, as long as the 'char *' parameter to
gnome_i18n_get_language_list() is maintained then old code will still compile
properly (although will behave oddly -- should document this).

Just one thing: isn't it fairly safe to assume that nobody will install libgnome
2.10 against bonobo 2.12?  This seems like a fairly broken behaviour (ie:
running half gnome 2.10 and half gnome 2.12) and it'd be nice to kill off this
symbol.

At this point I more or less agree with you.  Idealism is the wrong approach
when dealing with something so evil. :)

I commited this.

Did someone check the pmap output after the fix?  I'd like to advertise this on
behalf of the memory reduction effort, but need to know if it actually works :)

i didn't verify the memory use drop, no.

For a simple test app which opens an EBook, waits, and closes it again, these
are the pmap totals:

Before: 25504 Kb
 After: 25368 Kb

*** Bug 153443 has been marked as a duplicate of this bug. ***