From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
 by dpdk.org (Postfix) with ESMTP id F3A6DDE0
 for <dev@dpdk.org>; Tue, 19 Sep 2017 11:15:23 +0200 (CEST)
Received: from [37.168.108.167] (helo=droids-corp.org)
 by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.84_2) (envelope-from <olivier.matz@6wind.com>)
 id 1duEiC-00035T-1z; Tue, 19 Sep 2017 11:21:01 +0200
Received: by droids-corp.org (sSMTP sendmail emulation);
 Tue, 19 Sep 2017 11:15:12 +0200
Date: Tue, 19 Sep 2017 11:15:12 +0200
From: Olivier MATZ <olivier.matz@6wind.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: dev@dpdk.org, bruce.richardson@intel.com
Message-ID: <20170919091511.ildim4ds2lv7sy4d@neon>
References: <20170911081801.tnf7qtt4qq5jkofe@neon>
 <20170911084635.11707-1-olivier.matz@6wind.com>
 <20170913150013.GB4320@hmswarspite.think-freely.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170913150013.GB4320@hmswarspite.think-freely.org>
User-Agent: NeoMutt/20170113 (1.7.2)
Subject: Re: [dpdk-dev] [PATCH v3] devtools: rework abi checker script
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 19 Sep 2017 09:15:24 -0000

Hi Neil,

On Wed, Sep 13, 2017 at 11:00:13AM -0400, Neil Horman wrote:
> On Mon, Sep 11, 2017 at 10:46:35AM +0200, Olivier Matz wrote:
> > The initial version of the script had some limitations:
> > - cannot work on a non-clean workspace
> > - environment variables are not documented
> > - no compilation log in case of failure
> > - return success even it abi is incompatible
> > 
> > This patch addresses these issues and rework the code.
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> > 
> > v2->v3:
> > - fix when not launched from dpdk root dir
> > - use "-Og -Wno-error" instead of "-O0"
> > - fix typo in commit log
> > 
> > v1->v2:
> > - use /usr/bin/env to find bash (which is required)
> > - fix displayed path to html reports
> > - reword help for -f option
> > 
> >  devtools/validate-abi.sh | 392 ++++++++++++++++++++++++-----------------------
> >  1 file changed, 200 insertions(+), 192 deletions(-)
> > 
> 
> This is alot closer, I think theres just a little wierdness left.  When running
> this checker script, the end of the log shows this:
> 
> CMD: abi-compliance-checker -l librte_vhost.so -old /home/nhorman/git/dpdk/devtools/abi-check/222555480/librte_vhost.so.dump -new /home/nhorman/git/dpdk/devtools/abi-check/02657b4ad/librte_vhost.so.dump
> NOTICE: At least one call to abi-compliance-checker returned an error.
> NOTICE: ABI may be incompatible, please check logs for details.
> NOTICE: Incompatible list:  librte_cryptodev.so librte_eal.so librte_efd.so librte_ethdev.so librte_eventdev.so librte_hash.so librte_pdump.so librte_pmd_crypto_scheduler.so librte_pmd_ring.so librte_ring.so
> 
> Which I think is something of a false positive.  The line:
> NOTICE: At least one call to abi-compliance-checker returned an error.
>
> I think is emitted simply because abi-compilance-checker returns non-zero and
> reports that error if it finds any incompatibilities.  I'm not sure we want to
> flag that as an error per se.  It gives the impression something has gone wrong,
> rather than correctly identifying that there are incompatibilities.
> 
> Though on the other hand, maybe we do want to set that red flag so people look
> at what the incompatibilities are.  As I say it out loud, perhaps that
> preferable.

Yes, I can remove this line
"At least one call to abi-compliance-checker returned an error."
which does not give a lot of info anyway.

While reproducing the issue, I also realized that the report path
was not preoperly fixed. So I'm sending a v4 with these fixes.

Thanks,
Olivier