Bug 23275

Looks good in principle, but before we merge this, could you please hack up the build system so it temporarily produces a shared library, pass it through nm and c++filt, attach the result, and check that there aren't symbols that appear in our ABI but that we are not prepared to commit to?

A command like this might be helpful:

nm .libs/libtelepathy-qt4.a | grep " [DT] " | c++filt | sort -k3

Created attachment 28610 [details]
Exported symbols

(In reply to comment #1)
> Looks good in principle, but before we merge this, could you please hack up the
> build system so it temporarily produces a shared library, pass it through nm
> and c++filt, attach the result, and check that there aren't symbols that appear
> in our ABI but that we are not prepared to commit to?
> 
> A command like this might be helpful:
> 
> nm .libs/libtelepathy-qt4.a | grep " [DT] " | c++filt | sort -k3
> 

Despite the XXX::Private methods I don't see any other symbol that should not be there. Should we add TELEPATHY_QT4_NO_EXPORT to all Private structs? Qt does not do it, as they use shared d pointers in some places, but we don't, at least for now. I would let it as is.

Also exported non-class methods, enableDebug, registerTypes, ...

Rejected for now, please re-add patch tag when you've fixed it.

(In reply to comment #3)
> Despite the XXX::Private methods I don't see any other symbol that should not
> be there. Should we add TELEPATHY_QT4_NO_EXPORT to all Private structs?

Yes, definitely. At the moment, they're part of our shared library ABI; that means that we should never remove, rename, or change the semantics of these methods, which isn't desirable.

Also, if we have any private methods on public objects, we should make sure they're not exported either.

> Also exported non-class methods, enableDebug, registerTypes, ...

The extra patch looks fine.

(In reply to comment #4)
> Rejected for now, please re-add patch tag when you've fixed it.
> 
> (In reply to comment #3)
> > Despite the XXX::Private methods I don't see any other symbol that should not
> > be there. Should we add TELEPATHY_QT4_NO_EXPORT to all Private structs?
> 
> Yes, definitely. At the moment, they're part of our shared library ABI; that
> means that we should never remove, rename, or change the semantics of these
> methods, which isn't desirable.
> 
> Also, if we have any private methods on public objects, we should make sure
> they're not exported either.
Done.

Created attachment 28757 [details]
Exported symbols

Comment on attachment 28757 [details]
Exported symbols

>000000000007e000 T Tp::Account::gotAvatar(QDBusPendingCallWatcher*)
>0000000000086450 T Tp::Account::gotMainProperties(QDBusPendingCallWatcher*)

That doesn't look right. Seems we need to unexport private methods explicitly...

Updated to latest changes

Fixed upstream, will be in next release 0.2