DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: dev@dpdk.org
Cc: Aaron Conole <aconole@redhat.com>,
	Michael Santana <maicolgabriel@hotmail.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Bruce Richardson <bruce.richardson@intel.com>
Subject: [PATCH v2 2/2] devtools: stop depending on libabigail xml format
Date: Thu, 23 Mar 2023 18:15:02 +0100	[thread overview]
Message-ID: <20230323171502.3124049-3-david.marchand@redhat.com> (raw)
In-Reply-To: <20230323171502.3124049-1-david.marchand@redhat.com>

A ABI reference depends on:
- DPDK build options,
- toolchain compiler and versions,
- libabigail version.

The reason for the latter point is that, when the ABI reference was
generated, ABI xml files were dumped in a format depending on the
libabigail version.
Those xml files were then later used to compare against modified
code.

There are a few disadvantages with this method:
- since the xml files are dependent on the libabigail version, when
  updating CI environments, a change in the libabigail package requires
  regenerating the ABI references,
- comparing xml files with abidiff is not well tested, as we (DPDK)
  uncovered bugs in libabigail that were not hit with comparing .so,

Switch to comparing .so directly, remove this dependence and update GHA
script.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .ci/linux-build.sh            |  4 ----
 .github/workflows/build.yml   |  2 +-
 MAINTAINERS                   |  1 -
 devtools/check-abi.sh         | 17 +++++++++--------
 devtools/gen-abi.sh           | 27 ---------------------------
 devtools/test-meson-builds.sh |  5 -----
 6 files changed, 10 insertions(+), 46 deletions(-)
 delete mode 100755 devtools/gen-abi.sh

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 150b38bd7a..9631e342b5 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -130,8 +130,6 @@ fi
 if [ "$ABI_CHECKS" = "true" ]; then
     if [ "$(cat libabigail/VERSION 2>/dev/null)" != "$LIBABIGAIL_VERSION" ]; then
         rm -rf libabigail
-        # if we change libabigail, invalidate existing abi cache
-        rm -rf reference
     fi
 
     if [ ! -d libabigail ]; then
@@ -153,7 +151,6 @@ if [ "$ABI_CHECKS" = "true" ]; then
         meson setup $OPTS -Dexamples= $refsrcdir $refsrcdir/build
         ninja -C $refsrcdir/build
         DESTDIR=$(pwd)/reference ninja -C $refsrcdir/build install
-        devtools/gen-abi.sh reference
         find reference/usr/local -name '*.a' -delete
         rm -rf reference/usr/local/bin
         rm -rf reference/usr/local/share
@@ -161,7 +158,6 @@ if [ "$ABI_CHECKS" = "true" ]; then
     fi
 
     DESTDIR=$(pwd)/install ninja -C build install
-    devtools/gen-abi.sh install
     devtools/check-abi.sh reference install ${ABI_CHECKS_WARN_ONLY:-}
 fi
 
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index bbcb535afb..e24e47a216 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -70,7 +70,7 @@ jobs:
       run: |
         echo 'ccache=ccache-${{ matrix.config.os }}-${{ matrix.config.compiler }}-${{ matrix.config.cross }}-'$(date -u +%Y-w%W) >> $GITHUB_OUTPUT
         echo 'libabigail=libabigail-${{ matrix.config.os }}' >> $GITHUB_OUTPUT
-        echo 'abi=abi-${{ matrix.config.os }}-${{ matrix.config.compiler }}-${{ matrix.config.cross }}-${{ env.LIBABIGAIL_VERSION }}-${{ env.REF_GIT_TAG }}' >> $GITHUB_OUTPUT
+        echo 'abi=abi-${{ matrix.config.os }}-${{ matrix.config.compiler }}-${{ matrix.config.cross }}-${{ env.REF_GIT_TAG }}' >> $GITHUB_OUTPUT
     - name: Retrieve ccache cache
       uses: actions/cache@v3
       with:
diff --git a/MAINTAINERS b/MAINTAINERS
index 1a33ad8592..280058adfc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -94,7 +94,6 @@ F: devtools/check-spdx-tag.sh
 F: devtools/check-symbol-change.sh
 F: devtools/check-symbol-maps.sh
 F: devtools/checkpatches.sh
-F: devtools/gen-abi.sh
 F: devtools/get-maintainer.sh
 F: devtools/git-log-fixes.sh
 F: devtools/load-devel-config
diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
index f74432be5d..39e3798931 100755
--- a/devtools/check-abi.sh
+++ b/devtools/check-abi.sh
@@ -37,20 +37,21 @@ fi
 
 export newdir ABIDIFF_OPTIONS ABIDIFF_SUPPRESSIONS
 export diff_func='run_diff() {
-	dump=$1
-	name=$(basename $dump)
-	if grep -q "; SKIP_LIBRARY=${name%.dump}\>" $ABIDIFF_SUPPRESSIONS; then
+	lib=$1
+	name=$(basename $lib)
+	if grep -q "; SKIP_LIBRARY=${name%.so.*}\>" $ABIDIFF_SUPPRESSIONS; then
 		echo "Skipped $name" >&2
 		return 0
 	fi
-	dump2=$(find $newdir -name $name)
-	if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then
+	# Look for a library with the same major ABI version
+	lib2=$(find $newdir -name "${name%.*}.*" -a ! -type l)
+	if [ -z "$lib2" ] || [ ! -e "$lib2" ]; then
 		echo "Error: cannot find $name in $newdir" >&2
 		return 1
 	fi
-	abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
+	abidiff $ABIDIFF_OPTIONS $lib $lib2 || {
 		abiret=$?
-		echo "Error: ABI issue reported for abidiff $ABIDIFF_OPTIONS $dump $dump2" >&2
+		echo "Error: ABI issue reported for abidiff $ABIDIFF_OPTIONS $lib $lib2" >&2
 		if [ $(($abiret & 3)) -ne 0 ]; then
 			echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, this could be a script or environment issue." >&2
 		fi
@@ -65,7 +66,7 @@ export diff_func='run_diff() {
 }'
 
 error=
-find $refdir -name "*.dump" |
+find $refdir -name "*.so.*" -a ! -type l |
 xargs -n1 -P0 sh -c 'eval "$diff_func"; run_diff $0' ||
 error=1
 
diff --git a/devtools/gen-abi.sh b/devtools/gen-abi.sh
deleted file mode 100755
index 61f7510ea1..0000000000
--- a/devtools/gen-abi.sh
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/bin/sh -e
-# SPDX-License-Identifier: BSD-3-Clause
-# Copyright (c) 2019 Red Hat, Inc.
-
-if [ $# != 1 ]; then
-	echo "Usage: $0 installdir" >&2
-	exit 1
-fi
-
-installdir=$1
-if [ ! -d $installdir ]; then
-	echo "Error: install directory '$installdir' does not exist." >&2
-	exit 1
-fi
-
-dumpdir=$installdir/dump
-rm -rf $dumpdir
-mkdir -p $dumpdir
-for f in $(find $installdir -name "*.so.*"); do
-	if test -L $f; then
-		continue
-	fi
-
-	libname=$(basename $f)
-	echo $dumpdir/${libname%.so*}.dump $f
-done |
-xargs -n2 -P0 abidw --out-file
diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 48f4e52df3..9131088c9d 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -204,7 +204,6 @@ build () # <directory> <target cc | cross file> <ABI check> [meson options]
 				-Dexamples= $*
 			compile $abirefdir/build
 			install_target $abirefdir/build $abirefdir/$targetdir
-			$srcdir/devtools/gen-abi.sh $abirefdir/$targetdir
 
 			# save disk space by removing static libs and apps
 			find $abirefdir/$targetdir/usr/local -name '*.a' -delete
@@ -215,10 +214,6 @@ build () # <directory> <target cc | cross file> <ABI check> [meson options]
 		install_target $builds_dir/$targetdir \
 			$(readlink -f $builds_dir/$targetdir/install)
 		echo "Checking ABI compatibility of $targetdir" >&$verbose
-		echo $srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install) >&$veryverbose
-		$srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install) >&$veryverbose
 		echo $srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
 			$(readlink -f $builds_dir/$targetdir/install) >&$veryverbose
 		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
-- 
2.39.2


  parent reply	other threads:[~2023-03-23 17:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-03 15:47 [PATCH 0/2] ABI check updates David Marchand
2022-11-03 15:47 ` [PATCH 1/2] devtools: unify configuration for ABI check David Marchand
2022-11-03 15:47 ` [PATCH 2/2] devtools: stop depending on libabigail xml format David Marchand
2023-03-23 17:15 ` [PATCH v2 0/2] ABI check updates David Marchand
2023-03-23 17:15   ` [PATCH v2 1/2] devtools: unify configuration for ABI check David Marchand
2023-03-23 17:15   ` David Marchand [this message]
2023-03-28 18:38   ` [PATCH v2 0/2] ABI check updates Thomas Monjalon

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=20230323171502.3124049-3-david.marchand@redhat.com \
    --to=david.marchand@redhat.com \
    --cc=aconole@redhat.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=maicolgabriel@hotmail.com \
    --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).