Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi Neil,

2015-03-11 15:36, Neil Horman:
> On Thu, Mar 05, 2015 at 11:57:27AM -0500, Neil Horman wrote:
> > On Wed, Mar 04, 2015 at 05:49:50PM +0100, Thomas Monjalon wrote:
> > > 2015-03-04 11:26, Neil Horman:
> > > > +#trap on ctrl-c to clean up
> > > > +trap cleanup_and_exit SIGINT
> > > 
> > > I think INT is preffered over SIGINT.
> > > You may also add QUIT and TERM.
> > > With QUIT, you can replace cleanup_and_exit calls by a simple exit.
> > > 
> > > > +	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
> > > 
> > > May be simpler "git log -1 --format=%H"
> > > 
> > It might be, but the above is equivalent, and --format is a more recent git-log
> > feature.  Older versions still require --pretty=format
> > 
> > > > +log "INFO" "We're going to check and make sure that applications built"
> > > > +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> > > > +log "INFO" "against DPDK DSOs built from tag $TAG2."
> > > 
> > > I think it may be removed as no app is run.
> > > 
> > The above doesn't indicate that an application will be run, only that the
> > purpose of this script is to ensure that older applications will still run,
> > which I think is appropriate.
> > 
> > > > +# Make sure we configure SHARED libraries
> > > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > > 
> > > So you prefer modifying defconfig instead of .config, right?
> > > (you sent it while I was answering on v2)
> > > 
> > Yes, correct.
> > 
> > > > +# Checking abi compliance relies on using the dwarf information in
> > > > +# The shared objects.  Thats only included in the DSO's if we build
> > > > +# with -g
> > > > +export EXTRA_CFLAGS=-g
> > > > +export EXTRA_LDFLAGS=-g
> > > [...]
> > > > +export EXTRA_CFLAGS=-g
> > > > +export EXTRA_LDFLAGS=-g
> > > 
> > > Already exported.
> > > 
> > Yeah, I'll clean that up later.

OK, could you send a v4 please?

> > > > +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> > > 
> > > Could be OLDNAME=$(basename $i 1.dump)0.dump
> > > 
> > > > +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> > > 
> > > Could be LIBNAME=$(basename $i -ABI-1.dump)
> > > 
> > It could be, but I prefer the clarity of the sed replacement.
> > 
> > Neil
> > 
> > > Thanks
> > > 
> > > 
> > 
> 
> Ping Thomas, is this going to make 2.0?

Yes sure, waiting a v4.