| 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 |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
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 >
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>
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 --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):
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(-)