[ovs-dev] [PATCH v4 ovn] Add missing checkpatch.py script

2019-08-21 Thread Lorenzo Bianconi 
Add utility checkpatch.py script used to check patch correctness before
submitting it. Introduce checkpatch.at unit tests

Signed-off-by: Lorenzo Bianconi 
---
Changes since v3:
- update checkpatch.py script
Changes since v2:
- add checkpatch.at unit test
Changes since v1:
- fix subject
---
 tests/automake.mk   |1 +
 tests/checkpatch.at |  335 +
 tests/testsuite.at  |1 +
 utilities/automake.mk   |3 +-
 utilities/checkpatch.py | 1006 +++
 5 files changed, 1345 insertions(+), 1 deletion(-)
 create mode 100755 tests/checkpatch.at
 create mode 100755 utilities/checkpatch.py

diff --git a/tests/automake.mk b/tests/automake.mk
index 5411772a4..b19144fe4 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -19,6 +19,7 @@ COMMON_MACROS_AT = \
 
 TESTSUITE_AT = \
tests/testsuite.at \
+   tests/checkpatch.at \
tests/ovn.at \
tests/ovn-northd.at \
tests/ovn-nbctl.at \
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
new file mode 100755
index 0..fe21acdf2
--- /dev/null
+++ b/tests/checkpatch.at
@@ -0,0 +1,335 @@
+AT_BANNER([checkpatch])
+
+OVS_START_SHELL_HELPERS
+# try_checkpatch PATCH [ERRORS]
+#
+# Runs checkpatch under Python 2 and Python 3, if installed, on the given
+# PATCH, expecting the specified set of ERRORS (and warnings).
+try_checkpatch() {
+AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
+# Take the patch to test from $1.  Remove an initial four-space indent
+# from it and, if it is just headers with no body, add a null body.
+echo "$1" | sed 's/^//' > test.patch
+if grep '---' expout >/dev/null 2>&1; then :
+else
+printf '\n---\n' >> test.patch
+fi
+
+# Take expected output from $2.
+if test -n "$2"; then
+echo "$2" | sed 's/^//' > expout
+else
+: > expout
+fi
+
+try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
+try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
+}
+try_checkpatch__() {
+if test $1 = no; then
+:
+elif test -s expout; then
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
+ [1], [stdout])
+AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
+else
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
+fi
+}
+OVS_END_SHELL_HELPERS
+
+AT_SETUP([checkpatch - sign-offs])
+
+# Sign-off for single author who is also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A"
+try_checkpatch \
+   "Author: A
+Commit: A" \
+   "ERROR: Author A needs to sign off."
+
+# Single author but somehow the mailing list is the author.
+try_checkpatch \
+   "Author: Foo Bar via dev 
+Commit: A
+
+Signed-off-by: A" \
+   "ERROR: Author should not be mailing list."
+
+# Sign-off for single author and different committer.
+try_checkpatch \
+   "Author: A
+Commit: B
+
+Signed-off-by: A
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Committer B needs to sign off."
+
+# Sign-off for multiple authors with one author also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Author A needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B" \
+   "ERROR: Co-author B needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Co-author B needs to sign off."
+
+# Sign-off for multiple authors and separate committer.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Committer C needs to sign off."
+
+# Extra sign-offs:
+#
+#- If we know the committer, one extra sign-off raises a warning.
+#
+#- If we do not know the committer, two extra sign-offs raise a warning.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+   "WARNING: Unexpected sign-offs from developers who are not authors or 
co-authors or committers: D"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+   "WARNING: Unexpected sign-offs from developers who are not authors or 
co-authors or committers: C, D"
+
+# Missing committer is O

Re: [ovs-dev] [PATCH v4 ovn] Add missing checkpatch.py script

2019-08-21 Thread Dumitru Ceara 
On Wed, Aug 21, 2019 at 2:10 PM Lorenzo Bianconi
 wrote:
>
> Add utility checkpatch.py script used to check patch correctness before
> submitting it. Introduce checkpatch.at unit tests
>
> Signed-off-by: Lorenzo Bianconi 

Looks good to me. Thanks!

Acked-by: Dumitru Ceara 

> ---
> Changes since v3:
> - update checkpatch.py script
> Changes since v2:
> - add checkpatch.at unit test
> Changes since v1:
> - fix subject
> ---
>  tests/automake.mk   |1 +
>  tests/checkpatch.at |  335 +
>  tests/testsuite.at  |1 +
>  utilities/automake.mk   |3 +-
>  utilities/checkpatch.py | 1006 +++
>  5 files changed, 1345 insertions(+), 1 deletion(-)
>  create mode 100755 tests/checkpatch.at
>  create mode 100755 utilities/checkpatch.py
>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 5411772a4..b19144fe4 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -19,6 +19,7 @@ COMMON_MACROS_AT = \
>
>  TESTSUITE_AT = \
> tests/testsuite.at \
> +   tests/checkpatch.at \
> tests/ovn.at \
> tests/ovn-northd.at \
> tests/ovn-nbctl.at \
> diff --git a/tests/checkpatch.at b/tests/checkpatch.at
> new file mode 100755
> index 0..fe21acdf2
> --- /dev/null
> +++ b/tests/checkpatch.at
> @@ -0,0 +1,335 @@
> +AT_BANNER([checkpatch])
> +
> +OVS_START_SHELL_HELPERS
> +# try_checkpatch PATCH [ERRORS]
> +#
> +# Runs checkpatch under Python 2 and Python 3, if installed, on the given
> +# PATCH, expecting the specified set of ERRORS (and warnings).
> +try_checkpatch() {
> +AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
> +# Take the patch to test from $1.  Remove an initial four-space indent
> +# from it and, if it is just headers with no body, add a null body.
> +echo "$1" | sed 's/^//' > test.patch
> +if grep '---' expout >/dev/null 2>&1; then :
> +else
> +printf '\n---\n' >> test.patch
> +fi
> +
> +# Take expected output from $2.
> +if test -n "$2"; then
> +echo "$2" | sed 's/^//' > expout
> +else
> +: > expout
> +fi
> +
> +try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
> +try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
> +}
> +try_checkpatch__() {
> +if test $1 = no; then
> +:
> +elif test -s expout; then
> +AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
> + [1], [stdout])
> +AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
> +else
> +AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
> +fi
> +}
> +OVS_END_SHELL_HELPERS
> +
> +AT_SETUP([checkpatch - sign-offs])
> +
> +# Sign-off for single author who is also the committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A"
> +try_checkpatch \
> +   "Author: A
> +Commit: A" \
> +   "ERROR: Author A needs to sign off."
> +
> +# Single author but somehow the mailing list is the author.
> +try_checkpatch \
> +   "Author: Foo Bar via dev 
> +Commit: A
> +
> +Signed-off-by: A" \
> +   "ERROR: Author should not be mailing list."
> +
> +# Sign-off for single author and different committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: B
> +
> +Signed-off-by: A
> +Signed-off-by: B"
> +try_checkpatch \
> +   "Author: A
> +Commit: B" \
> +   "ERROR: Author A needs to sign off.
> +ERROR: Committer B needs to sign off."
> +
> +# Sign-off for multiple authors with one author also the committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B"
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Co-authored-by: B
> +Signed-off-by: B" \
> +   "ERROR: Author A needs to sign off."
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A
> +Co-authored-by: B" \
> +   "ERROR: Co-author B needs to sign off."
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Co-authored-by: B" \
> +   "ERROR: Author A needs to sign off.
> +ERROR: Co-author B needs to sign off."
> +
> +# Sign-off for multiple authors and separate committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B
> +Signed-off-by: C"
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B" \
> +   "ERROR: Committer C needs to sign off."
> +
> +# Extra sign-offs:
> +#
> +#- If we know the committer, one extra sign-off raises a warning.
> +#
> +#- If we do not know the committer, two extra sign-offs raise a warning.
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B
> +Signed-off-by: C
> +Signed-off-by: D" \
> +   "WARNING: Unexpected sign-offs from developers

Re: [ovs-dev] [PATCH v4 ovn] Add missing checkpatch.py script

2019-08-21 Thread Mark Michelson 

I pushed this to master. Thanks!

On 8/21/19 8:14 AM, Dumitru Ceara wrote:

On Wed, Aug 21, 2019 at 2:10 PM Lorenzo Bianconi
 wrote:


Add utility checkpatch.py script used to check patch correctness before
submitting it. Introduce checkpatch.at unit tests

Signed-off-by: Lorenzo Bianconi 


Looks good to me. Thanks!

Acked-by: Dumitru Ceara 


---
Changes since v3:
- update checkpatch.py script
Changes since v2:
- add checkpatch.at unit test
Changes since v1:
- fix subject
---
  tests/automake.mk   |1 +
  tests/checkpatch.at |  335 +
  tests/testsuite.at  |1 +
  utilities/automake.mk   |3 +-
  utilities/checkpatch.py | 1006 +++
  5 files changed, 1345 insertions(+), 1 deletion(-)
  create mode 100755 tests/checkpatch.at
  create mode 100755 utilities/checkpatch.py

diff --git a/tests/automake.mk b/tests/automake.mk
index 5411772a4..b19144fe4 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -19,6 +19,7 @@ COMMON_MACROS_AT = \

  TESTSUITE_AT = \
 tests/testsuite.at \
+   tests/checkpatch.at \
 tests/ovn.at \
 tests/ovn-northd.at \
 tests/ovn-nbctl.at \
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
new file mode 100755
index 0..fe21acdf2
--- /dev/null
+++ b/tests/checkpatch.at
@@ -0,0 +1,335 @@
+AT_BANNER([checkpatch])
+
+OVS_START_SHELL_HELPERS
+# try_checkpatch PATCH [ERRORS]
+#
+# Runs checkpatch under Python 2 and Python 3, if installed, on the given
+# PATCH, expecting the specified set of ERRORS (and warnings).
+try_checkpatch() {
+AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
+# Take the patch to test from $1.  Remove an initial four-space indent
+# from it and, if it is just headers with no body, add a null body.
+echo "$1" | sed 's/^//' > test.patch
+if grep '---' expout >/dev/null 2>&1; then :
+else
+printf '\n---\n' >> test.patch
+fi
+
+# Take expected output from $2.
+if test -n "$2"; then
+echo "$2" | sed 's/^//' > expout
+else
+: > expout
+fi
+
+try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
+try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
+}
+try_checkpatch__() {
+if test $1 = no; then
+:
+elif test -s expout; then
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
+ [1], [stdout])
+AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
+else
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
+fi
+}
+OVS_END_SHELL_HELPERS
+
+AT_SETUP([checkpatch - sign-offs])
+
+# Sign-off for single author who is also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A"
+try_checkpatch \
+   "Author: A
+Commit: A" \
+   "ERROR: Author A needs to sign off."
+
+# Single author but somehow the mailing list is the author.
+try_checkpatch \
+   "Author: Foo Bar via dev 
+Commit: A
+
+Signed-off-by: A" \
+   "ERROR: Author should not be mailing list."
+
+# Sign-off for single author and different committer.
+try_checkpatch \
+   "Author: A
+Commit: B
+
+Signed-off-by: A
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Committer B needs to sign off."
+
+# Sign-off for multiple authors with one author also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Author A needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B" \
+   "ERROR: Co-author B needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Co-author B needs to sign off."
+
+# Sign-off for multiple authors and separate committer.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Committer C needs to sign off."
+
+# Extra sign-offs:
+#
+#- If we know the committer, one extra sign-off raises a warning.
+#
+#- If we do not know the committer, two extra sign-offs raise a warning.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+   "WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: D"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: B