diff mbox series

build: fix soname info for 19.11 compatiblity

Message ID 20191211102642.983579-1-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Headers
Series build: fix soname info for 19.11 compatiblity |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/travis-robot warning Travis build: failed
ci/Intel-compilation fail apply issues

Commit Message

Bruce Richardson Dec. 11, 2019, 10:26 a.m. UTC 
  The soname for each stable ABI version should be just the ABI version major
number without the minor number. Unfortunately both major and minor were
used causing version 20.1 to be incompatible with 20.0.

This patch fixes the issue by switching from 2-part to 3-part ABI version
numbers so that we can keep 20.0 as soname and using the final digits to
identify the 20.x releases which are ABI compatible. This requires changes
to both make and meson builds to handle the three-digit version and shrink
it to 2-digit for soname.

Fixes: cba806e07d6f ("build: change ABI versioning to global")

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---

This patch contains an alternative fix to that implied by the previous patches:
http://patches.dpdk.org/patch/63726/
http://patches.dpdk.org/patch/63728/

---
 ABI_VERSION         | 2 +-
 drivers/meson.build | 4 ++--
 lib/meson.build     | 4 ++--
 mk/rte.lib.mk       | 5 ++++-
 4 files changed, 9 insertions(+), 6 deletions(-)

Comments

Luca Boccassi Dec. 11, 2019, 11:04 a.m. UTC | #1 
On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
> The soname for each stable ABI version should be just the ABI version
> major
> number without the minor number. Unfortunately both major and minor
> were
> used causing version 20.1 to be incompatible with 20.0.
> 
> This patch fixes the issue by switching from 2-part to 3-part ABI
> version
> numbers so that we can keep 20.0 as soname and using the final digits
> to
> identify the 20.x releases which are ABI compatible. This requires
> changes
> to both make and meson builds to handle the three-digit version and
> shrink
> it to 2-digit for soname.
> 
> Fixes: cba806e07d6f ("build: change ABI versioning to global")
> 
> Signed-off-by: Thomas Monjalon <
> thomas@monjalon.net
> >
> Signed-off-by: Bruce Richardson <
> bruce.richardson@intel.com
> >
> ---
> 
> This patch contains an alternative fix to that implied by the
> previous patches:
> http://patches.dpdk.org/patch/63726/
> 
> http://patches.dpdk.org/patch/63728/
> 
> 
> ---
>  ABI_VERSION         | 2 +-
>  drivers/meson.build | 4 ++--
>  lib/meson.build     | 4 ++--
>  mk/rte.lib.mk       | 5 ++++-
>  4 files changed, 9 insertions(+), 6 deletions(-)

Acked-by: Luca Boccassi <bluca@debian.org>

Thank you! I've set a reminder in my calendar for September to revert
it :-)
Thomas Monjalon Dec. 11, 2019, 11:08 a.m. UTC | #2 
11/12/2019 12:04, Luca Boccassi:
> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
> > The soname for each stable ABI version should be just the ABI version
> > major
> > number without the minor number. Unfortunately both major and minor
> > were
> > used causing version 20.1 to be incompatible with 20.0.
> > 
> > This patch fixes the issue by switching from 2-part to 3-part ABI
> > version
> > numbers so that we can keep 20.0 as soname and using the final digits
> > to
> > identify the 20.x releases which are ABI compatible. This requires
> > changes
> > to both make and meson builds to handle the three-digit version and
> > shrink
> > it to 2-digit for soname.
> > 
> > Fixes: cba806e07d6f ("build: change ABI versioning to global")
> > 
> > Signed-off-by: Thomas Monjalon <
> > thomas@monjalon.net
> > >
> > Signed-off-by: Bruce Richardson <
> > bruce.richardson@intel.com
> > >
> > ---
> > 
> > This patch contains an alternative fix to that implied by the
> > previous patches:
> > http://patches.dpdk.org/patch/63726/
> > 
> > http://patches.dpdk.org/patch/63728/
> > 
> > 
> > ---
> >  ABI_VERSION         | 2 +-
> >  drivers/meson.build | 4 ++--
> >  lib/meson.build     | 4 ++--
> >  mk/rte.lib.mk       | 5 ++++-
> >  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> Acked-by: Luca Boccassi <bluca@debian.org>
> 
> Thank you! I've set a reminder in my calendar for September to revert
> it :-)

I don't think we need to revert it.
The ABI version will have only 2 numbers (21.0).
In makefile there is no change.
What needs to be changed in meson?
Bruce Richardson Dec. 11, 2019, 11:11 a.m. UTC | #3 
On Wed, Dec 11, 2019 at 11:04:01AM +0000, Luca Boccassi wrote:
> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
> > The soname for each stable ABI version should be just the ABI version
> > major
> > number without the minor number. Unfortunately both major and minor
> > were
> > used causing version 20.1 to be incompatible with 20.0.
> > 
> > This patch fixes the issue by switching from 2-part to 3-part ABI
> > version
> > numbers so that we can keep 20.0 as soname and using the final digits
> > to
> > identify the 20.x releases which are ABI compatible. This requires
> > changes
> > to both make and meson builds to handle the three-digit version and
> > shrink
> > it to 2-digit for soname.
> > 
> > Fixes: cba806e07d6f ("build: change ABI versioning to global")
> > 
> > Signed-off-by: Thomas Monjalon <
> > thomas@monjalon.net
> > >
> > Signed-off-by: Bruce Richardson <
> > bruce.richardson@intel.com
> > >
> > ---
> > 
> > This patch contains an alternative fix to that implied by the
> > previous patches:
> > http://patches.dpdk.org/patch/63726/
> > 
> > http://patches.dpdk.org/patch/63728/
> > 
> > 
> > ---
> >  ABI_VERSION         | 2 +-
> >  drivers/meson.build | 4 ++--
> >  lib/meson.build     | 4 ++--
> >  mk/rte.lib.mk       | 5 ++++-
> >  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> Acked-by: Luca Boccassi <bluca@debian.org>
> 
> Thank you! I've set a reminder in my calendar for September to revert
> it :-)
> 
Lol, don't forget to put another reminder to fix things properly then too.
:-)

We also still need consensus in the community as to whether to take this
approach or to do a re-spin of 19.11. At this point, I'm swayed by your
arguments and think we should keep compatibility at the cost of a little
pain and weirdness in our .so filenames.

/Bruce
Bruce Richardson Dec. 11, 2019, 11:14 a.m. UTC | #4 
On Wed, Dec 11, 2019 at 12:08:25PM +0100, Thomas Monjalon wrote:
> 11/12/2019 12:04, Luca Boccassi:
> > On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
> > > The soname for each stable ABI version should be just the ABI version
> > > major
> > > number without the minor number. Unfortunately both major and minor
> > > were
> > > used causing version 20.1 to be incompatible with 20.0.
> > > 
> > > This patch fixes the issue by switching from 2-part to 3-part ABI
> > > version
> > > numbers so that we can keep 20.0 as soname and using the final digits
> > > to
> > > identify the 20.x releases which are ABI compatible. This requires
> > > changes
> > > to both make and meson builds to handle the three-digit version and
> > > shrink
> > > it to 2-digit for soname.
> > > 
> > > Fixes: cba806e07d6f ("build: change ABI versioning to global")
> > > 
> > > Signed-off-by: Thomas Monjalon <
> > > thomas@monjalon.net
> > > >
> > > Signed-off-by: Bruce Richardson <
> > > bruce.richardson@intel.com
> > > >
> > > ---
> > > 
> > > This patch contains an alternative fix to that implied by the
> > > previous patches:
> > > http://patches.dpdk.org/patch/63726/
> > > 
> > > http://patches.dpdk.org/patch/63728/
> > > 
> > > 
> > > ---
> > >  ABI_VERSION         | 2 +-
> > >  drivers/meson.build | 4 ++--
> > >  lib/meson.build     | 4 ++--
> > >  mk/rte.lib.mk       | 5 ++++-
> > >  4 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > Acked-by: Luca Boccassi <bluca@debian.org>
> > 
> > Thank you! I've set a reminder in my calendar for September to revert
> > it :-)
> 
> I don't think we need to revert it.
> The ABI version will have only 2 numbers (21.0).
> In makefile there is no change.
> What needs to be changed in meson?
> 
We need to remove the explicit use of so_version, and let meson just do the
right thing with the lib version. That being said, I can see about having
meson behave as make so that it will work even if we forget (i.e. even with
changed behaviour we should still remove the explicit soversion usage when
it's no longer needed).

/Bruce
Ferruh Yigit Dec. 11, 2019, 11:15 a.m. UTC | #5 
On 12/11/2019 10:26 AM, Bruce Richardson wrote:
> The soname for each stable ABI version should be just the ABI version major
> number without the minor number. Unfortunately both major and minor were
> used causing version 20.1 to be incompatible with 20.0.
> 
> This patch fixes the issue by switching from 2-part to 3-part ABI version
> numbers so that we can keep 20.0 as soname and using the final digits to
> identify the 20.x releases which are ABI compatible. This requires changes
> to both make and meson builds to handle the three-digit version and shrink
> it to 2-digit for soname.

What about following, does it makes file names better?

DPDK_20.02 (ABI_20.1):
 SONAME: .so.20.0
 library name: .so.20.1
 .so.20.0 --> .so.20.1
 .so --> .so.20.1


DPDK_20.05 (ABI_20.2):
 SONAME: .so.20.0
 library name: .so.20.2
 .so.20.0 --> .so.20.2
 .so --> .so.20.2


> 
> Fixes: cba806e07d6f ("build: change ABI versioning to global")
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> 
> This patch contains an alternative fix to that implied by the previous patches:
> http://patches.dpdk.org/patch/63726/
> http://patches.dpdk.org/patch/63728/
> 
> ---
>  ABI_VERSION         | 2 +-
>  drivers/meson.build | 4 ++--
>  lib/meson.build     | 4 ++--
>  mk/rte.lib.mk       | 5 ++++-
>  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/ABI_VERSION b/ABI_VERSION
> index 2e73f8d2a..fcc01369a 100644
> --- a/ABI_VERSION
> +++ b/ABI_VERSION
> @@ -1 +1 @@
> -20.1
> +20.0.1
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 72eec4608..5f1d72071 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -132,11 +132,11 @@ foreach class:dpdk_driver_classes
>  
>  			if is_experimental != 0
>  				lib_version = experimental_abi_version
> -				so_version = experimental_abi_version
>  			else
>  				lib_version = abi_version
> -				so_version = abi_version
>  			endif
> +			so_version = lib_version.split('.')
> +			so_version = so_version[0] + '.' + so_version[1]
>  
>  			# now build the static driver
>  			static_lib = static_library(lib_name,
> diff --git a/lib/meson.build b/lib/meson.build
> index 6ceb5e756..3b7dad348 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -113,11 +113,11 @@ foreach l:libraries
>  
>  			if is_experimental != 0
>  				lib_version = experimental_abi_version
> -				so_version = experimental_abi_version
>  			else
>  				lib_version = abi_version
> -				so_version = abi_version
>  			endif
> +			so_version = lib_version.split('.')
> +			so_version = so_version[0] + '.' + so_version[1]
>  
>  			# first build static lib
>  			static_lib = static_library(libname,
> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> index 655a1b143..4b4267021 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -19,6 +19,8 @@ LIBABIVER := 0.$(shell cat $(RTE_SRCDIR)/ABI_VERSION | tr -d '.')
>  endif
>  
>  ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
> +SOVER := $(basename $(LIBABIVER))
> +SONAME := $(patsubst %.a,%.so.$(SOVER),$(LIB))
>  LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))
>  ifeq ($(EXTLIB_BUILD),n)
>  CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
> @@ -74,7 +76,7 @@ NO_UNDEFINED := -z defs
>  endif
>  
>  O_TO_S = $(LD) -L$(RTE_SDK_BIN)/lib $(_CPU_LDFLAGS) $(EXTRA_LDFLAGS) \
> -	  -shared $(OBJS-y) $(NO_UNDEFINED) $(LDLIBS) -Wl,-soname,$(LIB) -o $(LIB)
> +	  -shared $(OBJS-y) $(NO_UNDEFINED) $(LDLIBS) -Wl,-soname,$(SONAME) -o $(LIB)
>  O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
>  O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
>  O_TO_S_DO = @set -e; \
> @@ -133,6 +135,7 @@ $(RTE_OUTPUT)/lib/$(LIB): $(LIB)
>  	$(Q)cp -f $(LIB) $(RTE_OUTPUT)/lib
>  ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
>  	$(Q)ln -s -f $< $(shell echo $@ | sed 's/\.so.*/.so/')
> +	$(Q)ln -s -f $< $(shell echo $@ | sed 's/\.so.*/.so.$(SOVER)/')
>  endif
>  
>  #
>
Ferruh Yigit Dec. 11, 2019, 11:19 a.m. UTC | #6 
On 12/11/2019 11:14 AM, Bruce Richardson wrote:
> On Wed, Dec 11, 2019 at 12:08:25PM +0100, Thomas Monjalon wrote:
>> 11/12/2019 12:04, Luca Boccassi:
>>> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
>>>> The soname for each stable ABI version should be just the ABI version
>>>> major
>>>> number without the minor number. Unfortunately both major and minor
>>>> were
>>>> used causing version 20.1 to be incompatible with 20.0.
>>>>
>>>> This patch fixes the issue by switching from 2-part to 3-part ABI
>>>> version
>>>> numbers so that we can keep 20.0 as soname and using the final digits
>>>> to
>>>> identify the 20.x releases which are ABI compatible. This requires
>>>> changes
>>>> to both make and meson builds to handle the three-digit version and
>>>> shrink
>>>> it to 2-digit for soname.
>>>>
>>>> Fixes: cba806e07d6f ("build: change ABI versioning to global")
>>>>
>>>> Signed-off-by: Thomas Monjalon <
>>>> thomas@monjalon.net
>>>>>
>>>> Signed-off-by: Bruce Richardson <
>>>> bruce.richardson@intel.com
>>>>>
>>>> ---
>>>>
>>>> This patch contains an alternative fix to that implied by the
>>>> previous patches:
>>>> http://patches.dpdk.org/patch/63726/
>>>>
>>>> http://patches.dpdk.org/patch/63728/
>>>>
>>>>
>>>> ---
>>>>  ABI_VERSION         | 2 +-
>>>>  drivers/meson.build | 4 ++--
>>>>  lib/meson.build     | 4 ++--
>>>>  mk/rte.lib.mk       | 5 ++++-
>>>>  4 files changed, 9 insertions(+), 6 deletions(-)
>>>
>>> Acked-by: Luca Boccassi <bluca@debian.org>
>>>
>>> Thank you! I've set a reminder in my calendar for September to revert
>>> it :-)
>>
>> I don't think we need to revert it.
>> The ABI version will have only 2 numbers (21.0).
>> In makefile there is no change.
>> What needs to be changed in meson?
>>
> We need to remove the explicit use of so_version, and let meson just do the
> right thing with the lib version. That being said, I can see about having
> meson behave as make so that it will work even if we forget (i.e. even with
> changed behaviour we should still remove the explicit soversion usage when
> it's no longer needed).
> 

Current major become two digits, "20.0", I think we fix it to single digit,
"21", again in 20.11.

I suggest adding this to deprecation notice document, with 20.11 target, so it
can stay there and remind us the change.
Ferruh Yigit Dec. 11, 2019, 11:25 a.m. UTC | #7 
On 12/11/2019 11:11 AM, Bruce Richardson wrote:
> On Wed, Dec 11, 2019 at 11:04:01AM +0000, Luca Boccassi wrote:
>> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
>>> The soname for each stable ABI version should be just the ABI version
>>> major
>>> number without the minor number. Unfortunately both major and minor
>>> were
>>> used causing version 20.1 to be incompatible with 20.0.
>>>
>>> This patch fixes the issue by switching from 2-part to 3-part ABI
>>> version
>>> numbers so that we can keep 20.0 as soname and using the final digits
>>> to
>>> identify the 20.x releases which are ABI compatible. This requires
>>> changes
>>> to both make and meson builds to handle the three-digit version and
>>> shrink
>>> it to 2-digit for soname.
>>>
>>> Fixes: cba806e07d6f ("build: change ABI versioning to global")
>>>
>>> Signed-off-by: Thomas Monjalon <
>>> thomas@monjalon.net
>>>>
>>> Signed-off-by: Bruce Richardson <
>>> bruce.richardson@intel.com
>>>>
>>> ---
>>>
>>> This patch contains an alternative fix to that implied by the
>>> previous patches:
>>> http://patches.dpdk.org/patch/63726/
>>>
>>> http://patches.dpdk.org/patch/63728/
>>>
>>>
>>> ---
>>>  ABI_VERSION         | 2 +-
>>>  drivers/meson.build | 4 ++--
>>>  lib/meson.build     | 4 ++--
>>>  mk/rte.lib.mk       | 5 ++++-
>>>  4 files changed, 9 insertions(+), 6 deletions(-)
>>
>> Acked-by: Luca Boccassi <bluca@debian.org>
>>
>> Thank you! I've set a reminder in my calendar for September to revert
>> it :-)
>>
> Lol, don't forget to put another reminder to fix things properly then too.
> :-)
> 
> We also still need consensus in the community as to whether to take this
> approach or to do a re-spin of 19.11. At this point, I'm swayed by your
> arguments and think we should keep compatibility at the cost of a little
> pain and weirdness in our .so filenames.
> 

I was leaning towards 19.11.1 but Luca was convincing :) and I agree he has a
point, so no objection to this change from me.
Bruce Richardson Dec. 11, 2019, 1:14 p.m. UTC | #8 
On Wed, Dec 11, 2019 at 11:15:29AM +0000, Ferruh Yigit wrote:
> On 12/11/2019 10:26 AM, Bruce Richardson wrote:
> > The soname for each stable ABI version should be just the ABI version major
> > number without the minor number. Unfortunately both major and minor were
> > used causing version 20.1 to be incompatible with 20.0.
> > 
> > This patch fixes the issue by switching from 2-part to 3-part ABI version
> > numbers so that we can keep 20.0 as soname and using the final digits to
> > identify the 20.x releases which are ABI compatible. This requires changes
> > to both make and meson builds to handle the three-digit version and shrink
> > it to 2-digit for soname.
> 
> What about following, does it makes file names better?
> 
> DPDK_20.02 (ABI_20.1):
>  SONAME: .so.20.0
>  library name: .so.20.1
>  .so.20.0 --> .so.20.1
>  .so --> .so.20.1
> 
> 
> DPDK_20.05 (ABI_20.2):
>  SONAME: .so.20.0
>  library name: .so.20.2
>  .so.20.0 --> .so.20.2
>  .so --> .so.20.2
> 
> 
Personally, I really don't like having symlinks for libraries with the same
"level" of version number, i.e. while linking from 20.0 -> 20.0.1 is fine,
linking from 20.1 to 20.0 seems wrong, and would also potentially cause
issues with having 19.11 installed in parallel to another version.

/Bruce
Ray Kinsella Dec. 12, 2019, 11:07 a.m. UTC | #9 
On 11/12/2019 11:19, Ferruh Yigit wrote:
> On 12/11/2019 11:14 AM, Bruce Richardson wrote:
>> On Wed, Dec 11, 2019 at 12:08:25PM +0100, Thomas Monjalon wrote:
>>> 11/12/2019 12:04, Luca Boccassi:
>>>> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
>>>>> The soname for each stable ABI version should be just the ABI version
>>>>> major
>>>>> number without the minor number. Unfortunately both major and minor
>>>>> were
>>>>> used causing version 20.1 to be incompatible with 20.0.
>>>>>
>>>>> This patch fixes the issue by switching from 2-part to 3-part ABI
>>>>> version
>>>>> numbers so that we can keep 20.0 as soname and using the final digits
>>>>> to
>>>>> identify the 20.x releases which are ABI compatible. This requires
>>>>> changes
>>>>> to both make and meson builds to handle the three-digit version and
>>>>> shrink
>>>>> it to 2-digit for soname.
>>>>>
>>>>> Fixes: cba806e07d6f ("build: change ABI versioning to global")
>>>>>
>>>>> Signed-off-by: Thomas Monjalon <
>>>>> thomas@monjalon.net
>>>>>>
>>>>> Signed-off-by: Bruce Richardson <
>>>>> bruce.richardson@intel.com
>>>>>>
>>>>> ---
>>>>>
>>>>> This patch contains an alternative fix to that implied by the
>>>>> previous patches:
>>>>> http://patches.dpdk.org/patch/63726/
>>>>>
>>>>> http://patches.dpdk.org/patch/63728/
>>>>>
>>>>>
>>>>> ---
>>>>>  ABI_VERSION         | 2 +-
>>>>>  drivers/meson.build | 4 ++--
>>>>>  lib/meson.build     | 4 ++--
>>>>>  mk/rte.lib.mk       | 5 ++++-
>>>>>  4 files changed, 9 insertions(+), 6 deletions(-)
>>>>
>>>> Acked-by: Luca Boccassi <bluca@debian.org>
>>>>
>>>> Thank you! I've set a reminder in my calendar for September to revert
>>>> it :-)
>>>
>>> I don't think we need to revert it.
>>> The ABI version will have only 2 numbers (21.0).
>>> In makefile there is no change.
>>> What needs to be changed in meson?
>>>
>> We need to remove the explicit use of so_version, and let meson just do the
>> right thing with the lib version. That being said, I can see about having
>> meson behave as make so that it will work even if we forget (i.e. even with
>> changed behaviour we should still remove the explicit soversion usage when
>> it's no longer needed).
>>
> 
> Current major become two digits, "20.0", I think we fix it to single digit,
> "21", again in 20.11.
> 
> I suggest adding this to deprecation notice document, with 20.11 target, so it
> can stay there and remind us the change.
> 
+1, we do.

So name, should be comprised of the library name and the major version only.

Ray K
Ray Kinsella Dec. 12, 2019, 11:14 a.m. UTC | #10 
On 11/12/2019 11:11, Bruce Richardson wrote:
> On Wed, Dec 11, 2019 at 11:04:01AM +0000, Luca Boccassi wrote:
>> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
>>> The soname for each stable ABI version should be just the ABI version
>>> major
>>> number without the minor number. Unfortunately both major and minor
>>> were
>>> used causing version 20.1 to be incompatible with 20.0.
>>>
>>> This patch fixes the issue by switching from 2-part to 3-part ABI
>>> version
>>> numbers so that we can keep 20.0 as soname and using the final digits
>>> to
>>> identify the 20.x releases which are ABI compatible. This requires
>>> changes
>>> to both make and meson builds to handle the three-digit version and
>>> shrink
>>> it to 2-digit for soname.
>>>
>>> Fixes: cba806e07d6f ("build: change ABI versioning to global")
>>>
>>> Signed-off-by: Thomas Monjalon <
>>> thomas@monjalon.net
>>>>
>>> Signed-off-by: Bruce Richardson <
>>> bruce.richardson@intel.com
>>>>
>>> ---
>>>
>>> This patch contains an alternative fix to that implied by the
>>> previous patches:
>>> http://patches.dpdk.org/patch/63726/
>>>
>>> http://patches.dpdk.org/patch/63728/
>>>
>>>
>>> ---
>>>  ABI_VERSION         | 2 +-
>>>  drivers/meson.build | 4 ++--
>>>  lib/meson.build     | 4 ++--
>>>  mk/rte.lib.mk       | 5 ++++-
>>>  4 files changed, 9 insertions(+), 6 deletions(-)
>>
>> Acked-by: Luca Boccassi <bluca@debian.org>
>>
>> Thank you! I've set a reminder in my calendar for September to revert
>> it :-)
>>
> Lol, don't forget to put another reminder to fix things properly then too.
> :-)
> 
> We also still need consensus in the community as to whether to take this
> approach or to do a re-spin of 19.11. At this point, I'm swayed by your
> arguments and think we should keep compatibility at the cost of a little
> pain and weirdness in our .so filenames.
> 
> /Bruce
>

My vote would be for a respin.
We don't yet know what challenges the weirdness or pain will be.
Why we would bother for the sake of a respin?

Ray K
Luca Boccassi Dec. 12, 2019, 1:58 p.m. UTC | #11 
On Thu, 2019-12-12 at 11:14 +0000, Ray Kinsella wrote:
> 
> On 11/12/2019 11:11, Bruce Richardson wrote:
> > On Wed, Dec 11, 2019 at 11:04:01AM +0000, Luca Boccassi wrote:
> > > On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
> > > > The soname for each stable ABI version should be just the ABI
> > > > version
> > > > major
> > > > number without the minor number. Unfortunately both major and
> > > > minor
> > > > were
> > > > used causing version 20.1 to be incompatible with 20.0.
> > > > 
> > > > This patch fixes the issue by switching from 2-part to 3-part
> > > > ABI
> > > > version
> > > > numbers so that we can keep 20.0 as soname and using the final
> > > > digits
> > > > to
> > > > identify the 20.x releases which are ABI compatible. This
> > > > requires
> > > > changes
> > > > to both make and meson builds to handle the three-digit version
> > > > and
> > > > shrink
> > > > it to 2-digit for soname.
> > > > 
> > > > Fixes: cba806e07d6f ("build: change ABI versioning to global")
> > > > 
> > > > Signed-off-by: Thomas Monjalon <
> > > > thomas@monjalon.net
> > > > 
> > > > 
> > > > Signed-off-by: Bruce Richardson <
> > > > bruce.richardson@intel.com
> > > > 
> > > > 
> > > > ---
> > > > 
> > > > This patch contains an alternative fix to that implied by the
> > > > previous patches:
> > > > http://patches.dpdk.org/patch/63726/
> > > > 
> > > > 
> > > > http://patches.dpdk.org/patch/63728/
> > > > 
> > > > 
> > > > 
> > > > ---
> > > >  ABI_VERSION         | 2 +-
> > > >  drivers/meson.build | 4 ++--
> > > >  lib/meson.build     | 4 ++--
> > > >  mk/rte.lib.mk       | 5 ++++-
> > > >  4 files changed, 9 insertions(+), 6 deletions(-)
> > > 
> > > Acked-by: Luca Boccassi <
> > > bluca@debian.org
> > > >
> > > 
> > > Thank you! I've set a reminder in my calendar for September to
> > > revert
> > > it :-)
> > > 
> > 
> > Lol, don't forget to put another reminder to fix things properly
> > then too.
> > :-)
> > 
> > We also still need consensus in the community as to whether to take
> > this
> > approach or to do a re-spin of 19.11. At this point, I'm swayed by
> > your
> > arguments and think we should keep compatibility at the cost of a
> > little
> > pain and weirdness in our .so filenames.
> > 
> > /Bruce
> > 
> 
> My vote would be for a respin.
> We don't yet know what challenges the weirdness or pain will be.
> Why we would bother for the sake of a respin?
> 
> Ray K

We already uploaded 19.11 to Debian last week, which means the tarball
is in the archive and it's hashsummed and signed:

http://deb.debian.org/debian/pool/main/d/dpdk/dpdk_19.11.orig.tar.xz

(it's in experimental, but the archive is the same)

A respin at this point would make my life not impossible, but quite
difficult.

IMHO respins are acceptable within a few hours - two weeks later it's
no longer a respin, it's a new version :-)
Ray Kinsella Dec. 12, 2019, 2:05 p.m. UTC | #12 
On 12/12/2019 13:58, Luca Boccassi wrote:
> On Thu, 2019-12-12 at 11:14 +0000, Ray Kinsella wrote:
>>
>> On 11/12/2019 11:11, Bruce Richardson wrote:
>>> On Wed, Dec 11, 2019 at 11:04:01AM +0000, Luca Boccassi wrote:
>>>> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
>>>>> The soname for each stable ABI version should be just the ABI
>>>>> version
>>>>> major
>>>>> number without the minor number. Unfortunately both major and
>>>>> minor
>>>>> were
>>>>> used causing version 20.1 to be incompatible with 20.0.
>>>>>
>>>>> This patch fixes the issue by switching from 2-part to 3-part
>>>>> ABI
>>>>> version
>>>>> numbers so that we can keep 20.0 as soname and using the final
>>>>> digits
>>>>> to
>>>>> identify the 20.x releases which are ABI compatible. This
>>>>> requires
>>>>> changes
>>>>> to both make and meson builds to handle the three-digit version
>>>>> and
>>>>> shrink
>>>>> it to 2-digit for soname.
>>>>>
>>>>> Fixes: cba806e07d6f ("build: change ABI versioning to global")
>>>>>
>>>>> Signed-off-by: Thomas Monjalon <
>>>>> thomas@monjalon.net
>>>>>
>>>>>
>>>>> Signed-off-by: Bruce Richardson <
>>>>> bruce.richardson@intel.com
>>>>>
>>>>>
>>>>> ---
>>>>>
>>>>> This patch contains an alternative fix to that implied by the
>>>>> previous patches:
>>>>> http://patches.dpdk.org/patch/63726/
>>>>>
>>>>>
>>>>> http://patches.dpdk.org/patch/63728/
>>>>>
>>>>>
>>>>>
>>>>> ---
>>>>>  ABI_VERSION         | 2 +-
>>>>>  drivers/meson.build | 4 ++--
>>>>>  lib/meson.build     | 4 ++--
>>>>>  mk/rte.lib.mk       | 5 ++++-
>>>>>  4 files changed, 9 insertions(+), 6 deletions(-)
>>>>
>>>> Acked-by: Luca Boccassi <
>>>> bluca@debian.org
>>>>>
>>>>
>>>> Thank you! I've set a reminder in my calendar for September to
>>>> revert
>>>> it :-)
>>>>
>>>
>>> Lol, don't forget to put another reminder to fix things properly
>>> then too.
>>> :-)
>>>
>>> We also still need consensus in the community as to whether to take
>>> this
>>> approach or to do a re-spin of 19.11. At this point, I'm swayed by
>>> your
>>> arguments and think we should keep compatibility at the cost of a
>>> little
>>> pain and weirdness in our .so filenames.
>>>
>>> /Bruce
>>>
>>
>> My vote would be for a respin.
>> We don't yet know what challenges the weirdness or pain will be.
>> Why we would bother for the sake of a respin?
>>
>> Ray K
> 
> We already uploaded 19.11 to Debian last week, which means the tarball
> is in the archive and it's hashsummed and signed:
> 
> http://deb.debian.org/debian/pool/main/d/dpdk/dpdk_19.11.orig.tar.xz
> 
> (it's in experimental, but the archive is the same)
> 
> A respin at this point would make my life not impossible, but quite
> difficult.
> 
> IMHO respins are acceptable within a few hours - two weeks later it's
> no longer a respin, it's a new version :-)
> 

Understood, we are stretching the acceptable terms of a re-spin.

If the version that is in the archive fundamentally broken, what are you going to do.
This is not a relatively easy circumstance that we can simply fix it with an apt-get update.

Is there precedent for pulling and re-releasing something that is broken in this way?

Ray K
Bruce Richardson Dec. 12, 2019, 3:08 p.m. UTC | #13 
On Thu, Dec 12, 2019 at 02:05:44PM +0000, Ray Kinsella wrote:
> 
> 
> On 12/12/2019 13:58, Luca Boccassi wrote:
> > On Thu, 2019-12-12 at 11:14 +0000, Ray Kinsella wrote:
> >>
> >> On 11/12/2019 11:11, Bruce Richardson wrote:
> >>> On Wed, Dec 11, 2019 at 11:04:01AM +0000, Luca Boccassi wrote:
> >>>> On Wed, 2019-12-11 at 10:26 +0000, Bruce Richardson wrote:
> >>>>> The soname for each stable ABI version should be just the ABI
> >>>>> version
> >>>>> major
> >>>>> number without the minor number. Unfortunately both major and
> >>>>> minor
> >>>>> were
> >>>>> used causing version 20.1 to be incompatible with 20.0.
> >>>>>
> >>>>> This patch fixes the issue by switching from 2-part to 3-part
> >>>>> ABI
> >>>>> version
> >>>>> numbers so that we can keep 20.0 as soname and using the final
> >>>>> digits
> >>>>> to
> >>>>> identify the 20.x releases which are ABI compatible. This
> >>>>> requires
> >>>>> changes
> >>>>> to both make and meson builds to handle the three-digit version
> >>>>> and
> >>>>> shrink
> >>>>> it to 2-digit for soname.
> >>>>>
> >>>>> Fixes: cba806e07d6f ("build: change ABI versioning to global")
> >>>>>
> >>>>> Signed-off-by: Thomas Monjalon <
> >>>>> thomas@monjalon.net
> >>>>>
> >>>>>
> >>>>> Signed-off-by: Bruce Richardson <
> >>>>> bruce.richardson@intel.com
> >>>>>
> >>>>>
> >>>>> ---
> >>>>>
> >>>>> This patch contains an alternative fix to that implied by the
> >>>>> previous patches:
> >>>>> http://patches.dpdk.org/patch/63726/
> >>>>>
> >>>>>
> >>>>> http://patches.dpdk.org/patch/63728/
> >>>>>
> >>>>>
> >>>>>
> >>>>> ---
> >>>>>  ABI_VERSION         | 2 +-
> >>>>>  drivers/meson.build | 4 ++--
> >>>>>  lib/meson.build     | 4 ++--
> >>>>>  mk/rte.lib.mk       | 5 ++++-
> >>>>>  4 files changed, 9 insertions(+), 6 deletions(-)
> >>>>
> >>>> Acked-by: Luca Boccassi <
> >>>> bluca@debian.org
> >>>>>
> >>>>
> >>>> Thank you! I've set a reminder in my calendar for September to
> >>>> revert
> >>>> it :-)
> >>>>
> >>>
> >>> Lol, don't forget to put another reminder to fix things properly
> >>> then too.
> >>> :-)
> >>>
> >>> We also still need consensus in the community as to whether to take
> >>> this
> >>> approach or to do a re-spin of 19.11. At this point, I'm swayed by
> >>> your
> >>> arguments and think we should keep compatibility at the cost of a
> >>> little
> >>> pain and weirdness in our .so filenames.
> >>>
> >>> /Bruce
> >>>
> >>
> >> My vote would be for a respin.
> >> We don't yet know what challenges the weirdness or pain will be.
> >> Why we would bother for the sake of a respin?
> >>
> >> Ray K
> > 
> > We already uploaded 19.11 to Debian last week, which means the tarball
> > is in the archive and it's hashsummed and signed:
> > 
> > http://deb.debian.org/debian/pool/main/d/dpdk/dpdk_19.11.orig.tar.xz
> > 
> > (it's in experimental, but the archive is the same)
> > 
> > A respin at this point would make my life not impossible, but quite
> > difficult.
> > 
> > IMHO respins are acceptable within a few hours - two weeks later it's
> > no longer a respin, it's a new version :-)
> > 
> 
> Understood, we are stretching the acceptable terms of a re-spin.
> 
> If the version that is in the archive fundamentally broken, what are you going to do.
> This is not a relatively easy circumstance that we can simply fix it with an apt-get update.
> 
> Is there precedent for pulling and re-releasing something that is broken in this way?
> 
The thing is that our existing package is not fundamentally broken, it just
has a wrong ABI version, which we can work around with a non-massive
amount of work. Given we have a fix that avoids any respinning, I see no
reason not just to go with it, and keep our ABI compatibility promise.

And I, too, have already uploaded a new build recipe, including package
checksums, to the FreeBSD ports collection. Respinning would be awkward
there too.

/Bruce
diff mbox series

Patch

diff --git a/ABI_VERSION b/ABI_VERSION
index 2e73f8d2a..fcc01369a 100644
--- a/ABI_VERSION
+++ b/ABI_VERSION
@@ -1 +1 @@ 
-20.1
+20.0.1
diff --git a/drivers/meson.build b/drivers/meson.build
index 72eec4608..5f1d72071 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -132,11 +132,11 @@  foreach class:dpdk_driver_classes
 
 			if is_experimental != 0
 				lib_version = experimental_abi_version
-				so_version = experimental_abi_version
 			else
 				lib_version = abi_version
-				so_version = abi_version
 			endif
+			so_version = lib_version.split('.')
+			so_version = so_version[0] + '.' + so_version[1]
 
 			# now build the static driver
 			static_lib = static_library(lib_name,
diff --git a/lib/meson.build b/lib/meson.build
index 6ceb5e756..3b7dad348 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -113,11 +113,11 @@  foreach l:libraries
 
 			if is_experimental != 0
 				lib_version = experimental_abi_version
-				so_version = experimental_abi_version
 			else
 				lib_version = abi_version
-				so_version = abi_version
 			endif
+			so_version = lib_version.split('.')
+			so_version = so_version[0] + '.' + so_version[1]
 
 			# first build static lib
 			static_lib = static_library(libname,
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index 655a1b143..4b4267021 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -19,6 +19,8 @@  LIBABIVER := 0.$(shell cat $(RTE_SRCDIR)/ABI_VERSION | tr -d '.')
 endif
 
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
+SOVER := $(basename $(LIBABIVER))
+SONAME := $(patsubst %.a,%.so.$(SOVER),$(LIB))
 LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))
 ifeq ($(EXTLIB_BUILD),n)
 CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
@@ -74,7 +76,7 @@  NO_UNDEFINED := -z defs
 endif
 
 O_TO_S = $(LD) -L$(RTE_SDK_BIN)/lib $(_CPU_LDFLAGS) $(EXTRA_LDFLAGS) \
-	  -shared $(OBJS-y) $(NO_UNDEFINED) $(LDLIBS) -Wl,-soname,$(LIB) -o $(LIB)
+	  -shared $(OBJS-y) $(NO_UNDEFINED) $(LDLIBS) -Wl,-soname,$(SONAME) -o $(LIB)
 O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
 O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
 O_TO_S_DO = @set -e; \
@@ -133,6 +135,7 @@  $(RTE_OUTPUT)/lib/$(LIB): $(LIB)
 	$(Q)cp -f $(LIB) $(RTE_OUTPUT)/lib
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 	$(Q)ln -s -f $< $(shell echo $@ | sed 's/\.so.*/.so/')
+	$(Q)ln -s -f $< $(shell echo $@ | sed 's/\.so.*/.so.$(SOVER)/')
 endif
 
 #