DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4] ABI: Add abi checking utility
Date: Tue, 17 Mar 2015 16:42:31 +0100	[thread overview]
Message-ID: <8343380.8uC93YKvWO@xps13> (raw)
In-Reply-To: <1426255750-3961-1-git-send-email-nhorman@tuxdriver.com>

Hi Neil,

I tested this tool and I see few small improvements possible.

2015-03-13 10:09, Neil Horman:
> There was a request for an abi validation utilty for the ongoing ABI stability
                                            utility
> work.  As it turns out there is a abi compliance checker in development that
> seems to be under active development and provides fairly detailed ABI compliance
> reports.  Its not yet intellegent enough to understand symbol versioning, but it
                        intelligent
> does provide the ability to identify symbols which have changed between
> releases, along with details of the change, and offers developers the
> opportunity to identify which symbols then need versioning and validation for a
> given update via manual testing.
> 
> This script automates the use of the compliance checker between two arbitrarily
> specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> and run:
> 
> ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> 
> where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> suitable for passing as the T= variable in the make config command.
> 
> Note the upstream source for the abi compliance checker is here:
> http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> 
> It generates a report for each DSO built from the requested tags that developers
> can review to find ABI compliance issues.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> ---
> 
> Change Notes:
> 
> v2) Fixed some typos as requested by Thomas
> 
> v3) Fixed some additional typos Thomas requested
>     Improved script to work from detached state
>     Added some documentation to the changelog
>     Added some comments to the scripts
> 
> v4) Remove duplicate exports.
>     Move restoration of starting branch/comit to cleanup_and_exit
> ---
[...]
> +TAG1=$1
> +TAG2=$2
> +TARGET=$3
> +ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`

+JOBS=$(grep -c '^processor' /proc/cpuinfo)

[...]
> +cleanup_and_exit() {
> +	rm -rf $ABI_DIR
> +	exit $1
> +	git checkout $CURRENT_BRANCH

Checkout is never done because of previous exit.

> +}
[...]
> +log "INFO" "Checking out version $TAG1 of the dpdk"
> +# Move to the old version of the tree
> +git checkout $TAG1

What about -q for quiet mode?

[...]
> +log "INFO" "Building DPDK $TAG1. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1

-j$JOBS would improve building time

[...]
> +# Move to the new version of the tree
> +log "INFO" "Checking out version $TAG2 of the dpdk"
> +git checkout $TAG2

-q ?

[...]
> +log "INFO" "Building DPDK $TAG2. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1

-j ?

[...]
> +# Start comparison of ABI dumps
> +for i in `ls $ABI_DIR/*-1.dump`
> +do
> +	NEWNAME=`basename $i`
> +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> +
> +	if [ ! -f $ABI_DIR/$OLDNAME ]
> +	then
> +		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
> +	fi
> +
> +	#compare the abi dumps
> +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> +done

It would be more convenient to generate an HTML index giving access to every
reports for every DSOs.

> +
> +git reset --hard
> +log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
> +cleanup_and_exit 0

After reading the report, it's not clear what would be tolerated or not.
Should we forbid every defects?

  reply	other threads:[~2015-03-17 15:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-30 21:16 [dpdk-dev] [PATCH] " Neil Horman
2015-02-02 18:18 ` [dpdk-dev] [PATCH v2] " Neil Horman
2015-02-27 13:48   ` Neil Horman
2015-02-27 13:55     ` Thomas Monjalon
2015-03-03 22:18   ` Thomas Monjalon
2015-03-04 11:49     ` Neil Horman
2015-03-04 12:54       ` Thomas Monjalon
2015-03-04 14:39         ` Neil Horman
2015-03-04 15:15           ` Thomas Monjalon
2015-03-04 15:42             ` Neil Horman
2015-03-04 16:15               ` Thomas Monjalon
2015-03-04 16:26 ` [dpdk-dev] [PATCH v3] " Neil Horman
2015-03-04 16:49   ` Thomas Monjalon
2015-03-05 16:57     ` Neil Horman
2015-03-11 19:36       ` Neil Horman
2015-03-13  8:51         ` Thomas Monjalon
2015-03-13 11:56   ` Kavanagh, Mark B
2015-03-13 14:10     ` Neil Horman
2015-03-13 14:25       ` Kavanagh, Mark B
2015-03-13 14:58         ` Neil Horman
2015-03-13 15:49           ` Kavanagh, Mark B
2015-03-13 14:09 ` [dpdk-dev] [PATCH v4] " Neil Horman
2015-03-17 15:42   ` Thomas Monjalon [this message]
2015-03-17 16:47     ` Thomas Monjalon
2015-03-17 18:08     ` Neil Horman
2015-03-17 18:08 ` [dpdk-dev] [PATCH v5] " Neil Horman
2015-03-17 21:17   ` 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=8343380.8uC93YKvWO@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=nhorman@tuxdriver.com \
    /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).