Re: [g-a-devel] RFC: AtkText simplification (take 3)

[Piñeiro <apinheiro igalia com>]

On 08/06/2013 10:11 PM, Trevor Saunders wrote:
> On Tue, Aug 06, 2013 at 04:22:40PM +0200, Mario wrote:
> > Hi Trevor,
> >
> > On 6 August 2013 16:05, Trevor Saunders <trev saunders gmail com> wrote:
> > > > 6. In atspi2-atk bridge, check which version of ATK a specific
> > > > application is implementing (using atk_get_version()) when
> > > > implementing this new atk_text_get_text_for_offset(), so we know
> > > > whether we can call atk_text_get_text_for_offset() or we need to use
> > > > the old atk_text_get_text_at_offset() and a *_START boundary instead
> > > > (for implementors of older versions of ATK).
> > > afaik atk_get_version() only tells you what version of libatk-1.0.so you
> > > are talking to not what version of the atk headers an application was
> > > compiled against.  So I believe that would cause breakage when people
> > > compile applications against atk pre this change and then run that
> > > application with a newer libatk-1.0.so.
> > Yes you are right. We already realized a while ago this part was wrong
> > and were already thinking that a better approach would be to use
> > ATK_CHECK_VERSION() to know whether we need to use the old API or if
> > we can use the new one (and maybe with a fallback to the old API in
> > case the app that is running has not been updated yet to it and it's
> > still implementing the old functions).
> Well, what's important is the version of atk that the application was
> compiled against not what version of atk you're compiling atk-bridge
> against. 

Both are important. In order to use the new method, atk-bridge needs to
compile and link against a version of atk that contains the new method.
And atk will provide a dummy implementation of that method, so in fact
they can call it.

In any case, you are right in something. ATK_CHECK_VERSION will inform
you about the atk version the bridge is compiling against, not the one
used by the implementor. So the only advantage of using
ATK_CHECK_VERSION is that you don't need to update the dependency of the
atk-bridge. And taking into account that this is the atk bridge, seems
like an overkill. Using ATK_CHECK_VERSION makes sense on projects like
WebKitGTK where there are several dependencies, and ATK is "just one
more", but in at-spi-atk, atk is one of the more important ones, so
updating the atk version needed is what makes sense, imho. I will
mention that on the patch review.

>  So about the only way I can think of to know what version of
> atk a an application was compiled with given only its binary is to see
> if it defines a symbol and then arrange to have atk define this symbol
> at the same time you add this new virtual function.  

The general idea is what Frederik proposed on an ATK/AT-SPI hackfest,
providing a way to know if the current implementation is providing a
feature or not. Anyway, your implementation proposal seems somewhat
intrusive, and in fact ...

> However having your
> headers inject symbols feels somewhat rude.  
... it seems that you agree with that.

> I *believe* the symbol
> approach will work, but changing vtables and not changing abi feels
> dangerous to me and I would try to avoid it unless I had no other
> reasonable option.

I think that in the end we shouldn't worry too much about this in this
scope. I would maintain the temporal wrapper to the old method for this
bug (fwiw, bgo#705581) and think about the other problem from a more
general pov. I wouldn't block this patch for that issue.

BR

-- 
Alejandro Piñeiro Iglesias