Re: [Evolution-hackers] Removing libical fork, moving to new upstream?
- From: Chenthill <pchenthill novell com>
- To: Patrick Ohly <Patrick Ohly gmx de>
- Cc: Wilfried Goesgens <dothebart uncensored citadel org>, Evolution Hackers <evolution-hackers gnome org>, IGnatius T Foobar <ajc citadel org>
- Subject: Re: [Evolution-hackers] Removing libical fork, moving to new upstream?
- Date: Wed, 10 Sep 2008 18:26:31 +0300
On Tue, 2008-09-09 at 18:00 +0200, Patrick Ohly wrote:
> On Tue, 2008-09-09 at 09:12 -0400, IGnatius T Foobar wrote:
> > Chenthill wrote:
> > > So it is better to inform all the stake holders about the change and let
> > > them depend on the library versions to decide whether to free the memory
> > > or not if they have a need to depend on the older versions of libical. I
> > > think no one deny to make the necessary changes knowing that the old API
> > > is not very stable.
> > >
> > > Atleast once I noticed the problem. I made this patch and made all the
> > > changes required in evolution, evolution-exchange and
> > > evolution-data-server. I would not really like to change them again with
> > > new APIS :)
> > Although I agree that the new memory model is a vast improvement over
> > the existing one, I think you may be underestimating the potential
> > effects of telling dozens of downstream projects that they will have to
> > rewrite their code *right* *now* in order to continue using libical.
> > Many will respond by forking, or staying forked, which as I mentioned
> > earlier is exactly the opposite of what we are trying to accomplish.
>
> I agree.
When I started writing the patch, since only the gnome packages which
depend on libecal are going be affected. I thought of incorporating
changes in all other modules myself since it was just to grep all the
apis and free the returned memory.
So I had a thinking that all the downstream projects would be ready to
change their code during a release time since stability is an issue and
change can be done fast. But if it was not the case and might result in
forking again, the only solution which I too see is create a new set of
API's which uses the new memory allocation.
>
> > How would you feel about a global flag which tells libical how to
> > allocate memory?
>
> The problem with that is that it is not possible to mix code which uses
> the old and the new semantic. For example, a program or library which
> uses libical directly, using the old semantic, couldn't be combined with
> the Evolution libraries.
Yes right. Its not possible to have the old semantic with changed memory
allocation.
>
> To let old and new code coexists I would suggest the following approach:
> 1. The Evolution patch gets applied, making the core functions safe
> to use.
> 2. The function implementations whose semantic has changed get
> renamed; I kind of like the _r suffix, but _alloc or _copy would
> also work.
> 3. Under the old names small wrappers are added which establish the
> old behavior again by copying the string into the ring buffer
> and freeing the dynamically allocated one: this incurs some
> overhead, but usage of these versions of the calls is
> discouraged anyway. By using function attributes it would be
> possible to trigger deprecation warnings for code which still
> uses them.
> 4. In the header file both variants are declared.
> 5. In addition, ical.h also redefines the old names to the new
> names if the HANDLE_LIBICAL_MEMORY define is set: Evolution code
> already does that and therefore continues to work with such a
> modified upstream libical without source code changes.
>
> I personally would prefer to avoid step 5 and rather do a search/replace
> in Evolution, but Chenthill didn't like that.
Since we do really want to remove the fork and pick up packages from
upstream, I can change the apis in evolution related packages if a new
set of apis with some suffix is provided from libical upstream.
The old patch is attached with bug 516408. You can find the patches at,
http://svn.gnome.org/viewvc/libical?view=revision&revision=633
and
http://svn.gnome.org/viewvc/libical?view=revision&revision=637
thanks, Chenthill.
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]