diff mbox series

[ovs-dev] checkpatch: Fix checkpatch's check-authors-file option in CirrusCI.

Message ID 9eed9d81d284231a515e88ecac6dac33c01a4bd1.1728567466.git.echaudro@redhat.com
State Accepted
Commit d1430f3d892fc12c98f74b9c1d95d1c34f02d2ea
Headers show
Series [ovs-dev] checkpatch: Fix checkpatch's check-authors-file option in CirrusCI. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Eelco Chaudron Oct. 10, 2024, 1:37 p.m. UTC
This patch makes sure that if git is missing it's not showing
any errors on the standard output. Secondly the OVS_SRC_DIR
environment variable is used to locate the OVS source directory.

Fixes: a6ccd111552d ("checkpatch: Add new check-authors-file option to checkpatch.py.")
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 tests/checkpatch.at     |  6 ++++--
 utilities/checkpatch.py | 12 +++++++-----
 2 files changed, 11 insertions(+), 7 deletions(-)

Comments

Mike Pattrick Oct. 11, 2024, 3:46 p.m. UTC | #1
On Thu, Oct 10, 2024 at 9:39 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
> This patch makes sure that if git is missing it's not showing
> any errors on the standard output. Secondly the OVS_SRC_DIR
> environment variable is used to locate the OVS source directory.
>
> Fixes: a6ccd111552d ("checkpatch: Add new check-authors-file option to checkpatch.py.")
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>

LGTM!

Acked-by: Mike Pattrick <mkp@redhat.com>

> ---
>  tests/checkpatch.at     |  6 ++++--
>  utilities/checkpatch.py | 12 +++++++-----
>  2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/tests/checkpatch.at b/tests/checkpatch.at
> index fa179c707..2ed2ec878 100755
> --- a/tests/checkpatch.at
> +++ b/tests/checkpatch.at
> @@ -29,11 +29,13 @@ Subject: Patch this is.
>      fi
>
>      if test -s expout; then
> -        AT_CHECK([$PYTHON3 $top_srcdir/utilities/checkpatch.py $3 -q test.patch],
> +        AT_CHECK([OVS_SRC_DIR=$top_srcdir $PYTHON3 \
> +                    $top_srcdir/utilities/checkpatch.py $3 -q test.patch],
>                   [1], [stdout])
>          AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
>      else
> -        AT_CHECK([$PYTHON3 $top_srcdir/utilities/checkpatch.py $3 -q test.patch])
> +        AT_CHECK([OVS_SRC_DIR=$top_srcdir $PYTHON3 \
> +                    $top_srcdir/utilities/checkpatch.py $3 -q test.patch])
>      fi
>  }
>  OVS_END_SHELL_HELPERS
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index 53b13bcf2..fe6aa79b0 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -18,6 +18,7 @@ import email
>  import getopt
>  import os
>  import re
> +import subprocess
>  import sys
>
>  RETURN_CHECK_INITIAL_STATE = 0
> @@ -867,13 +868,14 @@ def run_subject_checks(subject, spellcheck=False):
>
>
>  def get_top_directory():
> -    with os.popen('git rev-parse --show-toplevel') as pipe:
> -        path = pipe.read()
> +    result = subprocess.run('git rev-parse --show-toplevel',
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.DEVNULL, shell=True)
>
> -    if path:
> -        return path.strip()
> +    if result and result.returncode == 0:
> +        return result.stdout.decode('utf-8').strip()
>
> -    return "."
> +    return os.getenv('OVS_SRC_DIR', '.')
>
>
>  def update_missing_authors(diffed_line):
> --
> 2.46.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Kevin Traynor Oct. 11, 2024, 5:30 p.m. UTC | #2
On 10/10/2024 14:37, Eelco Chaudron wrote:
> This patch makes sure that if git is missing it's not showing
> any errors on the standard output. Secondly the OVS_SRC_DIR
> environment variable is used to locate the OVS source directory.
> 
> Fixes: a6ccd111552d ("checkpatch: Add new check-authors-file option to checkpatch.py.")
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>  tests/checkpatch.at     |  6 ++++--
>  utilities/checkpatch.py | 12 +++++++-----
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 

Test passing in CirrusCI with patch:
'24: checkpatch - AUTHORS.rst existence              ok'

Acked-by: Kevin Traynor <ktraynor@redhat.com>
Eelco Chaudron Oct. 14, 2024, 7:39 a.m. UTC | #3
On 11 Oct 2024, at 19:30, Kevin Traynor wrote:

> On 10/10/2024 14:37, Eelco Chaudron wrote:
>> This patch makes sure that if git is missing it's not showing
>> any errors on the standard output. Secondly the OVS_SRC_DIR
>> environment variable is used to locate the OVS source directory.
>>
>> Fixes: a6ccd111552d ("checkpatch: Add new check-authors-file option to checkpatch.py.")
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>>  tests/checkpatch.at     |  6 ++++--
>>  utilities/checkpatch.py | 12 +++++++-----
>>  2 files changed, 11 insertions(+), 7 deletions(-)
>>
>
> Test passing in CirrusCI with patch:
> '24: checkpatch - AUTHORS.rst existence              ok'
>
> Acked-by: Kevin Traynor <ktraynor@redhat.com>

Thanks Mike and Kevin for the review. Applied to main!

//Eelco
diff mbox series

Patch

diff --git a/tests/checkpatch.at b/tests/checkpatch.at
index fa179c707..2ed2ec878 100755
--- a/tests/checkpatch.at
+++ b/tests/checkpatch.at
@@ -29,11 +29,13 @@  Subject: Patch this is.
     fi
 
     if test -s expout; then
-        AT_CHECK([$PYTHON3 $top_srcdir/utilities/checkpatch.py $3 -q test.patch],
+        AT_CHECK([OVS_SRC_DIR=$top_srcdir $PYTHON3 \
+                    $top_srcdir/utilities/checkpatch.py $3 -q test.patch],
                  [1], [stdout])
         AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
     else
-        AT_CHECK([$PYTHON3 $top_srcdir/utilities/checkpatch.py $3 -q test.patch])
+        AT_CHECK([OVS_SRC_DIR=$top_srcdir $PYTHON3 \
+                    $top_srcdir/utilities/checkpatch.py $3 -q test.patch])
     fi
 }
 OVS_END_SHELL_HELPERS
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 53b13bcf2..fe6aa79b0 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -18,6 +18,7 @@  import email
 import getopt
 import os
 import re
+import subprocess
 import sys
 
 RETURN_CHECK_INITIAL_STATE = 0
@@ -867,13 +868,14 @@  def run_subject_checks(subject, spellcheck=False):
 
 
 def get_top_directory():
-    with os.popen('git rev-parse --show-toplevel') as pipe:
-        path = pipe.read()
+    result = subprocess.run('git rev-parse --show-toplevel',
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.DEVNULL, shell=True)
 
-    if path:
-        return path.strip()
+    if result and result.returncode == 0:
+        return result.stdout.decode('utf-8').strip()
 
-    return "."
+    return os.getenv('OVS_SRC_DIR', '.')
 
 
 def update_missing_authors(diffed_line):