| Message ID | 20200918132903.1848939-2-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | gitlab: add jobs for checking paches | expand |
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...
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
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 --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
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