From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8D8E6A09E8; Tue, 8 Dec 2020 16:22:49 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D5EA8A3; Tue, 8 Dec 2020 16:22:47 +0100 (CET) Received: from dal2relay136.mxroute.com (dal2relay136.mxroute.com [64.40.26.136]) by dpdk.org (Postfix) with ESMTP id D946498 for ; Tue, 8 Dec 2020 16:22:45 +0100 (CET) Received: from filter003.mxroute.com ([168.235.111.26] 168-235-111-26.cloud.ramnode.com) (Authenticated sender: mN4UYu2MZsgR) by dal2relay136.mxroute.com (ZoneMTA) with ESMTPSA id 17642f2884f0008081.001 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Tue, 08 Dec 2020 15:22:39 +0000 X-Zone-Loop: fccd58f0ab7a59f5f77085ba7f55644eefc1bfc91031 X-Originating-IP: [168.235.111.26] Received: from echo.mxrouting.net (echo.mxrouting.net [116.202.222.109]) by filter003.mxroute.com (Postfix) with ESMTPS id 129FA60043; Tue, 8 Dec 2020 15:22:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=ashroe.eu; s=x; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=5eeK92OmMv2+oCz6heYQ+CqdAAAVdv0TSPbvleuDHIo=; b=mgJMkTNhOB+nwbm5Tnv19j8qZy RygS0F3BybtKzkWPTACdDj6FkqEZIbvXIhxzQFK2hwdc46wEMnOTAeLaEP3fM24IyxoMvE0JKq7si Eoz2661NikzkeYgnWeJIbN3GGxtrjWn8YWjntemBhz2vJdoS23Cd9LG72Ld7Afkp1kupKuJQeF4IU l1+q1peAJu8roW+lk4crIInSMT61p4dyDDiA+IqYJ1byFMHrTfHe8iF5lniJ19La2Aq0nn93xMvMY Kk65SueLM8W0hLwHfhA2SGG3RE+kFnrpW7NDBf6yqw+4WOdsPWIfMI3IiXyxGvQD7H/yTCXDFNbN8 Gnp4uBJQ==; To: Thomas Monjalon , dev@dpdk.org Cc: david.marchand@redhat.com, bruce.richardson@intel.com, Neil Horman References: <20201207173235.1397351-1-thomas@monjalon.net> From: "Kinsella, Ray" Message-ID: <477b7775-0a3e-080b-3158-bc8fbae95c81@ashroe.eu> Date: Tue, 8 Dec 2020 15:22:35 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.5.1 MIME-Version: 1.0 In-Reply-To: <20201207173235.1397351-1-thomas@monjalon.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-AuthUser: mdr@ashroe.eu Subject: Re: [dpdk-dev] [PATCH 1/1] devtools: adjust verbosity of ABI check X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 07/12/2020 17:32, Thomas Monjalon wrote: > The scripts gen-abi.sh and check-abi.sh are updated > to print error messages to stderr so they are likely never ignored. > > When called from test-meson-builds.sh, the standard messages on stdout > can be more quiet depending on the verbosity settings. > The beginning of the ABI check is announced in verbose mode. > The commands are printed in very verbose mode. > The check result details are available in verbose mode. So there is a bit of a disconnect here - you change gen-abi/check-abi to correctly direct errors to sterr. You then however provide a method to ignore them in test_meson_build.sh. I thinking giving people a way of ignoring the indicated lines below, is a bad plan. No problem with the changes to check-abi/gen-abi - but I think the changes to test_meson_build.sh are a bad idea. > Signed-off-by: Thomas Monjalon > --- > devtools/check-abi.sh | 21 +++++++++++---------- > devtools/gen-abi.sh | 4 ++-- > devtools/test-meson-builds.sh | 9 +++++++-- > 3 files changed, 20 insertions(+), 14 deletions(-) > > diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh > index ab6748cfbc..381db2cdd1 100755 > --- a/devtools/check-abi.sh > +++ b/devtools/check-abi.sh > @@ -3,7 +3,7 @@ > # Copyright (c) 2019 Red Hat, Inc. > > if [ $# != 2 ] && [ $# != 3 ]; then > - echo "Usage: $0 refdir newdir [warnonly]" > + echo "Usage: $0 refdir newdir [warnonly]" >&2 > exit 1 > fi > > @@ -13,23 +13,23 @@ warnonly=${3:-} > ABIDIFF_OPTIONS="--suppr $(dirname $0)/libabigail.abignore --no-added-syms" > > if [ ! -d $refdir ]; then > - echo "Error: reference directory '$refdir' does not exist." > + echo "Error: reference directory '$refdir' does not exist." >&2 > exit 1 > fi > incdir=$(find $refdir -type d -a -name include) > if [ -z "$incdir" ] || [ ! -e "$incdir" ]; then > - echo "WARNING: could not identify a include directory for $refdir, expect false positives..." > + echo "WARNING: could not identify an include directory for $refdir, expect false positives..." >&2 > else > ABIDIFF_OPTIONS="$ABIDIFF_OPTIONS --headers-dir1 $incdir" > fi > > if [ ! -d $newdir ]; then > - echo "Error: directory to check '$newdir' does not exist." > + echo "Error: directory to check '$newdir' does not exist." >&2 > exit 1 > fi > incdir2=$(find $newdir -type d -a -name include) > if [ -z "$incdir2" ] || [ ! -e "$incdir2" ]; then > - echo "WARNING: could not identify a include directory for $newdir, expect false positives..." > + echo "WARNING: could not identify an include directory for $newdir, expect false positives..." >&2 > else > ABIDIFF_OPTIONS="$ABIDIFF_OPTIONS --headers-dir2 $incdir2" > fi > @@ -46,23 +46,24 @@ for dump in $(find $refdir -name "*.dump"); do > fi > dump2=$(find $newdir -name $name) > if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then > - echo "Error: can't find $name in $newdir" > + echo "Error: cannot find $name in $newdir" >&2 > error=1 > continue > fi > + echo abidiff $ABIDIFF_OPTIONS $dump $dump2 > abidiff $ABIDIFF_OPTIONS $dump $dump2 || { > abiret=$? > - echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'" > + echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'" >&2 > error=1 > echo > if [ $(($abiret & 3)) -ne 0 ]; then > - echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, this could be a script or environment issue." > + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, this could be a script or environment issue." >&2 > fi > if [ $(($abiret & 4)) -ne 0 ]; then > - echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)." > + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)." >&2 > fi > if [ $(($abiret & 8)) -ne 0 ]; then > - echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI." > + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI." >&2> fi > echo > } > diff --git a/devtools/gen-abi.sh b/devtools/gen-abi.sh > index c44b0e228a..f15a3b9aaf 100755 > --- a/devtools/gen-abi.sh > +++ b/devtools/gen-abi.sh > @@ -3,13 +3,13 @@ > # Copyright (c) 2019 Red Hat, Inc. > > if [ $# != 1 ]; then > - echo "Usage: $0 installdir" > + echo "Usage: $0 installdir" >&2 > exit 1 > fi > > installdir=$1 > if [ ! -d $installdir ]; then > - echo "Error: install directory '$installdir' does not exist." > + echo "Error: install directory '$installdir' does not exist." >&2 > exit 1 > fi > > diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh > index ed44d4ffb1..16a81b6241 100755 > --- a/devtools/test-meson-builds.sh > +++ b/devtools/test-meson-builds.sh > @@ -194,10 +194,15 @@ build () # > > 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) > + $(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 \ > - $(readlink -f $builds_dir/$targetdir/install) > + $(readlink -f $builds_dir/$targetdir/install) >&$verbose > fi > } > >