DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
@ 2015-12-02 16:50 Panu Matilainen
  2015-12-02 18:23 ` Neil Horman
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Panu Matilainen @ 2015-12-02 16:50 UTC (permalink / raw)
  To: dev

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 mybrach <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>
---
 scripts/validate-abi.sh | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/scripts/validate-abi.sh b/scripts/validate-abi.sh
index 4476433..0e3ccd7 100755
--- a/scripts/validate-abi.sh
+++ b/scripts/validate-abi.sh
@@ -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
 }
@@ -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
@@ -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 ../..
 
-- 
2.5.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-02 16:50 [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range Panu Matilainen
@ 2015-12-02 18:23 ` Neil Horman
  2015-12-03 12:14 ` Mcnamara, John
  2015-12-03 14:05 ` [dpdk-dev] [PATCH v2] " Panu Matilainen
  2 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2015-12-02 18:23 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: dev

On Wed, Dec 02, 2015 at 06:50:47PM +0200, 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 mybrach <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>

> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-02 16:50 [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range Panu Matilainen
  2015-12-02 18:23 ` Neil Horman
@ 2015-12-03 12:14 ` Mcnamara, John
  2015-12-03 13:28   ` Thomas Monjalon
  2015-12-03 13:39   ` Panu Matilainen
  2015-12-03 14:05 ` [dpdk-dev] [PATCH v2] " Panu Matilainen
  2 siblings, 2 replies; 14+ messages in thread
From: Mcnamara, John @ 2015-12-03 12:14 UTC (permalink / raw)
  To: Panu Matilainen, dev

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
> Sent: Wednesday, December 2, 2015 4:51 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] scripts: support any legal git revisions as
> abi validation range
> 
> 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 mybrach <target>" etc in addition to validating
> between tags. Makes it easier to run the validator for in-development
> work.

Hi Panu,

+1 for this.

You might also change the ABI validation section of the docs to go along
with this. Something like the patch below. If not I'll submit it
afterwards. 

Also, if someone has some bandwidth it would be good to add an option
to pass -j with an optional number to "make" in the script.

John


$ git diff               
diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
index 653c7d0..5ce5f9d 100644
--- a/doc/guides/contributing/versioning.rst
+++ b/doc/guides/contributing/versioning.rst
@@ -465,19 +465,23 @@ utilities which can be installed via a package manager. For example::
 
    sudo yum install abi-compliance-checker
    sudo yum install abi-dumper
+   sudo yum install greadelf
 
 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 commits:
+   ./scripts/validate-abi.sh HEAD~1 HEAD x86_64-native-linuxapp-gcc
+
+   # Check between two tags:
+   ./scripts/validate-abi.sh v2.1.0 v2.2.0 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-03 12:14 ` Mcnamara, John
@ 2015-12-03 13:28   ` Thomas Monjalon
  2015-12-03 13:44     ` Panu Matilainen
  2015-12-03 13:39   ` Panu Matilainen
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2015-12-03 13:28 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev

2015-12-03 12:14, Mcnamara, John:
> Also, if someone has some bandwidth it would be good to add an option
> to pass -j with an optional number to "make" in the script.

We can use -j without any number:
"make will not limit the number of jobs that can run simultaneously".
It is a not so bad default.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-03 12:14 ` Mcnamara, John
  2015-12-03 13:28   ` Thomas Monjalon
@ 2015-12-03 13:39   ` Panu Matilainen
  2015-12-03 13:41     ` Thomas Monjalon
  1 sibling, 1 reply; 14+ messages in thread
From: Panu Matilainen @ 2015-12-03 13:39 UTC (permalink / raw)
  To: Mcnamara, John, dev

On 12/03/2015 02:14 PM, Mcnamara, John wrote:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
>> Sent: Wednesday, December 2, 2015 4:51 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] scripts: support any legal git revisions as
>> abi validation range
>>
>> 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 mybrach <target>" etc in addition to validating
>> between tags. Makes it easier to run the validator for in-development
>> work.
>
> Hi Panu,
>
> +1 for this.
>
> You might also change the ABI validation section of the docs to go along
> with this. Something like the patch below. If not I'll submit it
> afterwards.

Good points, including changing the usage message to REV instead of TAG. 
I'll send an improved version based on this, thanks.

>
> Also, if someone has some bandwidth it would be good to add an option
> to pass -j with an optional number to "make" in the script.

Can do, although I'm still waiting fo my previous, semi-related 
validate-abi patches from September to be applied...

	- Panu -

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-03 13:39   ` Panu Matilainen
@ 2015-12-03 13:41     ` Thomas Monjalon
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2015-12-03 13:41 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: dev

2015-12-03 15:39, Panu Matilainen:
> > Also, if someone has some bandwidth it would be good to add an option
> > to pass -j with an optional number to "make" in the script.
> 
> Can do, although I'm still waiting fo my previous, semi-related 
> validate-abi patches from September to be applied...

Oh yes, sorry. I will apply it shortly.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-03 13:28   ` Thomas Monjalon
@ 2015-12-03 13:44     ` Panu Matilainen
  2015-12-03 13:49       ` Richardson, Bruce
  2015-12-03 15:46       ` Mcnamara, John
  0 siblings, 2 replies; 14+ messages in thread
From: Panu Matilainen @ 2015-12-03 13:44 UTC (permalink / raw)
  To: Thomas Monjalon, Mcnamara, John; +Cc: dev

On 12/03/2015 03:28 PM, Thomas Monjalon wrote:
> 2015-12-03 12:14, Mcnamara, John:
>> Also, if someone has some bandwidth it would be good to add an option
>> to pass -j with an optional number to "make" in the script.
>
> We can use -j without any number:
> "make will not limit the number of jobs that can run simultaneously".
> It is a not so bad default.
>

Hmm, my memory associates an unlimited -j to make with sudden death by 
fork-bomb. But that was a long time ago and on a different codebase (the 
kernel maybe), with DPDK on current hardware it doesn't seem that bad at 
all.

OTOH we can also simply ask the system for a reasonable value, eg
$ /usr/bin/getconf _NPROCESSORS_ONLN

	- Panu -

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-03 13:44     ` Panu Matilainen
@ 2015-12-03 13:49       ` Richardson, Bruce
  2015-12-03 15:46       ` Mcnamara, John
  1 sibling, 0 replies; 14+ messages in thread
From: Richardson, Bruce @ 2015-12-03 13:49 UTC (permalink / raw)
  To: Panu Matilainen, Thomas Monjalon, Mcnamara, John; +Cc: dev

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
> Sent: Thursday, December 3, 2015 1:44 PM
> To: Thomas Monjalon <thomas.monjalon@6wind.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions
> as abi validation range
> 
> On 12/03/2015 03:28 PM, Thomas Monjalon wrote:
> > 2015-12-03 12:14, Mcnamara, John:
> >> Also, if someone has some bandwidth it would be good to add an option
> >> to pass -j with an optional number to "make" in the script.
> >
> > We can use -j without any number:
> > "make will not limit the number of jobs that can run simultaneously".
> > It is a not so bad default.
> >
> 
> Hmm, my memory associates an unlimited -j to make with sudden death by
> fork-bomb. But that was a long time ago and on a different codebase (the
> kernel maybe), with DPDK on current hardware it doesn't seem that bad at
> all.
> 
> OTOH we can also simply ask the system for a reasonable value, eg $
> /usr/bin/getconf _NPROCESSORS_ONLN
> 
> 	- Panu -

+1 to this.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2] scripts: support any legal git revisions as abi validation range
  2015-12-02 16:50 [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range Panu Matilainen
  2015-12-02 18:23 ` Neil Horman
  2015-12-03 12:14 ` Mcnamara, John
@ 2015-12-03 14:05 ` Panu Matilainen
  2015-12-07 14:09   ` Panu Matilainen
  2015-12-07 22:38   ` Thomas Monjalon
  2 siblings, 2 replies; 14+ messages in thread
From: Panu Matilainen @ 2015-12-03 14:05 UTC (permalink / raw)
  To: dev

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(-)

diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
index 653c7d0..015ebb7 100644
--- a/doc/guides/contributing/versioning.rst
+++ b/doc/guides/contributing/versioning.rst
@@ -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
diff --git a/scripts/validate-abi.sh b/scripts/validate-abi.sh
index 4476433..e49c425 100755
--- a/scripts/validate-abi.sh
+++ b/scripts/validate-abi.sh
@@ -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 ../..
 
-- 
2.5.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range
  2015-12-03 13:44     ` Panu Matilainen
  2015-12-03 13:49       ` Richardson, Bruce
@ 2015-12-03 15:46       ` Mcnamara, John
  1 sibling, 0 replies; 14+ messages in thread
From: Mcnamara, John @ 2015-12-03 15:46 UTC (permalink / raw)
  To: Panu Matilainen, Thomas Monjalon; +Cc: dev



> -----Original Message-----
> From: Panu Matilainen [mailto:pmatilai@redhat.com]
> Sent: Thursday, December 3, 2015 1:44 PM
> To: Thomas Monjalon; Mcnamara, John
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] scripts: support any legal git revisions
> as abi validation range
> 
> On 12/03/2015 03:28 PM, Thomas Monjalon wrote:
> > 2015-12-03 12:14, Mcnamara, John:
> >> Also, if someone has some bandwidth it would be good to add an option
> >> to pass -j with an optional number to "make" in the script.
> >
> > We can use -j without any number:
> > "make will not limit the number of jobs that can run simultaneously".
> > It is a not so bad default.
> >
> 
> Hmm, my memory associates an unlimited -j to make with sudden death by
> fork-bomb. But that was a long time ago and on a different codebase (the
> kernel maybe), with DPDK on current hardware it doesn't seem that bad at
> all.
> 
> OTOH we can also simply ask the system for a reasonable value, eg $
> /usr/bin/getconf _NPROCESSORS_ONLN

Hi,

To be clear, it would be nice to be able to pass "-j" or "-j n" like you can do to directly to 'make'. 

For a compile heavy codebase (no DPDK) "-j" can, as you say, choke your system but that would be up to the user to specify, or not. 

John.
-- 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] scripts: support any legal git revisions as abi validation range
  2015-12-03 14:05 ` [dpdk-dev] [PATCH v2] " Panu Matilainen
@ 2015-12-07 14:09   ` Panu Matilainen
  2015-12-07 14:32     ` Thomas Monjalon
  2015-12-07 22:38   ` Thomas Monjalon
  1 sibling, 1 reply; 14+ messages in thread
From: Panu Matilainen @ 2015-12-07 14:09 UTC (permalink / raw)
  To: dev

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 -

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] scripts: support any legal git revisions as abi validation range
  2015-12-07 14:09   ` Panu Matilainen
@ 2015-12-07 14:32     ` Thomas Monjalon
  2015-12-07 16:08       ` Panu Matilainen
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2015-12-07 14:32 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: dev

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] scripts: support any legal git revisions as abi validation range
  2015-12-07 14:32     ` Thomas Monjalon
@ 2015-12-07 16:08       ` Panu Matilainen
  0 siblings, 0 replies; 14+ messages in thread
From: Panu Matilainen @ 2015-12-07 16:08 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

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
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] scripts: support any legal git revisions as abi validation range
  2015-12-03 14:05 ` [dpdk-dev] [PATCH v2] " Panu Matilainen
  2015-12-07 14:09   ` Panu Matilainen
@ 2015-12-07 22:38   ` Thomas Monjalon
  1 sibling, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2015-12-07 22:38 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: dev

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-12-07 22:39 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-02 16:50 [dpdk-dev] [PATCH] scripts: support any legal git revisions as abi validation range Panu Matilainen
2015-12-02 18:23 ` Neil Horman
2015-12-03 12:14 ` Mcnamara, John
2015-12-03 13:28   ` Thomas Monjalon
2015-12-03 13:44     ` Panu Matilainen
2015-12-03 13:49       ` Richardson, Bruce
2015-12-03 15:46       ` Mcnamara, John
2015-12-03 13:39   ` Panu Matilainen
2015-12-03 13:41     ` Thomas Monjalon
2015-12-03 14:05 ` [dpdk-dev] [PATCH v2] " Panu Matilainen
2015-12-07 14:09   ` Panu Matilainen
2015-12-07 14:32     ` Thomas Monjalon
2015-12-07 16:08       ` Panu Matilainen
2015-12-07 22:38   ` Thomas Monjalon

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).