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

[David Marchand <david.marchand@redhat.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\.


-- 
David Marchand