Re: [Tarantool-patches] [PATCH 2.X 0/7] RFC: module api: extend for external merger Lua module

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 02.10.2020 14:23, Timur Safin wrote:
> Before getting into all gory details and start to actively participate in discussion 
> I want to highlight some basic principles which we used while preparing these simple 
> patches (some are obvious, but some not that much):
>
> - I try to offload functionality to the code existing inside of Tarantool as much as 
> possible. If there are some structures or functions - refer to them. And this principle 
> better be applied not only to Tarantool core specific functionality, but also to all 3rd 
> party functions it uses. E.g. small, msgpuck, or even heap.
>
> - And current principle to compile against them is simple - we use these 3rd party headers 
> for compilation of functions in the module, but ignore libraries for a moment. We delay 
> binding till run-time where system loader would substitute symbol from Tarantool executable 
> which eventually called us as external module. If we could use msgpuck or small in this 
> manner then we almost entirely avoiding most ABI incompatibility problems. 

The headers don't contain just symbols. They contain inlined function
definitions and non-opaque structures. Even if your module does not link
with these libraries, still some of their functions and their structs layout
penetrate into your module's binary file. And if tarantool is built with a
bit different headers with slightly changed structs layout, it will lead
to UB.

That is the main problem with ABI now. It is not about symbols. If some
symbols are missing, your module will fail to load.

> - The question is: where to get those 3rd party headers? We may use several approaches 
>   in modules:
> 
> 1. via cmake ExternalProject_Add to github repo;
> 2. via git submodule to github repo;
> 3. via systems package manager dependency and binary dependency (e.g. apt or rpm).>
> So happened, that in merger we used several of them, e.g. submodule for small https://github.com/tsafin/tarantool-merge/tree/master/src
> tarantool-dev system package for msgpuck. And simply copy-pasted heap.h from salad.
> [IMVHO, salad worth to be treated similarly as small. For better reuse we may export
> It to external repository]
> 
> 3 different approaches for quite similar cases of code reuse. :) Eventually we may 
> develop the consistent approach here - how to reuse 3rd party Tarantool library 
> in ABI stable way, which may be originating of different versions of Tarantool? 
> (e.g. small in 2 its incarnations from 2.x and 1.10)
> 
> Would it be always enough to use older version of headers, and run-time linking with 
> parent executable? At the moment I see no reason to not do so. As far as I can see 
> small is quite similar from ABI prospective between 1.10 and 2.x at the moment, 
> we could start away as is, but certainly should add binary compatibility check somewhere 
> into CI (to small repo, to Tarantool repo, and/or to merger repo) for future 
> compatibility guarantees. I do assume that small repository was created just for such
> usage scenarios like here (module reusing some useful functionality), and it's 
> correct way to use it as submodule or ExternalProject_ in cmake. We just need to
> make sure it will not broken later.
> 
> Same for msgpuck - it's already created for better code reuse - we just need to 
> Make sure it's possible to be used in future compatible way. (Via extra CI here 
> and there).
> 
> Having said all that, now we could return to patches discussions - see my 
> other emails...

If you are proposing to introduce some kind of public version of
small API, doing all the compatibility stuff, then it sounds good.
(msgpuck is already public.)

Now we are trying to solve that inside of tarantool's API, but maybe
it would be easier and more portable the other way.