Re: [Evolution-hackers] More e-d-s ABI breakage ?
Hi Ross,
On Fri, 2006-08-11 at 17:32 +0100, Ross Burton wrote:
> The change was required for the 770 as ferrying the photos over the bus
Sure - I like the core change, but not the API/ABI detail :-)
> If it's required it might be possible to revert that change and
> introduce an alternative. As you say, a "photo2" property that returns
> a new type should be sufficient, I'll have a look later.
Sure, although of course it may be more complex than that; this was a
1st glance review.
> I have Grand Plans to write a replacement for EVCard and EContact
> though. I've been adding new API to e-vcard to make it more usable, but
> that is just complicating the API for no gain. Extending EContact is a
> pain as all of the data types are public structs with no padding.
Sounds lovely. From an OO.o perspective we want a thread-safe 'cursor /
iterator' API ;-) as in:
for (line = do_query_get_1st; line; line = line->getnext()) {
...
}
but I guess we handle all our async stuff via threads.
> EContact also manages to hit that sweet spot between
> complicated-but-powerful and easy-but-limited, so it's
> complicated-and-limited at once.
Heh ;-) I'm sure there are (no doubt) tons of problems with the API -
however, a drip, drip of incompatible breakage is no solution IMHO.
Clearly building your nice new API in parallel, deprecating the old API,
and [ in a few versions time ] doing a big bang switch to the new (by
now perfect, tested, GObject based, extensible etc. ;-) API seems a
better approach. Hopefully we can write an 'evoab3' connector to it
then.
All the best,
Michael.
--
[EMAIL PROTECTED] <><, Pseudo Engineer, itinerant idiot
___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] More e-d-s ABI breakage ?
Hi dudie, On Fri, 2006-08-11 at 21:47 +0530, Harish Krishnaswamy wrote: > I had not reviewed the patch or explored the alternatives to avoid > breakage. I would let the addressbook hackers to comment on that. > I do think you have a point above, though. Good 'oh :-) should be easy to fix and downgrade that .so number at least. > Point taken - The original patch would have introduced a break - this > was reworked as an append before it was committed. Which is great; thanks. > > > The #313533 patch was vital for Ross Burton's dbus-based EDS and running > > > EDS on devices (say Nokia 770) would not be possible w/o this change. > > > > Nonsense - at least the link above has no API change that is necessary > > for dbus or Nokia 770 support - unless I'm missing something huge; good > > buzz-words though :-) > > I feel sorry to know you would think I would stoop to get around the > issue with buzzwords. So - I think we're talking at cross purposes here, :-) so - my issue is that the whole Nokia / performance thing / whatever is -almost always- orthogonal to the ABI breakage. I'm *excited* about adding new features to e-d-s, don't suppose for a moment that I want to stop that process, hinder features getting in, and/or make your life a misery: that is emphatically not my aim. My interest is in getting these things done compatibly; and I've yet to see an addition that could not be done (quite neatly) in such a way. So - when I ask why the ABI was broken, and I hear this "it was vital for Nokia" thing :-) I get confused. > The performance tests showed this was prohibitive for EDS on DBUS. Sure, sure it's a nice feature - but as I say, my problem is not at all with the feature itself, only the unnecessary ABI/API breakage. > See http://www.go-evolution.org/DBus_Port_of_EDS . And I'm well up for this too :-) HTH, Michael. -- [EMAIL PROTECTED] <><, Pseudo Engineer, itinerant idiot ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] More e-d-s ABI breakage ?
On Fri, 2006-08-11 at 15:54 +0100, Michael Meeks wrote: > > The #313533 patch was vital for Ross Burton's dbus-based EDS and running > > EDS on devices (say Nokia 770) would not be possible w/o this change. > > Nonsense - at least the link above has no API change that is necessary > for dbus or Nokia 770 support - unless I'm missing something huge; good > buzz-words though :-) The change was required for the 770 as ferrying the photos over the bus every time the contact is transferred is too large a hit for the relatively low number of places that a photo is used. The solution was to expand the photo struct to support URIs as well as inline, and then store the photo on disk. If it's required it might be possible to revert that change and introduce an alternative. As you say, a "photo2" property that returns a new type should be sufficient, I'll have a look later. I have Grand Plans to write a replacement for EVCard and EContact though. I've been adding new API to e-vcard to make it more usable, but that is just complicating the API for no gain. Extending EContact is a pain as all of the data types are public structs with no padding. EContact also manages to hit that sweet spot between complicated-but-powerful and easy-but-limited, so it's complicated-and-limited at once. Ross -- Ross Burton mail: [EMAIL PROTECTED] jabber: [EMAIL PROTECTED] www: http://www.burtonini.com./ PGP Fingerprint: 1A21 F5B0 D8D0 CFE3 81D4 E25A 2D09 E447 D0B4 33DF signature.asc Description: This is a digitally signed message part ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] More e-d-s ABI breakage ?
On Fri, 2006-08-11 at 15:54 +0100, Michael Meeks wrote: > Hi Harish, > > First - thanks for digging these changes out for me. But - no, I'm not > just interested in ebook (though for OO.o that is all), but I'm > -primarily- interested Evo. itself, in being able to use and test the > most recent version to help avoid regressions, and indeed ship it for > older platforms. > > So - if you could do the same for the other e-d-s libraries, it'd be > great to see what changed there too. > > On Fri, 2006-08-11 at 20:03 +0530, Harish Krishnaswamy wrote: > > The changes in question are as follows : > > http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.20&r2=1.21 > > So - this changed the EContactPhoto structure - why ? surely that is > rather pointless. You could easily have added an EContactMimePhoto type > and added a synthetic back-compat field that would handle the old case > [ if it was of (whatever) mime type you expected ]. So - I see no > problem at all doing this compatibly whatsoever. Perhaps a few more > (~20) lines of code tops. > I had not reviewed the patch or explored the alternatives to avoid breakage. I would let the addressbook hackers to comment on that. I do think you have a point above, though. [1] OTH, I did approve the change into the release on the clear basis that ferrying Photo images on Contacts was prohibitively hampering the performance of the dbus port and the library had all to gain by ferrying a url instead. > > http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.21&r2=1.22 > > You converted a gpointer value to a 'const gpointer value' - I don't > see that that is particularly necessary, or likely to break the ABI of > anything unless it reflects some underlying lifecycle issue. Also the > enum insertions were (this time) added at the end of the enumeration, so > why should that break anything ? surely that's a compatible extension. Point taken - The original patch would have introduced a break - this was reworked as an append before it was committed. > > > > The #313533 patch was vital for Ross Burton's dbus-based EDS and running > > EDS on devices (say Nokia 770) would not be possible w/o this change. > > Nonsense - at least the link above has no API change that is necessary > for dbus or Nokia 770 support - unless I'm missing something huge; good > buzz-words though :-) > I feel sorry to know you would think I would stoop to get around the issue with buzzwords. http://www.burtonini.com/blog//2006/Jul/22 The performance tests showed this was prohibitive for EDS on DBUS. > I didn't explored the problem any deeper but appears that there is > some problem with e_book_get_contacts code. > > For detailed timing reports on this test download the following files > and open them with sysprof. See http://www.go-evolution.org/DBus_Port_of_EDS . --Harish ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] More e-d-s ABI breakage ?
On Fri, 2006-08-11 at 21:47 +0530, Harish Krishnaswamy wrote: > On Fri, 2006-08-11 at 15:54 +0100, Michael Meeks wrote: > > Hi Harish, > > > > First - thanks for digging these changes out for me. But - no, I'm not > > just interested in ebook (though for OO.o that is all), but I'm > > -primarily- interested Evo. itself, in being able to use and test the > > most recent version to help avoid regressions, and indeed ship it for > > older platforms. > > > > So - if you could do the same for the other e-d-s libraries, it'd be > > great to see what changed there too. > > > > On Fri, 2006-08-11 at 20:03 +0530, Harish Krishnaswamy wrote: > > > The changes in question are as follows : > > > http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.20&r2=1.21 > > > > So - this changed the EContactPhoto structure - why ? surely that is > > rather pointless. You could easily have added an EContactMimePhoto type > > and added a synthetic back-compat field that would handle the old case > > [ if it was of (whatever) mime type you expected ]. So - I see no > > problem at all doing this compatibly whatsoever. Perhaps a few more > > (~20) lines of code tops. > > > I had not reviewed the patch or explored the alternatives to avoid > breakage. I would let the addressbook hackers to comment on that. > I do think you have a point above, though. > > [1] OTH, I did approve the change into the release on the clear basis > that ferrying Photo images on Contacts was prohibitively hampering the > performance of the dbus port and the library had all to gain by ferrying > a url instead. > > > > http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.21&r2=1.22 > > > > You converted a gpointer value to a 'const gpointer value' - I don't > > see that that is particularly necessary, or likely to break the ABI of > > anything unless it reflects some underlying lifecycle issue. Also the > > enum insertions were (this time) added at the end of the enumeration, so > > why should that break anything ? surely that's a compatible extension. > > Point taken - The original patch would have introduced a break - this > was reworked as an append before it was committed. > > > > > > > The #313533 patch was vital for Ross Burton's dbus-based EDS and running > > > EDS on devices (say Nokia 770) would not be possible w/o this change. > > > > Nonsense - at least the link above has no API change that is necessary > > for dbus or Nokia 770 support - unless I'm missing something huge; good > > buzz-words though :-) > > > > See http://www.go-evolution.org/DBus_Port_of_EDS . The charts on the link seems to be broken ATM..I will post the performance charts to the thread. BTW, An interesting link I came across... http://applications.linux.com/article.pl?sid=06/08/04/2158214&from=rss --Harish ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] More e-d-s ABI breakage ?
Hi Harish, First - thanks for digging these changes out for me. But - no, I'm not just interested in ebook (though for OO.o that is all), but I'm -primarily- interested Evo. itself, in being able to use and test the most recent version to help avoid regressions, and indeed ship it for older platforms. So - if you could do the same for the other e-d-s libraries, it'd be great to see what changed there too. On Fri, 2006-08-11 at 20:03 +0530, Harish Krishnaswamy wrote: > The changes in question are as follows : > http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.20&r2=1.21 So - this changed the EContactPhoto structure - why ? surely that is rather pointless. You could easily have added an EContactMimePhoto type and added a synthetic back-compat field that would handle the old case [ if it was of (whatever) mime type you expected ]. So - I see no problem at all doing this compatibly whatsoever. Perhaps a few more (~20) lines of code tops. > http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.21&r2=1.22 You converted a gpointer value to a 'const gpointer value' - I don't see that that is particularly necessary, or likely to break the ABI of anything unless it reflects some underlying lifecycle issue. Also the enum insertions were (this time) added at the end of the enumeration, so why should that break anything ? surely that's a compatible extension. > This was not a trigger-happy change. I beg to differ. > The patch for the first bug was submitted on 15 Aug 2005 (almost a year > ago) and that for the second on 3 Mar 2004 ( more than 2 years ago) and > had been in the back-burner only because we had wanted to preserve the > ABI. You realise you can *extend* the API/ABI without breaking the ABI right ? > The #313533 patch was vital for Ross Burton's dbus-based EDS and running > EDS on devices (say Nokia 770) would not be possible w/o this change. Nonsense - at least the link above has no API change that is necessary for dbus or Nokia 770 support - unless I'm missing something huge; good buzz-words though :-) > The #259536 patch had been blocking a large number of users in Poland > where Gadu Gadu is the defacto IM standard. And doesn't break ABI anyway (AFAICS). > IMO, a single break after two major releases against such gains is > justifiable. These patches were reviewed and approved by the addressbook > hackers before the bump. I'm sure they were reviewed ;-) I'm not debating that. > Could these improvements have been absorbed w/o breaking the ABI and > with not too much additional costs in the process? Yes - clearly so. > I do not know - nor did the patch authors or the reviewers. But if you > could show us, I am all willing to learn. Hokay - so - to get the new functionality you require changes on the client side anyway, so we can ignore that as an issue. e-contact.c *already* supports 'synthetic' properties [ that duplicate state in other properties ]. We should have simply added a Photo2 structure [ or some renamed thing ], changed the name of the property to reflect that, and updated all callers that cared. [ NB. we had to update all callers anyway, but with your change, since they using a type-unsafe gobject property, unchanged callers will SEGV when they call this method ]. While this was being fixed we should have used a GObject for EContactMimePhoto ( or whatever ) so we could compatibly extend it in future. > Can you also help me understand the severity of the break in OO.o ? OO.o is one symptom of the huge system-wide problem that you cause for every application developer foolhardy enough to link against evolution-data-server. Every user, now who wants to eg. upgrade 'Ekiga' to the latest version [ across this breakage ] will discover that going from 1.1.x to 1.2.y he will have to upgrade some huge chunk of his system - or simply give-up and not test/use the latest version. That is a horrible idea for anyone - particularly when it's totally unnecessary. > Why are the library files with SONAMEs hard-coded in a source file ? Some applications exist that have to run on whatever is there ? - this is called being an ISV :-) - Sun is an ISV for Linux, this means a world of pain [ created by exactly this kind of breakage ]. ie. if OO.o is going to run on NLD9, and SL10.0, and SL10.1, [ and now SL 10.2 ] how do you recommend we integrate with evolution-data-server any other way ? - we -have- to dlopen libraries since they may not be there, and we then have to try to adapt to whatever breakage they introduced across versions. > Why would you not use (say) pkg-config instead ? This is not what pkg-config files are for, and anyway: $ rpm -qf /opt/gnome/lib/pkgconfig/libebook-1.2.pc evolution-data-server-devel-1.6.0-43.20 I'd love to see this change undone ASAP, before code-freeze for Gno
Re: [Evolution-hackers] More e-d-s ABI breakage ?
On Fri, 2006-08-11 at 10:00 +0100, Michael Meeks wrote:
> So,
> --- source/drivers/evoab2/EApi.cxx
> +++ source/drivers/evoab2/EApi.cxx
> @@ -41,9 +41,10 @@
> #include "EApi.h"
> #endif
> static char *eBookLibNames[] = {
> - "libebook.so.8", // evolution-2.0
> + "libebook-1.2.so.9", // ** evolution-2.8 *
> + "libebook-1.2.so.5", // evolution-2.4 and 2.6+
> "libebook-1.2.so.3", // evolution-2.2
> - "libebook-1.2.so.5" // evolution-2.4 and 2.6+
> + "libebook.so.8" // evolution-2.0
> };
>
> WTF is going on !? why was this done ? what was the reason for this
> [ FWIW it seems that no other code changes were necessary (so far) ].
We discussed about the calendar changes in IRC but I believe it is
libebook that you really care about.
The changes in question are as follows :
http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.20&r2=1.21
http://cvs.gnome.org/viewcvs/evolution-data-server/addressbook/libebook/e-contact.h?r1=1.21&r2=1.22
The bugs that drove this change are :
* http://bugzilla.gnome.org/show_bug.cgi?id=313533
[ Allow EContactPhoto to refer to images with URIs]
* http://bugzilla.gnome.org/show_bug.cgi?id=259536
[Missing Gadu-Gadu Messenger in Contact entry]
This was not a trigger-happy change.
The patch for the first bug was submitted on 15 Aug 2005 (almost a year
ago) and that for the second on 3 Mar 2004 ( more than 2 years ago) and
had been in the back-burner only because we had wanted to preserve the
ABI.
The #313533 patch was vital for Ross Burton's dbus-based EDS and running
EDS on devices (say Nokia 770) would not be possible w/o this change.
The #259536 patch had been blocking a large number of users in Poland
where Gadu Gadu is the defacto IM standard.
In Bugzilla (cf. above links) and elsewhere, there have been vocal
demands for absorbing these patches. See my comment -
http://bugzilla.gnome.org/show_bug.cgi?id=259536#c22 We did aggregate
the changes before the break.
IMO, a single break after two major releases against such gains is
justifiable. These patches were reviewed and approved by the addressbook
hackers before the bump.
Could these improvements have been absorbed w/o breaking the ABI and
with not too much additional costs in the process?
I do not know - nor did the patch authors or the reviewers. But if you
could show us, I am all willing to learn.
Can you also help me understand the severity of the break in OO.o ?
Why are the library files with SONAMEs hard-coded in a source file ?
Why would you not use (say) pkg-config instead ?
--Harish
___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Adding documentation to Evolution's code
On Fri, 2006-08-11 at 16:40 +0530, Mayank Jain wrote: > On 8/11/06, Ross Burton <[EMAIL PROTECTED]> wrote: > > On Fri, 2006-08-11 at 16:09 +0530, Mayank Jain wrote: > > > But using gtk-doc style commenting create a problem? > > > When I go ahead & document the non-public functions, their doc will > > > also be included in gtk-doc output when its generated. > > > > > > This will grossly confuse the API user as he'll be trying to use a > > > private function. > > > > I don't believe that private functions will be included in the docs, as > > (AFAIK) it uses the headers to determine what to output. > > Lets take an example... > reset_layout is a function related to EText widget & is private to it > and is in file > $evo/widgets/text/e-text.c > > /** > * reset_layout: > * @text: The text widget on which this function will be called > * > * Resets layout to its default values, checks for an already existing layout > * resets it to default values if it exists else create a new layout > from #create_layout call > * > * Return value: Nothing > **/ > > If i include this as doc for the reset_layout function, will it be > included in the gtk-doc output? As far as I know, if its a static function and thus it isn't in e-text.h, then it won't be output. Ross -- Ross Burton mail: [EMAIL PROTECTED] jabber: [EMAIL PROTECTED] www: http://www.burtonini.com./ PGP Fingerprint: 1A21 F5B0 D8D0 CFE3 81D4 E25A 2D09 E447 D0B4 33DF signature.asc Description: This is a digitally signed message part ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Adding documentation to Evolution's code
On 8/11/06, Ross Burton <[EMAIL PROTECTED]> wrote: > On Fri, 2006-08-11 at 16:09 +0530, Mayank Jain wrote: > > But using gtk-doc style commenting create a problem? > > When I go ahead & document the non-public functions, their doc will > > also be included in gtk-doc output when its generated. > > > > This will grossly confuse the API user as he'll be trying to use a > > private function. > > I don't believe that private functions will be included in the docs, as > (AFAIK) it uses the headers to determine what to output. Lets take an example... reset_layout is a function related to EText widget & is private to it and is in file $evo/widgets/text/e-text.c /** * reset_layout: * @text: The text widget on which this function will be called * * Resets layout to its default values, checks for an already existing layout * resets it to default values if it exists else create a new layout from #create_layout call * * Return value: Nothing **/ If i include this as doc for the reset_layout function, will it be included in the gtk-doc output? Thanks, Mayank ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Adding documentation to Evolution's code
On Fri, 2006-08-11 at 16:09 +0530, Mayank Jain wrote: > But using gtk-doc style commenting create a problem? > When I go ahead & document the non-public functions, their doc will > also be included in gtk-doc output when its generated. > > This will grossly confuse the API user as he'll be trying to use a > private function. I don't believe that private functions will be included in the docs, as (AFAIK) it uses the headers to determine what to output. Ross -- Ross Burton mail: [EMAIL PROTECTED] jabber: [EMAIL PROTECTED] www: http://www.burtonini.com./ PGP Fingerprint: 1A21 F5B0 D8D0 CFE3 81D4 E25A 2D09 E447 D0B4 33DF signature.asc Description: This is a digitally signed message part ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Adding documentation to Evolution's code
On 8/11/06, Mayank Jain <[EMAIL PROTECTED]> wrote: > On 8/10/06, Andre Klapper <[EMAIL PROTECTED]> wrote: > > hej mayank, > > > > Am Mittwoch, den 09.08.2006, 17:43 +0530 schrieb Mayank Jain: > > > With that in mind, I would like to add more documentation, comments > > > (with whatever knowledge I have) to the code. > > > > any additional documentation is most appreciated! > > matthew already posted about gtk-doc which would be imo the best way to > > do this. i'd prefer to have a bug report for each component and to > > attach patches (cvs diff -pu) to them. > > > > thanks a lot for coming up with this, > > andre But using gtk-doc style commenting create a problem? When I go ahead & document the non-public functions, their doc will also be included in gtk-doc output when its generated. This will grossly confuse the API user as he'll be trying to use a private function. Thanks, Makuchaku ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] More e-d-s ABI breakage ?
So,
After all the acute pain we suffered in the past with e-d-s breakage,
and the monstrous lack of testing this caused by ensuring it was not
possible to run newer versions on older systems; which [ combined with
the lack of snapshots IMHO ] in turn caused large numbers of
undiscovered regressions in evo 2.6; I -thought- we had some agreement
written in blood that the e-d-s ABI was now frozen.
Then I saw this new patch[1] to the OO.o hacks to work around the
churning e-d-s ABI breakage going into OO.o:
--- source/drivers/evoab2/EApi.cxx
+++ source/drivers/evoab2/EApi.cxx
@@ -41,9 +41,10 @@
#include "EApi.h"
#endif
static char *eBookLibNames[] = {
- "libebook.so.8", // evolution-2.0
+ "libebook-1.2.so.9", // ** evolution-2.8 *
+ "libebook-1.2.so.5", // evolution-2.4 and 2.6+
"libebook-1.2.so.3", // evolution-2.2
- "libebook-1.2.so.5" // evolution-2.4 and 2.6+
+ "libebook.so.8" // evolution-2.0
};
WTF is going on !? why was this done ? what was the reason for this
[ FWIW it seems that no other code changes were necessary (so far) ].
Can it be undone ? preferably ASAP - and some huge comment added to the
libtool versioning piece that this major must never change again ?
Urgh,
Michael.
[1] - http://www.openoffice.org/issues/show_bug.cgi?id=68383
--
[EMAIL PROTECTED] <><, Pseudo Engineer, itinerant idiot
___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers
