Re: [dpdk-dev] [PATCH v1 1/3] devtools: create internal ignore file for libabigail

[Ray Kinsella <mdr@ashroe.eu>]

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\.
> 
>