Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs

[Neil Horman <nhorman@tuxdriver.com>]

On Mon, Jul 06, 2015 at 03:49:50PM +0200, Thomas Monjalon wrote:
> 2015-07-06 09:35, Neil Horman:
> > On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > > Any comment or ack?
> > > 
> > > 2015-07-03 00:05, Thomas Monjalon:
> > > > When a change makes really hard to keep ABI compatibility,
> > > > instead of waiting next release to break the ABI, it is smoother
> > > > to introduce the new code and enable it only for static libraries.
> > > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > > When the release is out, a dynamically linked application can use
> > > > the new shared libraries without rebuild while developpers can prepare
> > > > their application for the next ABI by reading the deprecation notice
> > > > and easily testing the new code.
> > > > When starting the next release cycle, the "ifdefs" will be removed
> > > > and the ABI break will be marked by incrementing LIBABIVER.
> > > > 
> > > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > > It is automatically enabled for static libraries and disabled for
> > > > shared libraries.
> > > > It can be forced to another value by editing the generated .config file.
> > > > It shouldn't be enabled for shared libraries because it would break the
> > > > ABI without changing the version number LIBABIVER. That's why a warning
> > > > is printed in this case.
> > > > 
> > > > The guideline is also updated to integrate this new possibility.
> > > > 
> > > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > 
> > > 
> > Yeah, I'm not sure why this is necessecary.  That is to say, if you want to
> 
> It's explained at the beginning:
> "When a change makes really hard to keep ABI compatibility", e.g. mbuf change.
> 

Thats not what I was referring to.  I was referring to the need to split out
ABI's based on a separate config item only for static libraries.  I understand
that sometimes you want a 'preview' of the next abi.

> > introduce a new ABI operation prior to the old one being removed, that is precisely what
> > the versioning macros are for, and can be used to map the static api to the new
> > version. e.g, given function X that you want to enhance in an ABI breaking way:
> > 
> > 1) Separate function X to X_v1 and X_v2
> > 2) Map X_v2 to X@DPDK_v2, map X_v1 to X@DPDK_v1
> > 3) Map the static symbol X to X_v2
> > 4) Post the deprecation notice of X for release 3 immediately
> 
> We cannot do that for mbuf change.
> 
You can actually, its just alot of work.  Also, I know this doesn't relate very
closely to the subject, and I apologize, I was really just reacting to the
immediately preceding sentence in the origional post.

> > Splitting the static ABI from the shared ABI just means that applications will
> > have the opportunity to isolate themselves to one kind of build, which is a bad
> > idea.
> 
> It helps to be prepared for the next release by testing the app with static libraries.
> We agreed to allow API breaking for important changes like mbuf rework.
> This option NEXT_ABI is a step between announcement and effective ABI breaking.
> 
> I think it's a reasonnable approach. But if nobody ack it, I'm perfectly OK to
> drop it and related features (unified packet type and interrupt mode).
> 

I'd be ok with it iff:

1) It applies to static and shared ABI's together.  That is to say that setting
the NEXT_ABI config flag creates the same ABI changes regardless of other build
configuration.  It needs to be used in such a way that a consistent ABI is
presented when set, otherwise it won't be useful.

2) It only applies to the next ABI.  That is to say, it can't be a hodgepodge of
the next ABI and the one after that, and the one after that, or it won't provide
an appropriate preview for anyone.


If we can meet those two standards, it would likely be a useful feature to have.
Neil

>