Re: [dpdk-dev] [PATCH v3] devtools: rework abi checker script

[Neil Horman <nhorman@tuxdriver.com>]

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.

Thoughts
Neil