From: Neil Horman <nhorman@tuxdriver.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: Ray Kinsella <mdr@ashroe.eu>, dev@dpdk.org, david.marchand@redhat.com
Subject: Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree
Date: Wed, 22 Apr 2020 08:01:08 -0400 [thread overview]
Message-ID: <20200422120108.GA864272@hmswarspite.think-freely.org> (raw)
In-Reply-To: <1695354.8hzESeGDPO@thomas>
On Tue, Apr 21, 2020 at 11:42:42PM +0200, Thomas Monjalon wrote:
> 21/04/2020 20:56, Neil Horman:
> > On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote:
> > > 21/04/2020 13:12, Neil Horman:
> > > > On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote:
> > > > > On 17/04/2020 13:10, Thomas Monjalon wrote:
> > > > > > 17/04/2020 13:47, Ray Kinsella:
> > > > > >> On 17/04/2020 11:20, Thomas Monjalon wrote:
> > > > > >>> 17/04/2020 12:11, Ray Kinsella:
> > > > > >>>> check-abi.sh appears to be backward step in terms of usability.
> > > > > >>>
> > > > > >>> No, check-abi.sh benefits from a nice integration in build scripts.
> > > > > >>> See below.
> > > > > >>>
> > > > > >>>> With validate-abi.sh I do can do a "validate-abi.sh HEAD~1 HEAD".
> > > > > >>>> And it will do the build, install, dump and comparison for me.
> > > > > >>>> And it picked up my 20.0.2 - > 21.0 changes no problem.
> > > > > >>>>
> > > > > >>>> With check-abi on the other hand, I need to the build and install myself.
> > > > > >>>> check-abi requires dump files, but I see no reference in the documentation to how these are created.
> > > > > >>>> It silently fails when it doesn't find any ...
> > > > > >>>>
> > > > > >>>> Do I run abi-dumper on the so's myself, or how does it work?
> > > > > >>>
> > > > > >>> check-abi.sh is integrated in test-build.sh and test-meson-builds.sh.
> > > > > >>> Probably we should document usage in these scripts.
> > > > > >>
> > > > > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious.
> > > > > >> Any tips or tricks would be welcome.
> > > > > >
> > > > > > export DPDK_ABI_REF_VERSION=v20.02
> > > > > > or
> > > > > > export DPDK_ABI_REF_VERSION=v19.11
> > > > > >
> > > > > > Depends on which compatibility you want to test...
> > > > > >
> > > > >
> > > > > Few things ...
> > > > >
> > > > > 1. test-meson-build.sh keep barfing complaining about reference paths.
> > > > > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app
> > > > >
> > > > > Under the hood, ninja install is failing complaining that it needs an absolute path.
> > > > > I fixed this in test_meson_build.sh and will send a patch in a minute.
> > > > > Though it's strange no-one else has seen it?
> > > > >
> > > > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense.
> > > > >
> > > > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally.
> > > > > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case.
> > > > > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD`
> > > > >
> > > > I think this code in test-meson-build.sh should probably be fixed:
> > > >
> > > > if [ ! -d $abirefdir/src ]; then
> > > > git clone --local --no-hardlinks \
> > > > --single-branch \
> > > > -b $DPDK_ABI_REF_VERSION \
> > > > $srcdir $abirefdir/src
> > > > fi
> > > >
> > > > Like you noted, using -b allows us to checkout a tag/branch in the cloned
> > > > repository but requires that it exist locally. We should probably prefix the
> > > > checkout with a git fetch --tags
> > >
> > > I don't understand your concern.
> > > A reference is an older version, so it should be in the git tree.
> > >
> > yes, but not unless you've done a recent pull or fetch. If you set
> > DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you
> > updated the tree, it won't be there (which it sounds like what is being
> > encountered here). You can fix that by doing a git pull or git fetch prior to
> > running this script (or internal to the script)
>
> Sorry I still don't understand the case.
> We want to compare the current version C with a reference R which is older.
> If the reference R is not in the tree, it means the version C is not in the tree.
> But C is the current version, so it is in the tree by definition.
>
+---+
| |
+---+ | | DPDK 20.11
| |Feature Branch commit +-+-+
| | |
+-+-+ +---+ +-v-+
| | | master HEAD | | master HEAD
| +-+-+ +-+-+
| | |
| | |
| +-v-+ +-v-+
+---->+ | DPDK 19.11 | | DPDK 19.11
+---+ +---+
X X X X
XXXXXXX XXXXXXXXX XXXXXXX XXXXXXXXX
XXX XX XXX XX
XXXX XXXX
Local GIT copy DPDK.ORG
If a user clones from dpdk.org when dpdk 19.11 is tagged in the tree, or any
time before dpdk 20.11 is tagged, then creates a feature branch, they may want
to compare their abi to the latest stable version. If they check dpdk.org and
see that dpdk 20.11 is the latest tag in the tree, they can run check-abi.sh
with the reference tag specified as 20.11, which exists in the dpdk.org tree,
but may not have been pulled into their local copy yet.
I'm not saying this is definately what happened, but it would explain the
reported observations.
Neil
next prev parent reply other threads:[~2020-04-22 12:01 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-16 14:54 Neil Horman
2020-04-17 10:11 ` Ray Kinsella
2020-04-17 10:20 ` Thomas Monjalon
2020-04-17 10:35 ` Ray Kinsella
2020-04-17 11:46 ` Thomas Monjalon
2020-04-17 11:47 ` Ray Kinsella
2020-04-17 12:10 ` Thomas Monjalon
2020-04-17 15:42 ` Ray Kinsella
2020-04-17 16:10 ` Thomas Monjalon
2020-04-20 8:43 ` Ray Kinsella
2020-04-21 11:12 ` Neil Horman
2020-04-21 11:46 ` Thomas Monjalon
2020-04-21 18:56 ` Neil Horman
2020-04-21 21:42 ` Thomas Monjalon
2020-04-22 11:43 ` Ray Kinsella
2020-04-22 12:07 ` Neil Horman
2020-04-22 12:18 ` Thomas Monjalon
2020-04-22 13:19 ` Ray Kinsella
2020-04-22 13:30 ` Thomas Monjalon
2020-04-23 11:03 ` Neil Horman
2020-04-22 12:01 ` Neil Horman [this message]
2020-04-22 12:16 ` Thomas Monjalon
2020-04-23 10:57 ` Neil Horman
2020-05-24 20:34 ` [dpdk-dev] [PATCH v4] devtools: remove old ABI validation script 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=20200422120108.GA864272@hmswarspite.think-freely.org \
--to=nhorman@tuxdriver.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=mdr@ashroe.eu \
--cc=thomas@monjalon.net \
/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).