diff mbox series

[1/3] gitlab: add a CI job for running checkpatch.pl

Message ID 20200918132903.1848939-2-berrange@redhat.com (mailing list archive)
State New, archived
Headers show
Series gitlab: add jobs for checking paches | expand

Commit Message

Daniel P. Berrangé Sept. 18, 2020, 1:29 p.m. UTC 
This job is advisory since it is expected that certain patches will fail
the style checks and checkpatch.pl provides no way to mark exceptions to
the rules.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 .gitlab-ci.d/check-patch.py | 48 +++++++++++++++++++++++++++++++++++++
 .gitlab-ci.yml              | 12 ++++++++++
 2 files changed, 60 insertions(+)
 create mode 100755 .gitlab-ci.d/check-patch.py

Comments

Philippe Mathieu-Daudé Sept. 18, 2020, 1:46 p.m. UTC | #1 
On 9/18/20 3:29 PM, Daniel P. Berrangé wrote:
> This job is advisory since it is expected that certain patches will fail
> the style checks and checkpatch.pl provides no way to mark exceptions to
> the rules.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  .gitlab-ci.d/check-patch.py | 48 +++++++++++++++++++++++++++++++++++++
>  .gitlab-ci.yml              | 12 ++++++++++
>  2 files changed, 60 insertions(+)
>  create mode 100755 .gitlab-ci.d/check-patch.py
> 
> diff --git a/.gitlab-ci.d/check-patch.py b/.gitlab-ci.d/check-patch.py
> new file mode 100755
> index 0000000000..5a14a25b13
> --- /dev/null
> +++ b/.gitlab-ci.d/check-patch.py
> @@ -0,0 +1,48 @@
> +#!/usr/bin/env python3
> +#
> +# check-patch.py: run checkpatch.pl across all commits in a branch
> +#
> +# Copyright (C) 2020 Red Hat, Inc.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import os
> +import os.path
> +import sys
> +import subprocess
> +
> +namespace = "qemu-project"
> +if len(sys.argv) >= 2:
> +    namespace = sys.argv[1]
> +
> +cwd = os.getcwd()
> +reponame = os.path.basename(cwd)
> +repourl = "https://gitlab.com/%s/%s.git" % (namespace, reponame)
> +
> +# GitLab CI environment does not give us any direct info about the
> +# base for the user's branch. We thus need to figure out a common
> +# ancestor between the user's branch and current git master.
> +subprocess.check_call(["git", "remote", "add", "check-patch", repourl])
> +subprocess.check_call(["git", "fetch", "check-patch", "master"],
> +                      stdout=subprocess.DEVNULL,
> +                      stderr=subprocess.DEVNULL)
> +
> +ancestor = subprocess.check_output(["git", "merge-base",
> +                                    "check-patch/master", "HEAD"],
> +                                   universal_newlines=True)
> +
> +ancestor = ancestor.strip()
> +
> +subprocess.check_call(["git", "remote", "rm", "check-patch"])
> +
> +errors = False
> +
> +print("\nChecking all commits since %s...\n" % ancestor)
> +
> +ret = subprocess.run(["scripts/checkpatch.pl", ancestor + "..."])
> +
> +if ret.returncode != 0:
> +    print("    ❌ FAIL one or more commits failed scripts/checkpatch.pl")
> +    sys.exit(1)
> +
> +sys.exit(0)

Hmm I'm very tempted to add various check I've been reluctant to
add to checkpatch.pl here, and use check-patch.py instead...
Thomas Huth Oct. 13, 2020, 8:08 a.m. UTC | #2 
On 18/09/2020 15.29, Daniel P. Berrangé wrote:
> This job is advisory since it is expected that certain patches will fail
> the style checks and checkpatch.pl provides no way to mark exceptions to
> the rules.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  .gitlab-ci.d/check-patch.py | 48 +++++++++++++++++++++++++++++++++++++
>  .gitlab-ci.yml              | 12 ++++++++++
>  2 files changed, 60 insertions(+)
>  create mode 100755 .gitlab-ci.d/check-patch.py
[...]
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index a18e18b57e..3ed724c720 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -369,3 +369,15 @@ check-crypto-only-gnutls:
>    variables:
>      IMAGE: centos7
>      MAKE_CHECK_ARGS: check
> +
> +
> +check-patch:
> +  stage: test

Would it be ok to move this to the "build" stage, so that the job does not
have to wait for all the slow build jobs to finish?
(same question applies for the next patch, too)

If you agree, I can do the change when picking up the patches, no need to
resend just because of this.

 Thomas
Daniel P. Berrangé Oct. 13, 2020, 8:09 a.m. UTC | #3 
On Tue, Oct 13, 2020 at 10:08:56AM +0200, Thomas Huth wrote:
> On 18/09/2020 15.29, Daniel P. Berrangé wrote:
> > This job is advisory since it is expected that certain patches will fail
> > the style checks and checkpatch.pl provides no way to mark exceptions to
> > the rules.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  .gitlab-ci.d/check-patch.py | 48 +++++++++++++++++++++++++++++++++++++
> >  .gitlab-ci.yml              | 12 ++++++++++
> >  2 files changed, 60 insertions(+)
> >  create mode 100755 .gitlab-ci.d/check-patch.py
> [...]
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index a18e18b57e..3ed724c720 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -369,3 +369,15 @@ check-crypto-only-gnutls:
> >    variables:
> >      IMAGE: centos7
> >      MAKE_CHECK_ARGS: check
> > +
> > +
> > +check-patch:
> > +  stage: test
> 
> Would it be ok to move this to the "build" stage, so that the job does not
> have to wait for all the slow build jobs to finish?
> (same question applies for the next patch, too)
> 
> If you agree, I can do the change when picking up the patches, no need to
> resend just because of this.

Sure, fine with me.


Regards,
Daniel
diff mbox series

Patch

diff --git a/.gitlab-ci.d/check-patch.py b/.gitlab-ci.d/check-patch.py
new file mode 100755
index 0000000000..5a14a25b13
--- /dev/null
+++ b/.gitlab-ci.d/check-patch.py
@@ -0,0 +1,48 @@ 
+#!/usr/bin/env python3
+#
+# check-patch.py: run checkpatch.pl across all commits in a branch
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+import os.path
+import sys
+import subprocess
+
+namespace = "qemu-project"
+if len(sys.argv) >= 2:
+    namespace = sys.argv[1]
+
+cwd = os.getcwd()
+reponame = os.path.basename(cwd)
+repourl = "https://gitlab.com/%s/%s.git" % (namespace, reponame)
+
+# GitLab CI environment does not give us any direct info about the
+# base for the user's branch. We thus need to figure out a common
+# ancestor between the user's branch and current git master.
+subprocess.check_call(["git", "remote", "add", "check-patch", repourl])
+subprocess.check_call(["git", "fetch", "check-patch", "master"],
+                      stdout=subprocess.DEVNULL,
+                      stderr=subprocess.DEVNULL)
+
+ancestor = subprocess.check_output(["git", "merge-base",
+                                    "check-patch/master", "HEAD"],
+                                   universal_newlines=True)
+
+ancestor = ancestor.strip()
+
+subprocess.check_call(["git", "remote", "rm", "check-patch"])
+
+errors = False
+
+print("\nChecking all commits since %s...\n" % ancestor)
+
+ret = subprocess.run(["scripts/checkpatch.pl", ancestor + "..."])
+
+if ret.returncode != 0:
+    print("    ❌ FAIL one or more commits failed scripts/checkpatch.pl")
+    sys.exit(1)
+
+sys.exit(0)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index a18e18b57e..3ed724c720 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -369,3 +369,15 @@  check-crypto-only-gnutls:
   variables:
     IMAGE: centos7
     MAKE_CHECK_ARGS: check
+
+
+check-patch:
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/centos8:latest
+  script: .gitlab-ci.d/check-patch.py
+  except:
+    variables:
+      - $CI_PROJECT_NAMESPACE == 'qemu-project' && $CI_COMMIT_BRANCH == 'master'
+  variables:
+    GIT_DEPTH: 1000
+  allow_failure: true