[v1,1/3] devtools: create internal ignore file for libabigail
Checks
Commit Message
Create file libabigail.internalignore where it is specified to ignore
all symbols in library file librte_pmd_mlx.* during an ABI check.
Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
devtools/check-abi.sh | 4 +++-
devtools/libabigail.internalignore | 4 ++++
2 files changed, 7 insertions(+), 1 deletion(-)
create mode 100644 devtools/libabigail.internalignore
Comments
From: Ophir Munk
> Create file libabigail.internalignore where it is specified to ignore all symbols
> in library file librte_pmd_mlx.* during an ABI check.
>
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
Don't forget to Cc: Ray.
On Tue, May 5, 2020 at 1:21 PM Ophir Munk <ophirmu@mellanox.com> wrote:
>
> Create file libabigail.internalignore where it is specified to ignore
> all symbols in library file librte_pmd_mlx.* during an ABI check.
I don't like the separate file, having rules with comments in a single
place is easier to maintain.
Why waive all changes in all mlx drivers?
There is a single issue with 20.02, see below.
Besides, looking at next patches, why keep experimental symbols?
If all symbols in common/mlx5 are for internal use, mark them _all_ as such.
>
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
> devtools/check-abi.sh | 4 +++-
> devtools/libabigail.internalignore | 4 ++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
> create mode 100644 devtools/libabigail.internalignore
>
> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
> index dd9120e..3bad19a 100755
> --- a/devtools/check-abi.sh
> +++ b/devtools/check-abi.sh
> @@ -10,7 +10,9 @@ fi
> refdir=$1
> newdir=$2
> warnonly=${3:-}
> -ABIDIFF_OPTIONS="--suppr $(dirname $0)/libabigail.abignore --no-added-syms"
> +ABIDIFF_OPTIONS="--suppr $(dirname $0))/libabigail.abignore" \
> + " --suppr $(dirname $0))/libabigail.internalignore" \
> + " --no-added-syms"
>
> if [ ! -d $refdir ]; then
> echo "Error: reference directory '$refdir' does not exist."
> diff --git a/devtools/libabigail.internalignore b/devtools/libabigail.internalignore
> new file mode 100644
> index 0000000..55df5b1
> --- /dev/null
> +++ b/devtools/libabigail.internalignore
> @@ -0,0 +1,4 @@
> +; These functions were moved from stable to internal.
> +; This is a temporary exception till DPDK 20.11.
This is not until 20.11 if we merge this patch now.
It is only when comparing 20.02 to 20.05.
After this, symbols will be marked INTERNAL and will be skipped by the
generic rule, so we can remove this exception in 20.08-rc0.
> +[suppress_file]
> + file_name_regexp = librte*mlx5*
> --
> 2.8.4
A stricter rule is enough, can you try something like:
+[suppress_file]
+ file_name_regexp = ^librte_common_mlx5\.
On 05/05/2020 15:53, David Marchand wrote:
> Don't forget to Cc: Ray.
>
> On Tue, May 5, 2020 at 1:21 PM Ophir Munk <ophirmu@mellanox.com> wrote:
>>
>> Create file libabigail.internalignore where it is specified to ignore
>> all symbols in library file librte_pmd_mlx.* during an ABI check.
>
> I don't like the separate file, having rules with comments in a single
> place is easier to maintain.
agreed, it would become confusing which file should be updated?
>
> Why waive all changes in all mlx drivers?
> There is a single issue with 20.02, see below.
isn't this exactly what rte_internal was created for right?
>
>
> Besides, looking at next patches, why keep experimental symbols?
> If all symbols in common/mlx5 are for internal use, mark them _all_ as such.
>
>
>>
>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>> ---
>> devtools/check-abi.sh | 4 +++-
>> devtools/libabigail.internalignore | 4 ++++
>> 2 files changed, 7 insertions(+), 1 deletion(-)
>> create mode 100644 devtools/libabigail.internalignore
>>
>> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
>> index dd9120e..3bad19a 100755
>> --- a/devtools/check-abi.sh
>> +++ b/devtools/check-abi.sh
>> @@ -10,7 +10,9 @@ fi
>> refdir=$1
>> newdir=$2
>> warnonly=${3:-}
>> -ABIDIFF_OPTIONS="--suppr $(dirname $0)/libabigail.abignore --no-added-syms"
>> +ABIDIFF_OPTIONS="--suppr $(dirname $0))/libabigail.abignore" \
>> + " --suppr $(dirname $0))/libabigail.internalignore" \
>> + " --no-added-syms"
>>
>> if [ ! -d $refdir ]; then
>> echo "Error: reference directory '$refdir' does not exist."
>> diff --git a/devtools/libabigail.internalignore b/devtools/libabigail.internalignore
>> new file mode 100644
>> index 0000000..55df5b1
>> --- /dev/null
>> +++ b/devtools/libabigail.internalignore
>> @@ -0,0 +1,4 @@
>> +; These functions were moved from stable to internal.
>> +; This is a temporary exception till DPDK 20.11.
>
> This is not until 20.11 if we merge this patch now.
> It is only when comparing 20.02 to 20.05.
> After this, symbols will be marked INTERNAL and will be skipped by the
> generic rule, so we can remove this exception in 20.08-rc0.
>
>
>> +[suppress_file]
>> + file_name_regexp = librte*mlx5*
>> --
>> 2.8.4
>
> A stricter rule is enough, can you try something like:
>
> +[suppress_file]
> + file_name_regexp = ^librte_common_mlx5\.
>
>
05/05/2020 16:53, David Marchand:
> On Tue, May 5, 2020 at 1:21 PM Ophir Munk <ophirmu@mellanox.com> wrote:
> > +; These functions were moved from stable to internal.
> > +; This is a temporary exception till DPDK 20.11.
>
> This is not until 20.11 if we merge this patch now.
> It is only when comparing 20.02 to 20.05.
> After this, symbols will be marked INTERNAL and will be skipped by the
> generic rule, so we can remove this exception in 20.08-rc0.
I think we need to keep this rule until 20.11,
because we want to ignore symbols which were in 19.11 and 20.02,
and removed (as internal) in 20.05.
If we remove this rule in 20.08, and comparing with 19.11,
the tool will complain about removed symbols, isn't it?
On Wed, May 6, 2020 at 2:27 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 05/05/2020 16:53, David Marchand:
> > On Tue, May 5, 2020 at 1:21 PM Ophir Munk <ophirmu@mellanox.com> wrote:
> > > +; These functions were moved from stable to internal.
> > > +; This is a temporary exception till DPDK 20.11.
> >
> > This is not until 20.11 if we merge this patch now.
> > It is only when comparing 20.02 to 20.05.
> > After this, symbols will be marked INTERNAL and will be skipped by the
> > generic rule, so we can remove this exception in 20.08-rc0.
>
> I think we need to keep this rule until 20.11,
> because we want to ignore symbols which were in 19.11 and 20.02,
> and removed (as internal) in 20.05.
Those symbols were exported in 20.02.
Before this, they were internal to mlx5 net driver.
>
> If we remove this rule in 20.08, and comparing with 19.11,
> the tool will complain about removed symbols, isn't it?
If we remove the rule in 20.08 and compare to _20.02_ then yes, we
would get an issue.
I was only thinking of comparing 20.08 with 20.05.
We can keep this until 20.11 if you think it is safer.
06/05/2020 14:42, David Marchand:
> On Wed, May 6, 2020 at 2:27 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 05/05/2020 16:53, David Marchand:
> > > On Tue, May 5, 2020 at 1:21 PM Ophir Munk <ophirmu@mellanox.com> wrote:
> > > > +; These functions were moved from stable to internal.
> > > > +; This is a temporary exception till DPDK 20.11.
> > >
> > > This is not until 20.11 if we merge this patch now.
> > > It is only when comparing 20.02 to 20.05.
> > > After this, symbols will be marked INTERNAL and will be skipped by the
> > > generic rule, so we can remove this exception in 20.08-rc0.
> >
> > I think we need to keep this rule until 20.11,
> > because we want to ignore symbols which were in 19.11 and 20.02,
> > and removed (as internal) in 20.05.
>
> Those symbols were exported in 20.02.
Right, no issue when comparing with 19.11.
> Before this, they were internal to mlx5 net driver.
>
>
> >
> > If we remove this rule in 20.08, and comparing with 19.11,
> > the tool will complain about removed symbols, isn't it?
>
> If we remove the rule in 20.08 and compare to _20.02_ then yes, we
> would get an issue.
> I was only thinking of comparing 20.08 with 20.05.
>
> We can keep this until 20.11 if you think it is safer.
Yes I think we must make sure any comparison can be done during
compatibility lifetime.
On 06/05/2020 13:52, Thomas Monjalon wrote:
> 06/05/2020 14:42, David Marchand:
>> On Wed, May 6, 2020 at 2:27 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>>
>>> 05/05/2020 16:53, David Marchand:
>>>> On Tue, May 5, 2020 at 1:21 PM Ophir Munk <ophirmu@mellanox.com> wrote:
>>>>> +; These functions were moved from stable to internal.
>>>>> +; This is a temporary exception till DPDK 20.11.
>>>>
>>>> This is not until 20.11 if we merge this patch now.
>>>> It is only when comparing 20.02 to 20.05.
>>>> After this, symbols will be marked INTERNAL and will be skipped by the
>>>> generic rule, so we can remove this exception in 20.08-rc0.
>>>
>>> I think we need to keep this rule until 20.11,
>>> because we want to ignore symbols which were in 19.11 and 20.02,
>>> and removed (as internal) in 20.05.
>>
>> Those symbols were exported in 20.02.
>
> Right, no issue when comparing with 19.11.
Right, which also makes it different to the Traffic Manager issue.
As these symbols where never in v20 - 19.11, they won't be missed.
We never had a commitment to maintain them.
If there were in v20, the situation might be considered differently.
>
>> Before this, they were internal to mlx5 net driver.
>>
>>
>>>
>>> If we remove this rule in 20.08, and comparing with 19.11,
>>> the tool will complain about removed symbols, isn't it?
>>
>> If we remove the rule in 20.08 and compare to _20.02_ then yes, we
>> would get an issue.
>> I was only thinking of comparing 20.08 with 20.05.
>>
>> We can keep this until 20.11 if you think it is safer.
>
> Yes I think we must make sure any comparison can be done during
> compatibility lifetime.
As David said, 20.08 _should_ be being compared to 20.05 by the CI.
So it should be find to drop the rule then.
However
It might make easier though to let libabigail.ignore accumulate these kind of
suppressions between major releases / lts (v20 -> v21), and the completely
reset, and start over.
Ray K
06/05/2020 16:15, Ray Kinsella:
>
> On 06/05/2020 13:52, Thomas Monjalon wrote:
> > 06/05/2020 14:42, David Marchand:
> >> On Wed, May 6, 2020 at 2:27 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >>>
> >>> 05/05/2020 16:53, David Marchand:
> >>>> On Tue, May 5, 2020 at 1:21 PM Ophir Munk <ophirmu@mellanox.com> wrote:
> >>>>> +; These functions were moved from stable to internal.
> >>>>> +; This is a temporary exception till DPDK 20.11.
> >>>>
> >>>> This is not until 20.11 if we merge this patch now.
> >>>> It is only when comparing 20.02 to 20.05.
> >>>> After this, symbols will be marked INTERNAL and will be skipped by the
> >>>> generic rule, so we can remove this exception in 20.08-rc0.
> >>>
> >>> I think we need to keep this rule until 20.11,
> >>> because we want to ignore symbols which were in 19.11 and 20.02,
> >>> and removed (as internal) in 20.05.
> >>
> >> Those symbols were exported in 20.02.
> >
> > Right, no issue when comparing with 19.11.
>
> Right, which also makes it different to the Traffic Manager issue.
> As these symbols where never in v20 - 19.11, they won't be missed.
> We never had a commitment to maintain them.
>
> If there were in v20, the situation might be considered differently.
>
> >
> >> Before this, they were internal to mlx5 net driver.
> >>
> >>
> >>>
> >>> If we remove this rule in 20.08, and comparing with 19.11,
> >>> the tool will complain about removed symbols, isn't it?
> >>
> >> If we remove the rule in 20.08 and compare to _20.02_ then yes, we
> >> would get an issue.
> >> I was only thinking of comparing 20.08 with 20.05.
> >>
> >> We can keep this until 20.11 if you think it is safer.
> >
> > Yes I think we must make sure any comparison can be done during
> > compatibility lifetime.
>
> As David said, 20.08 _should_ be being compared to 20.05 by the CI.
> So it should be find to drop the rule then.
"should", but users "could" do something else ;-)
> However
> It might make easier though to let libabigail.ignore accumulate these kind of
> suppressions between major releases / lts (v20 -> v21), and the completely
> reset, and start over.
Yes it's simpler and safer.
Thanks all for this discussion. I have sent v2 of this patchset.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, May 6, 2020 5:23 PM
> To: David Marchand <david.marchand@redhat.com>; Ophir Munk
> <ophirmu@mellanox.com>; Ray Kinsella <mdr@ashroe.eu>
> Cc: dev <dev@dpdk.org>; Raslan Darawsheh <rasland@mellanox.com>;
> Matan Azrad <matan@mellanox.com>; Neil Horman
> <nhorman@tuxdriver.com>
> Subject: Re: [dpdk-dev] [PATCH v1 1/3] devtools: create internal ignore file
> for libabigail
>
> 06/05/2020 16:15, Ray Kinsella:
> >
> > On 06/05/2020 13:52, Thomas Monjalon wrote:
> > > 06/05/2020 14:42, David Marchand:
> > >> On Wed, May 6, 2020 at 2:27 PM Thomas Monjalon
> <thomas@monjalon.net> wrote:
> > >>>
> > >>> 05/05/2020 16:53, David Marchand:
> > >>>> On Tue, May 5, 2020 at 1:21 PM Ophir Munk
> <ophirmu@mellanox.com> wrote:
> > >>>>> +; These functions were moved from stable to internal.
> > >>>>> +; This is a temporary exception till DPDK 20.11.
> > >>>>
> > >>>> This is not until 20.11 if we merge this patch now.
> > >>>> It is only when comparing 20.02 to 20.05.
> > >>>> After this, symbols will be marked INTERNAL and will be skipped
> > >>>> by the generic rule, so we can remove this exception in 20.08-rc0.
> > >>>
> > >>> I think we need to keep this rule until 20.11, because we want to
> > >>> ignore symbols which were in 19.11 and 20.02, and removed (as
> > >>> internal) in 20.05.
> > >>
> > >> Those symbols were exported in 20.02.
> > >
> > > Right, no issue when comparing with 19.11.
> >
> > Right, which also makes it different to the Traffic Manager issue.
> > As these symbols where never in v20 - 19.11, they won't be missed.
> > We never had a commitment to maintain them.
> >
> > If there were in v20, the situation might be considered differently.
> >
> > >
> > >> Before this, they were internal to mlx5 net driver.
> > >>
> > >>
> > >>>
> > >>> If we remove this rule in 20.08, and comparing with 19.11, the
> > >>> tool will complain about removed symbols, isn't it?
> > >>
> > >> If we remove the rule in 20.08 and compare to _20.02_ then yes, we
> > >> would get an issue.
> > >> I was only thinking of comparing 20.08 with 20.05.
> > >>
> > >> We can keep this until 20.11 if you think it is safer.
> > >
> > > Yes I think we must make sure any comparison can be done during
> > > compatibility lifetime.
> >
> > As David said, 20.08 _should_ be being compared to 20.05 by the CI.
> > So it should be find to drop the rule then.
>
> "should", but users "could" do something else ;-)
>
> > However
> > It might make easier though to let libabigail.ignore accumulate these
> > kind of suppressions between major releases / lts (v20 -> v21), and
> > the completely reset, and start over.
>
> Yes it's simpler and safer.
>
@@ -10,7 +10,9 @@ fi
refdir=$1
newdir=$2
warnonly=${3:-}
-ABIDIFF_OPTIONS="--suppr $(dirname $0)/libabigail.abignore --no-added-syms"
+ABIDIFF_OPTIONS="--suppr $(dirname $0))/libabigail.abignore" \
+ " --suppr $(dirname $0))/libabigail.internalignore" \
+ " --no-added-syms"
if [ ! -d $refdir ]; then
echo "Error: reference directory '$refdir' does not exist."
new file mode 100644
@@ -0,0 +1,4 @@
+; These functions were moved from stable to internal.
+; This is a temporary exception till DPDK 20.11.
+[suppress_file]
+ file_name_regexp = librte*mlx5*