[dpdk-dev,v2] scripts: support any legal git revisions as abi validation range
Commit Message
In addition to git tags, support validating abi between any legal
gitrevisions(7) syntaxes, such as "validate-abi.sh -1 . <target>"
"validate-abi.sh master mybranch <target>" etc in addition to
validating between tags. Makes it easier to run the validator
for in-development work.
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
v2 changes:
- update usage and error messages to match new behavior
- update documentation too (as suggested by John McNamara)
doc/guides/contributing/versioning.rst | 20 ++++++++++++-------
scripts/validate-abi.sh | 36 ++++++++++++++++++++--------------
2 files changed, 34 insertions(+), 22 deletions(-)
Comments
On 12/03/2015 04:05 PM, Panu Matilainen wrote:
> In addition to git tags, support validating abi between any legal
> gitrevisions(7) syntaxes, such as "validate-abi.sh -1 . <target>"
> "validate-abi.sh master mybranch <target>" etc in addition to
> validating between tags. Makes it easier to run the validator
> for in-development work.
>
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> ---
>
> v2 changes:
> - update usage and error messages to match new behavior
> - update documentation too (as suggested by John McNamara)
>
I started wondering why this didn't get applied along with the other
abi-validator changes and noticed this is sitting in patchwork in
"changes requested" state, which doesn't seem right: v2 added the
requested documentation.
The discussion around this patch did spur another request (ability to
pass parallel build flags to make) but that's entirely unrelated, so it
shouldn't hold up this one.
I've no intention of sending a v3 of this patch because AFAIK there's
nothing to fix and the make-flags thing does not belong here, but
resetting the state to "new" by myself feels like cheating or something
:) So what's the correct action here? There's preciously little
documentation about expected patchwork workflow and such.
- Panu -
2015-12-07 16:09, Panu Matilainen:
> On 12/03/2015 04:05 PM, Panu Matilainen wrote:
> > In addition to git tags, support validating abi between any legal
> > gitrevisions(7) syntaxes, such as "validate-abi.sh -1 . <target>"
> > "validate-abi.sh master mybranch <target>" etc in addition to
> > validating between tags. Makes it easier to run the validator
> > for in-development work.
> >
> > Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > ---
> >
> > v2 changes:
> > - update usage and error messages to match new behavior
> > - update documentation too (as suggested by John McNamara)
> >
>
> I started wondering why this didn't get applied along with the other
> abi-validator changes and noticed this is sitting in patchwork in
> "changes requested" state, which doesn't seem right: v2 added the
> requested documentation.
It seems to be an error.
> The discussion around this patch did spur another request (ability to
> pass parallel build flags to make) but that's entirely unrelated, so it
> shouldn't hold up this one.
Yes
> I've no intention of sending a v3 of this patch because AFAIK there's
> nothing to fix and the make-flags thing does not belong here, but
> resetting the state to "new" by myself feels like cheating or something
> :) So what's the correct action here? There's preciously little
> documentation about expected patchwork workflow and such.
It's not cheating.
Changing patchwork status and send such an email looks to be the right thing
to do.
Yes maybe we can improve the contributing guide.
Thanks
On 12/07/2015 04:32 PM, Thomas Monjalon wrote:
> 2015-12-07 16:09, Panu Matilainen:
>> On 12/03/2015 04:05 PM, Panu Matilainen wrote:
>>> In addition to git tags, support validating abi between any legal
>>> gitrevisions(7) syntaxes, such as "validate-abi.sh -1 . <target>"
>>> "validate-abi.sh master mybranch <target>" etc in addition to
>>> validating between tags. Makes it easier to run the validator
>>> for in-development work.
>>>
>>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
>>> Acked-by: Neil Horman <nhorman@tuxdriver.com>
>>> ---
>>>
>>> v2 changes:
>>> - update usage and error messages to match new behavior
>>> - update documentation too (as suggested by John McNamara)
>>>
>>
>> I started wondering why this didn't get applied along with the other
>> abi-validator changes and noticed this is sitting in patchwork in
>> "changes requested" state, which doesn't seem right: v2 added the
>> requested documentation.
>
> It seems to be an error.
>
>> The discussion around this patch did spur another request (ability to
>> pass parallel build flags to make) but that's entirely unrelated, so it
>> shouldn't hold up this one.
>
> Yes
>
>> I've no intention of sending a v3 of this patch because AFAIK there's
>> nothing to fix and the make-flags thing does not belong here, but
>> resetting the state to "new" by myself feels like cheating or something
>> :) So what's the correct action here? There's preciously little
>> documentation about expected patchwork workflow and such.
>
> It's not cheating.
> Changing patchwork status and send such an email looks to be the right thing
> to do.
Ok, done. Thanks for clarifying.
>
> Yes maybe we can improve the contributing guide.
Perhaps this could be used as a base, or referred to (assuming of course
the info is rasonably applicaple to dpdk too)?
https://sourceware.org/glibc/wiki/Patch%20Review%20Workflow
- Panu -
> Thanks
>
2015-12-03 16:05, Panu Matilainen:
> In addition to git tags, support validating abi between any legal
> gitrevisions(7) syntaxes, such as "validate-abi.sh -1 . <target>"
> "validate-abi.sh master mybranch <target>" etc in addition to
> validating between tags. Makes it easier to run the validator
> for in-development work.
>
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks
@@ -468,16 +468,22 @@ utilities which can be installed via a package manager. For example::
The syntax of the ``validate-abi.sh`` utility is::
- ./scripts/validate-abi.sh <TAG1> <TAG2> <TARGET>
+ ./scripts/validate-abi.sh <REV1> <REV2> <TARGET>
-Where ``TAG1`` and ``TAG2`` are valid git tags on the local repo and target is
-the usual DPDK compilation target.
+Where ``REV1`` and ``REV2`` are valid gitrevisions(7)
+https://www.kernel.org/pub/software/scm/git/docs/gitrevisions.html
+on the local repo and target is the usual DPDK compilation target.
-For example to test the current committed HEAD against a previous release tag
-we could add a temporary tag and run the utility as follows::
+For example:
- git tag MY_TEMP_TAG
- ./scripts/validate-abi.sh v2.0.0 MY_TEMP_TAG x86_64-native-linuxapp-gcc
+ # Check between the previous and latest commit:
+ ./scripts/validate-abi.sh HEAD~1 HEAD x86_64-native-linuxapp-gcc
+
+ # Check between two tags:
+ ./scripts/validate-abi.sh v2.0.0 v2.1.0 x86_64-native-linuxapp-gcc
+
+ # Check between git master and local topic-branch "vhost-hacking":
+ ./scripts/validate-abi.sh master vhost-hacking x86_64-native-linuxapp-gcc
After the validation script completes (it can take a while since it need to
compile both tags) it will create compatibility reports in the
@@ -33,7 +33,7 @@ TARGET=$3
ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
usage() {
- echo "$0 <TAG1> <TAG2> <TARGET>"
+ echo "$0 <REV1> <REV2> <TARGET>"
}
log() {
@@ -43,16 +43,15 @@ log() {
}
validate_tags() {
- git tag -l | grep -q "$TAG1"
- if [ $? -ne 0 ]
+
+ if [ -z "$HASH1" ]
then
- echo "$TAG1 is invalid"
+ echo "invalid revision: $TAG1"
return
fi
- git tag -l | grep -q "$TAG2"
- if [ $? -ne 0 ]
+ if [ -z "$HASH2" ]
then
- echo "$TAG2 is invalid"
+ echo "invalid revision: $TAG2"
return
fi
}
@@ -60,12 +59,12 @@ validate_tags() {
validate_args() {
if [ -z "$TAG1" ]
then
- echo "Must Specify TAG1"
+ echo "Must Specify REV1"
return
fi
if [ -z "$TAG2" ]
then
- echo "Must Specify TAG2"
+ echo "Must Specify REV2"
return
fi
if [ -z "$TARGET" ]
@@ -112,6 +111,9 @@ then
cleanup_and_exit 1
fi
+HASH1=$(git show -s --format=%H "$TAG1" -- 2> /dev/null)
+HASH2=$(git show -s --format=%H "$TAG2" -- 2> /dev/null)
+
# Make sure our tags exist
res=$(validate_tags)
if [ -n "$res" ]
@@ -120,6 +122,10 @@ then
cleanup_and_exit 1
fi
+# Make hashes available in output for non-local reference
+TAG1="$TAG1 ($HASH1)"
+TAG2="$TAG2 ($HASH2)"
+
ABICHECK=`which abi-compliance-checker 2>/dev/null`
if [ $? -ne 0 ]
then
@@ -135,8 +141,8 @@ then
fi
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."
+log "INFO" "against DPDK DSOs from version $TAG1 will still run when executed"
+log "INFO" "against DPDK DSOs built from version $TAG2."
log "INFO" ""
# Check to make sure we have a clean tree
@@ -152,7 +158,7 @@ cd $(dirname $0)/..
log "INFO" "Checking out version $TAG1 of the dpdk"
# Move to the old version of the tree
-git checkout $TAG1
+git checkout $HASH1
# Make sure we configure SHARED libraries
# Also turn off IGB and KNI as those require kernel headers to build
@@ -185,7 +191,7 @@ cd $TARGET/lib
log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
for i in `ls *.so`
do
- $ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
+ $ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $HASH1
done
cd ../..
@@ -194,7 +200,7 @@ git clean -f -d
git reset --hard
# Move to the new version of the tree
log "INFO" "Checking out version $TAG2 of the dpdk"
-git checkout $TAG2
+git checkout $HASH2
# Make sure we configure SHARED libraries
# Also turn off IGB and KNI as those require kernel headers to build
@@ -220,7 +226,7 @@ cd $TARGET/lib
log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
for i in `ls *.so`
do
- $ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
+ $ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $HASH2
done
cd ../..