DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ray Kinsella <mdr@ashroe.eu>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, stephen@networkplumber.org,
	bruce.richardson@intel.com, ferruh.yigit@intel.com,
	konstantin.ananyev@intel.com, jerinj@marvell.com,
	olivier.matz@6wind.com, nhorman@tuxdriver.com,
	maxime.coquelin@redhat.com, john.mcnamara@intel.com,
	marko.kovacevic@intel.com, hemant.agrawal@nxp.com,
	ktraynor@redhat.com, aconole@redhat.com
Subject: Re: [dpdk-dev] [PATCH v8 2/4] doc: changes to abi policy introducing major abi versions
Date: Wed, 6 Nov 2019 08:49:32 +0000	[thread overview]
Message-ID: <dd31798b-a702-ecde-aafa-480c52a843d8@ashroe.eu> (raw)
In-Reply-To: <1582816.H2LZHI4QzJ@xps>



On 06/11/2019 00:11, Thomas Monjalon wrote:
> I'm sorry I still have some comments.
> But on the positive side, you can see that I carefuly read this doc.

no worries. 

> 
> 05/11/2019 16:24, Ray Kinsella:
>> +#. Major ABI versions are declared every **year** and are then supported for one
>> +   year, typically aligned with the :ref:`LTS release <stable_lts_releases>`.
> 
> As discussed earlier, a major ABI version can be declared less often
> than one year in the future.
> An ABI is supported more than one year, because of the LTS branches.
> That's why I propose to replace with this sentence:
> "
> Major ABI versions are declared regularly and are then supported for
> at least one year, typically aligned with the :ref:`LTS release <stable_lts_releases>`.
> "

So look, this one was a decision of the technical board.
My position is still what was agreed was, "declared every year, and supported for one year".
I like it, it's crystal clear what is the policy, until we change the policy. 
 
That said, I can make the change no problem, but I need some others to chime in to ok it. 
Perhaps at the head of the Techboard today?

> 
>> +#. The ABI version is managed at a project level in DPDK, with the ABI version
>> +   reflected in all library's soname.
> 
> Yes, even the experimental libraries should have the same version,
> with the minor number incremented at each release.
> (just a comment to change a policy at the end of this patch)

It's described elsewhere in the document, experimental libraries have a major 
ABI version of 0, to indicate they exist outside of ABI management.
Minor number then changes as the experimental library changes as before.

> 
>> +   In 2019, the DPDK community stated its intention to move to ABI stable
>> +   releases, over a number of release cycles. This change begins with
>> +   maintaining ABI stability through one year of DPDK releases starting from
>> +   DPDK 19.11. This policy will be reviewed in 2020, with intention of
>> +   lengthening the stability period.
> 
> Great, the schedule is clear here.
> 
>> +A major ABI version is declared every year, aligned with that year's LTS
>> +release, e.g. v19.11. This ABI version is then supported for one year by all
>> +subsequent releases within that time period, until the next LTS release, e.g.
>> +v20.11.
> 
> Let's reword like this:
> "
> A new major ABI version can be declared when a new LTS branch is started,
> e.g. ABI 19 for DPDK 19.11.0.
> This major ABI version is then supported until the next one,
> e.g. ABI 20 for DPDK 20.11.0.
> All ABI changes must be detailed in the release notes.
> "

This is more ambiguous, although what I said above stands.
If there is general agreement with changing this part of the policy, I am ok to make 
the change.

> 
>> +At the declaration of a major ABI version, major version numbers encoded in
>> +libraries' sonames are bumped to indicate the new version, with the minor
>> +version reset to ``0``. An example would be ``librte_eal.so.20.3`` would become
>> +``librte_eal.so.21.0``.
>>  
>> +Minor versions are incremented to indicate the release of a new ABI compatible
>> +DPDK release, typically the DPDK quarterly releases. An example of this, might
>> +be that ``librte_eal.so.20.1`` would indicate the first ABI compatible DPDK
>> +release, following the declaration of the new major ABI version ``20``.
>>  
>> +An ABI version is supported in all new releases until the next major ABI version
>> +is declared.
> 
> This sentence is repetitive.

ACK

>> When changing the major ABI version, the release notes will detail
>> +all ABI changes.
> 
> I suggest to move and reword this sentence above (as in my above reword).

ACK

> 
>> +   ABI breakages due to changes such as reorganizing public
>> +   structure fields for aesthetic or readability purposes should be avoided.
> 
> Why it should be avoided?
> If the ABI is broken anyway, I don't see any reason to not break it more.

This is text from the original ABI Policy - I think the general sentiment still stands.
Just because you have an ABI Breakage window doesn't mean you should feel free to break
the ABI. The 3 ACKs required from Technical Board member to change the ABI, are another
reflection of this. 

As a general rule.
Unnecessary changes should still be avoided, to reduce ABI churn between ABI versions. 

>> +#. ABI breaking changes (including an alternative map file) can be included with
>> +   deprecation notice, in wrapped way by the ``RTE_NEXT_ABI`` option, to provide
>> +   more details about oncoming changes. ``RTE_NEXT_ABI`` wrapper will be removed
>> +   at the declaration of the next major ABI version.
> 
> I missed that in discussions.
> I thought we wanted to wait for the next major ABI.
> If we allow NEXT_ABI ifdef, we will have 2 DPDK versions
> (stable and next ABI) to test.

This is text from the original ABI Policy - the purpose remains the same.
If you add an ABI breaking change in say 20.02, that clearly can't see the light of day until 20.11

You may still opt to prepare the community for the change, by putting your code out wrapped 
in a NEXT_ABI and flagging it. You then get the option for people, so inclined, to build and try your code.

I can't see it being used often, but it is another tool in the box of managing ABI change.

> 
>> +Libraries marked as ``experimental`` are entirely not considered part of an ABI
>> +version, and may change without warning at any time. Experimental libraries
>> +always have a major version of ``0`` to indicate they exist outside of
>> +ABI Versioning, with the minor version incremented with each ABI change
>> +to library.
> 
> It means not all libraries will have the same ABI version.
> It is contrary of "ABI version is managed at a project level",
> and I don't see a real benefit of a different version number.

There is a benefit, major version 0 is a very clear indication that 
the library exists outside of ABI management. 
A library isn't in the ABI, until it is in the ABI - an then it gets
added to the major version number. 

> Anyway, some experimental functions can live inside a library
> with a stable ABI version number
True, but if an entire library is experimental - let's be crystal 
clear about that. 

  reply	other threads:[~2019-11-06  8:52 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-05 15:24 [dpdk-dev] [PATCH v8 0/4] " Ray Kinsella
2019-11-05 15:24 ` [dpdk-dev] [PATCH v8 1/4] doc: separate versioning.rst into version and policy Ray Kinsella
2019-11-05 17:37   ` Stephen Hemminger
2019-11-06 16:11   ` Mcnamara, John
2019-11-05 15:24 ` [dpdk-dev] [PATCH v8 2/4] doc: changes to abi policy introducing major abi versions Ray Kinsella
2019-11-05 17:37   ` Stephen Hemminger
2019-11-06  0:11   ` Thomas Monjalon
2019-11-06  8:49     ` Ray Kinsella [this message]
2019-11-06  9:06       ` Thomas Monjalon
2019-11-06  9:21         ` David Marchand
2019-11-06  9:22         ` Ray Kinsella
2019-11-06 21:07           ` Thomas Monjalon
2019-11-08 14:09             ` Ray Kinsella
2019-11-06 16:12   ` Mcnamara, John
2019-11-05 15:24 ` [dpdk-dev] [PATCH v8 3/4] doc: updates to versioning guide for " Ray Kinsella
2019-11-05 17:37   ` Stephen Hemminger
2019-11-06 16:13   ` Mcnamara, John
2019-11-05 15:24 ` [dpdk-dev] [PATCH v8 4/4] doc: add maintainer for abi policy Ray Kinsella
2019-11-06 16:13   ` Mcnamara, John

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dd31798b-a702-ecde-aafa-480c52a843d8@ashroe.eu \
    --to=mdr@ashroe.eu \
    --cc=aconole@redhat.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinj@marvell.com \
    --cc=john.mcnamara@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=ktraynor@redhat.com \
    --cc=marko.kovacevic@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=nhorman@tuxdriver.com \
    --cc=olivier.matz@6wind.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).