Re: [PATCHv4 0/4] dpdk: enhance EXPERIMENTAL api tagging

[Neil Horman <nhorman@tuxdriver.com>]

On Sat, Dec 30, 2017 at 08:20:58PM +0100, Luca Boccassi wrote:
> On Wed, 2017-12-13 at 10:17 -0500, Neil Horman wrote:
> > Hey all-
> >         A few days ago, I was lamenting the fact that, when reviewing
> > patches I
> > would frequently complain about ABI changes that were actually
> > considered safe
> > because they were part of the EXPERIMENTAL api set.  John M. asked me
> > then what
> > I might do to improve the situation, and the following patch set is a
> > proposal
> > that I've come up with.
> > 
> >         In thinking about the problem I identified two issues that I
> > think we
> > can improve on in this area:
> > 
> > 1) Make experimental api calls more visible in the source code.  That
> > is to say,
> > when reviewing patches, it would be nice to have some sort of visual
> > reference
> > that indicates that the changes being made are part of an
> > experimental API and
> > therefore ABI concerns need not be addressed
> > 
> > 2) Make experimenal api usage more visible to consumers of the DPDK,
> > so that
> > they can make a more informed decision about the API's they consume
> > in their
> > application.  We make an effort to document all the experimental
> > API's, but
> > there is no guarantee that a user will check the documentation before
> > making use
> > of a new library.
> > 
> > This patch set attempts to achieve both of the above goals.  To do
> > this I've
> > added an __experimental macro tag, suitable for inserting into api
> > forward
> > declarations and definitions.
> > 
> > The presence of the tag in the header and c files where the api code
> > resides
> > increases the likelyhood that any patch submitted against them will
> > include the
> > tag in the context, making it clear to reviewers that ABI stability
> > isn't a
> > concern here.
> > 
> > Also, This tag marks each function it is used on with an attibute
> > causing any
> > use of the fuction to emit a warning during the build
> > with a message indicating that the API call in question is not yet
> > part of the
> > stable interface.  Developers can then make an informed decision to
> > suppress
> > that warning or not.
> > 
> > Because there is internal use of several experimental API's, this set
> > also
> > includes a new override macro ALLOW_EXPERIMENTAL_APIS to
> > automatically
> > suprress these warnings.  I think its fair to assume that, for
> > internal use, we
> > almost always want to suppress these warnings, as by definition any
> > change to
> > the apis (even their removal) must be done in parallel with an
> > appropriate
> > change in the calling locations, lest the dpdk build itself break.
> > 
> > Neil
> > 
> > ---
> > Change Notes:
> > v2)
> > * Cleaned up checkpatch errors
> > * Added Allowance for building experimental on BSD
> > * Swapped Patch 3 and 4 so that we didn't have a commit level that
> > issued
> >   warnings/errors without need
> > 
> > v3)
> > * On suggestion from Bruce, modify ALLOW_EXPERIMENTAL_APIS to be
> > defined in
> >   CFLAGS rather than a makefile variable.  This is more flexible in
> > that it
> >   allows us to suppress this specific feature rather than all uses of
> > the
> >   deprecated attribute, as we might use it for other features in the
> > furute
> > 
> > v4)
> > * Added documentation patch to contributors guide
> 
> Acked-by: Luca Boccassi <bluca@debian.org>
> 
> I really like the idea of showing warnings at build time for users of
> the libraries, in fact I like it so much I'm going to shamelessly steal
> it for another few projects I work on where we have an experimental
> (DRAFT) api system :-)
> 
You're welcome to it :)
Neil