[{"id":29950,"web_url":"https://patchwork.libcamera.org/comment/29950/","msgid":"<171838095010.2248009.8508047212641481447@ping.linuxembedded.co.uk>","date":"2024-06-14T16:02:30","subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-06-14 16:57:18)\n> Wrap the existing libcamera pre-push hook into a lint task to make sure\n> that the rules we apply to merging are conveyed as part of the CI jobs.\n> \n\nA couple of test/examples:\n\nSuccessful pipeline: https://gitlab.freedesktop.org/kbingham/libcamera/-/pipelines/1201492\nFailed pipeline: https://gitlab.freedesktop.org/kbingham/libcamera/-/pipelines/1201497\n\n--\nKieran\n\n> Signed-off-by: Kieran Bingham \n> ---\n> .gitlab-ci/lint-pre-push.sh | 27 +++++++++++++++++++++++++++\n> gitlab-ci.yml | 16 +++++++++++++++-\n> 2 files changed, 42 insertions(+), 1 deletion(-)\n> create mode 100755 .gitlab-ci/lint-pre-push.sh\n> \n> diff --git a/.gitlab-ci/lint-pre-push.sh b/.gitlab-ci/lint-pre-push.sh\n> new file mode 100755\n> index 000000000000..8a4283cc0f15\n> --- /dev/null\n> +++ b/.gitlab-ci/lint-pre-push.sh\n> @@ -0,0 +1,27 @@\n> +#!/bin/bash\n> +\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +# SPDX-FileCopyrightText: © 2023 Laurent Pinchart \n> +#\n> +# Pre-merge checks\n> +\n> +set -e\n> +\n> +source \"$(dirname \"$0\")/lib.sh\"\n> +\n> +libcamera_premerge() {\n> + echo \"Running Pre-merge checks for $CI_COMMIT_REF_NAME ($base..$CI_COMMIT_SHA)\"\n> +\n> + # Wrap parameters as git would send them to the pre-push hooks directly.\n> + echo \"$CI_COMMIT_REF_NAME $CI_COMMIT_SHA refs/heads/integration/pre-push-lint-test $base\" \\\n> + | ./utils/hooks/pre-push origin \"$CI_DEFAULT_BRANCH\"\n> +}\n> +\n> +base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA)\n> +\n> +if [[ $base == $CI_COMMIT_SHA ]] ; then\n> + echo \"No commit to test, skipping\"\n> + exit 0\n> +fi\n> +\n> +run libcamera_premerge\n> diff --git a/gitlab-ci.yml b/gitlab-ci.yml\n> index 50b81e591458..d9b31ec8259e 100644\n> --- a/gitlab-ci.yml\n> +++ b/gitlab-ci.yml\n> @@ -311,7 +311,7 @@ build-package:cros:\n> dotenv: env\n> \n> # ------------------------------------------------------------------------------\n> -# Lint stage - Run checkstyle.py\n> +# Lint stage - Run checkstyle.py and check merge suitability\n> # ------------------------------------------------------------------------------\n> \n> lint:\n> @@ -330,6 +330,20 @@ lint:\n> script:\n> - $CI_PROJECT_DIR/.gitlab-ci/lint-libcamera.sh\n> \n> +merge-check:\n> + extends:\n> + - .fdo.distribution-image@debian\n> + - .history-jobs\n> + - .libcamera-ci.debian:12\n> + - .libcamera-ci.scripts\n> + stage: lint\n> + needs:\n> + - job: container-debian:12\n> + artifacts: false\n> + script:\n> + - $CI_PROJECT_DIR/.gitlab-ci/lint-pre-push.sh\n> +\n> +\n> # ------------------------------------------------------------------------------\n> # Test stage - Run unit tests and hardware tests\n> # ------------------------------------------------------------------------------\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D8916C3237\n\tfor ;\n\tFri, 14 Jun 2024 16:02:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F02FA65461;\n\tFri, 14 Jun 2024 18:02:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 64AD661A2A\n\tfor ;\n\tFri, 14 Jun 2024 18:02:32 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 771D42E0\n\tfor ;\n\tFri, 14 Jun 2024 18:02:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DOkxE4Iz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718380937;\n\tbh=3CaV6r7mUkY6b5ryZKGABXY9x7KobQIOGRij4Kr6Yos=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=DOkxE4IzWSPI6BXuK1RK37seHfcV0U99rZTCtSyp6FRDZ3qAnLAUjjGysAeC0Zc0W\n\tgFM+IoPijkELxEXkxO/DQkY2zgdb8kUL8ASlQpzK6TuwosbnfC7omm+oJ7IR1/2UQc\n\tkWUAmnyizeQReHPEtsAq+YmCdwzlG8PQ83wnjg8w=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240614155718.10902-1-kieran.bingham@ideasonboard.com>","References":"<20240614155718.10902-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","From":"Kieran Bingham ","Cc":"","To":"libcamera devel ","Date":"Fri, 14 Jun 2024 17:02:30 +0100","Message-ID":"<171838095010.2248009.8508047212641481447@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29951,"web_url":"https://patchwork.libcamera.org/comment/29951/","msgid":"<20240614182420.GC9171@pendragon.ideasonboard.com>","date":"2024-06-14T18:24:20","subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Fri, Jun 14, 2024 at 04:57:18PM +0100, Kieran Bingham wrote:\n> Wrap the existing libcamera pre-push hook into a lint task to make sure\n> that the rules we apply to merging are conveyed as part of the CI jobs.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> .gitlab-ci/lint-pre-push.sh | 27 +++++++++++++++++++++++++++\n> gitlab-ci.yml | 16 +++++++++++++++-\n> 2 files changed, 42 insertions(+), 1 deletion(-)\n> create mode 100755 .gitlab-ci/lint-pre-push.sh\n> \n> diff --git a/.gitlab-ci/lint-pre-push.sh b/.gitlab-ci/lint-pre-push.sh\n> new file mode 100755\n> index 000000000000..8a4283cc0f15\n> --- /dev/null\n> +++ b/.gitlab-ci/lint-pre-push.sh\n> @@ -0,0 +1,27 @@\n> +#!/bin/bash\n> +\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +# SPDX-FileCopyrightText: © 2023 Laurent Pinchart \n> +#\n> +# Pre-merge checks\n> +\n> +set -e\n> +\n> +source \"$(dirname \"$0\")/lib.sh\"\n> +\n> +libcamera_premerge() {\n> +\techo \"Running Pre-merge checks for $CI_COMMIT_REF_NAME ($base..$CI_COMMIT_SHA)\"\n> +\n> +\t# Wrap parameters as git would send them to the pre-push hooks directly.\n> +\techo \"$CI_COMMIT_REF_NAME $CI_COMMIT_SHA refs/heads/integration/pre-push-lint-test $base\" \\\n> +\t\t| ./utils/hooks/pre-push origin \"$CI_DEFAULT_BRANCH\"\n> +}\n> +\n> +base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA)\n> +\n> +if [[ $base == $CI_COMMIT_SHA ]] ; then\n> +\techo \"No commit to test, skipping\"\n> +\texit 0\n> +fi\n> +\n> +run libcamera_premerge\n> diff --git a/gitlab-ci.yml b/gitlab-ci.yml\n> index 50b81e591458..d9b31ec8259e 100644\n> --- a/gitlab-ci.yml\n> +++ b/gitlab-ci.yml\n> @@ -311,7 +311,7 @@ build-package:cros:\n> dotenv: env\n> \n> # ------------------------------------------------------------------------------\n> -# Lint stage - Run checkstyle.py\n> +# Lint stage - Run checkstyle.py and check merge suitability\n> # ------------------------------------------------------------------------------\n> \n> lint:\n> @@ -330,6 +330,20 @@ lint:\n> script:\n> - $CI_PROJECT_DIR/.gitlab-ci/lint-libcamera.sh\n> \n> +merge-check:\n> + extends:\n> + - .fdo.distribution-image@debian\n> + - .history-jobs\n> + - .libcamera-ci.debian:12\n> + - .libcamera-ci.scripts\n> + stage: lint\n> + needs:\n> + - job: container-debian:12\n> + artifacts: false\n> + script:\n> + - $CI_PROJECT_DIR/.gitlab-ci/lint-pre-push.sh\n\nMaking this a hard failure means that we'll have to allow merging\nbranches that fail CI in some cases (I'm thinking for instance of urgent\nfixes to the master branch that can't get a review in time). For now\nthat's not a problem as CI doesn't gate merging. If that changes in the\nfuture we'll have to either make sure we can merge with failed CI\npipelines, or update this check someone. That's an issue with can deal\nwith later, so\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +\n> # ------------------------------------------------------------------------------\n> # Test stage - Run unit tests and hardware tests\n> # ------------------------------------------------------------------------------","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C2282BD87C\n\tfor ;\n\tFri, 14 Jun 2024 18:24:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BEA486548D;\n\tFri, 14 Jun 2024 20:24:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 34FBC61A2A\n\tfor ;\n\tFri, 14 Jun 2024 20:24:41 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 09DB4316;\n\tFri, 14 Jun 2024 20:24:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PKiItYK0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718389466;\n\tbh=ymaPVbG5N6IbSX+FkaOOfJLaRTCm8qvESQmDEESNdu0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PKiItYK0PLoC8GnK0SfiOpeFxkLpJhwvUCeAde2XmUiROALpPcMYuIMjQCgb2tfmd\n\tKw+OlqGEr6PKI68+R+3/5UPj27kGN9sB/ZqxVwRg2rdalPGoNESIaiVb3DMJZ95enX\n\t8/ElCs+9gO3SlcN3+gzAQ+VTvzMPOR8aMT8J3QEI=","Date":"Fri, 14 Jun 2024 21:24:20 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera devel ","Subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","Message-ID":"<20240614182420.GC9171@pendragon.ideasonboard.com>","References":"<20240614155718.10902-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240614155718.10902-1-kieran.bingham@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29953,"web_url":"https://patchwork.libcamera.org/comment/29953/","msgid":"<171845610235.1292894.5260303427480712043@ping.linuxembedded.co.uk>","date":"2024-06-15T12:55:02","subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-06-14 19:24:20)\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Fri, Jun 14, 2024 at 04:57:18PM +0100, Kieran Bingham wrote:\n> > Wrap the existing libcamera pre-push hook into a lint task to make sure\n> > that the rules we apply to merging are conveyed as part of the CI jobs.\n> > \n> > Signed-off-by: Kieran Bingham \n> > ---\n> > .gitlab-ci/lint-pre-push.sh | 27 +++++++++++++++++++++++++++\n> > gitlab-ci.yml | 16 +++++++++++++++-\n> > 2 files changed, 42 insertions(+), 1 deletion(-)\n> > create mode 100755 .gitlab-ci/lint-pre-push.sh\n> > \n> > diff --git a/.gitlab-ci/lint-pre-push.sh b/.gitlab-ci/lint-pre-push.sh\n> > new file mode 100755\n> > index 000000000000..8a4283cc0f15\n> > --- /dev/null\n> > +++ b/.gitlab-ci/lint-pre-push.sh\n> > @@ -0,0 +1,27 @@\n> > +#!/bin/bash\n> > +\n> > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > +# SPDX-FileCopyrightText: © 2023 Laurent Pinchart \n> > +#\n> > +# Pre-merge checks\n> > +\n> > +set -e\n> > +\n> > +source \"$(dirname \"$0\")/lib.sh\"\n> > +\n> > +libcamera_premerge() {\n> > + echo \"Running Pre-merge checks for $CI_COMMIT_REF_NAME ($base..$CI_COMMIT_SHA)\"\n> > +\n> > + # Wrap parameters as git would send them to the pre-push hooks directly.\n> > + echo \"$CI_COMMIT_REF_NAME $CI_COMMIT_SHA refs/heads/integration/pre-push-lint-test $base\" \\\n> > + | ./utils/hooks/pre-push origin \"$CI_DEFAULT_BRANCH\"\n> > +}\n> > +\n> > +base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA)\n> > +\n> > +if [[ $base == $CI_COMMIT_SHA ]] ; then\n> > + echo \"No commit to test, skipping\"\n> > + exit 0\n> > +fi\n> > +\n> > +run libcamera_premerge\n> > diff --git a/gitlab-ci.yml b/gitlab-ci.yml\n> > index 50b81e591458..d9b31ec8259e 100644\n> > --- a/gitlab-ci.yml\n> > +++ b/gitlab-ci.yml\n> > @@ -311,7 +311,7 @@ build-package:cros:\n> > dotenv: env\n> > \n> > # ------------------------------------------------------------------------------\n> > -# Lint stage - Run checkstyle.py\n> > +# Lint stage - Run checkstyle.py and check merge suitability\n> > # ------------------------------------------------------------------------------\n> > \n> > lint:\n> > @@ -330,6 +330,20 @@ lint:\n> > script:\n> > - $CI_PROJECT_DIR/.gitlab-ci/lint-libcamera.sh\n> > \n> > +merge-check:\n> > + extends:\n> > + - .fdo.distribution-image@debian\n> > + - .history-jobs\n> > + - .libcamera-ci.debian:12\n> > + - .libcamera-ci.scripts\n> > + stage: lint\n> > + needs:\n> > + - job: container-debian:12\n> > + artifacts: false\n> > + script:\n> > + - $CI_PROJECT_DIR/.gitlab-ci/lint-pre-push.sh\n> \n> Making this a hard failure means that we'll have to allow merging\n> branches that fail CI in some cases (I'm thinking for instance of urgent\n> fixes to the master branch that can't get a review in time). For now\n\nI think if one of us is merging a patch without a review, they can\nsupply an 'Acked-by: tag' to acknowledge that responsibility.\n\nThat's how I get around the merge rule on release commits which I don't\nget review tags for.\n\nI can't imagine anything really being 'so urgent' it can't get an 'Ok\nsure - go ahead and merge that'. We don't have hard-realtime constraints\non merging.\n\n> that's not a problem as CI doesn't gate merging. If that changes in the\n> future we'll have to either make sure we can merge with failed CI\n> pipelines, or update this check someone. That's an issue with can deal\n> with later, so\n\nYes, I think this is something we can consider if it comes up. And at\nthe moment, I think the likelyhood should be low - because the whole\npoint of the CI is to make the likelyhood lower that we merge anything\nunsuitable in the first place.\n\n\nAnd this specific instance can be resolved with an:\n\nAcked-by: Author , which even publicly records that the\naction was taken to bypass a check.\n\n> Reviewed-by: Laurent Pinchart \n\nThanks.\n\nKieran\n\n\n> \n> > +\n> > +\n> > # ------------------------------------------------------------------------------\n> > # Test stage - Run unit tests and hardware tests\n> > # ------------------------------------------------------------------------------\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A72B0BD87C\n\tfor ;\n\tSat, 15 Jun 2024 12:55:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2EE565458;\n\tSat, 15 Jun 2024 14:55:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7212065458\n\tfor ;\n\tSat, 15 Jun 2024 14:55:05 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C67D12E0;\n\tSat, 15 Jun 2024 14:54:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"h8CkpAW9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718456089;\n\tbh=SO0RPhv+OrZBE4aqkto0hdrxez7tkx+qlu2H0+uoDRk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=h8CkpAW9RC3MHVVoO1xg8aW+JSSJD6NIDNXnvklYKHousGs8F1QzDRG4Xn9TRpwwu\n\tOzJ3/7RLM4W031uP5sfds224BYor9GWG4VIAaDl0qnI/UFliYtyoGfewfazAn9VhSS\n\th/uxYr5Zq/8IxUmO1+gCr63XTKULZ/0iGWYKS4Wc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240614182420.GC9171@pendragon.ideasonboard.com>","References":"<20240614155718.10902-1-kieran.bingham@ideasonboard.com>\n\t<20240614182420.GC9171@pendragon.ideasonboard.com>","Subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","From":"Kieran Bingham ","Cc":"libcamera devel ","To":"Laurent Pinchart ","Date":"Sat, 15 Jun 2024 13:55:02 +0100","Message-ID":"<171845610235.1292894.5260303427480712043@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29954,"web_url":"https://patchwork.libcamera.org/comment/29954/","msgid":"<20240615134818.GA22903@pendragon.ideasonboard.com>","date":"2024-06-15T13:48:18","subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sat, Jun 15, 2024 at 01:55:02PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2024-06-14 19:24:20)\n> > Hi Kieran,\n> > \n> > Thank you for the patch.\n> > \n> > On Fri, Jun 14, 2024 at 04:57:18PM +0100, Kieran Bingham wrote:\n> > > Wrap the existing libcamera pre-push hook into a lint task to make sure\n> > > that the rules we apply to merging are conveyed as part of the CI jobs.\n> > > \n> > > Signed-off-by: Kieran Bingham \n> > > ---\n> > > .gitlab-ci/lint-pre-push.sh | 27 +++++++++++++++++++++++++++\n> > > gitlab-ci.yml | 16 +++++++++++++++-\n> > > 2 files changed, 42 insertions(+), 1 deletion(-)\n> > > create mode 100755 .gitlab-ci/lint-pre-push.sh\n> > > \n> > > diff --git a/.gitlab-ci/lint-pre-push.sh b/.gitlab-ci/lint-pre-push.sh\n> > > new file mode 100755\n> > > index 000000000000..8a4283cc0f15\n> > > --- /dev/null\n> > > +++ b/.gitlab-ci/lint-pre-push.sh\n> > > @@ -0,0 +1,27 @@\n> > > +#!/bin/bash\n> > > +\n> > > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > > +# SPDX-FileCopyrightText: © 2023 Laurent Pinchart \n> > > +#\n> > > +# Pre-merge checks\n> > > +\n> > > +set -e\n> > > +\n> > > +source \"$(dirname \"$0\")/lib.sh\"\n> > > +\n> > > +libcamera_premerge() {\n> > > + echo \"Running Pre-merge checks for $CI_COMMIT_REF_NAME ($base..$CI_COMMIT_SHA)\"\n> > > +\n> > > + # Wrap parameters as git would send them to the pre-push hooks directly.\n> > > + echo \"$CI_COMMIT_REF_NAME $CI_COMMIT_SHA refs/heads/integration/pre-push-lint-test $base\" \\\n> > > + | ./utils/hooks/pre-push origin \"$CI_DEFAULT_BRANCH\"\n> > > +}\n> > > +\n> > > +base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA)\n> > > +\n> > > +if [[ $base == $CI_COMMIT_SHA ]] ; then\n> > > + echo \"No commit to test, skipping\"\n> > > + exit 0\n> > > +fi\n> > > +\n> > > +run libcamera_premerge\n> > > diff --git a/gitlab-ci.yml b/gitlab-ci.yml\n> > > index 50b81e591458..d9b31ec8259e 100644\n> > > --- a/gitlab-ci.yml\n> > > +++ b/gitlab-ci.yml\n> > > @@ -311,7 +311,7 @@ build-package:cros:\n> > > dotenv: env\n> > > \n> > > # ------------------------------------------------------------------------------\n> > > -# Lint stage - Run checkstyle.py\n> > > +# Lint stage - Run checkstyle.py and check merge suitability\n> > > # ------------------------------------------------------------------------------\n> > > \n> > > lint:\n> > > @@ -330,6 +330,20 @@ lint:\n> > > script:\n> > > - $CI_PROJECT_DIR/.gitlab-ci/lint-libcamera.sh\n> > > \n> > > +merge-check:\n> > > + extends:\n> > > + - .fdo.distribution-image@debian\n> > > + - .history-jobs\n> > > + - .libcamera-ci.debian:12\n> > > + - .libcamera-ci.scripts\n> > > + stage: lint\n> > > + needs:\n> > > + - job: container-debian:12\n> > > + artifacts: false\n> > > + script:\n> > > + - $CI_PROJECT_DIR/.gitlab-ci/lint-pre-push.sh\n> > \n> > Making this a hard failure means that we'll have to allow merging\n> > branches that fail CI in some cases (I'm thinking for instance of urgent\n> > fixes to the master branch that can't get a review in time). For now\n> \n> I think if one of us is merging a patch without a review, they can\n> supply an 'Acked-by: tag' to acknowledge that responsibility.\n> \n> That's how I get around the merge rule on release commits which I don't\n> get review tags for.\n> \n> I can't imagine anything really being 'so urgent' it can't get an 'Ok\n> sure - go ahead and merge that'. We don't have hard-realtime constraints\n> on merging.\n> \n> > that's not a problem as CI doesn't gate merging. If that changes in the\n> > future we'll have to either make sure we can merge with failed CI\n> > pipelines, or update this check someone. That's an issue with can deal\n> > with later, so\n> \n> Yes, I think this is something we can consider if it comes up. And at\n> the moment, I think the likelyhood should be low - because the whole\n> point of the CI is to make the likelyhood lower that we merge anything\n> unsuitable in the first place.\n> \n> \n> And this specific instance can be resolved with an:\n> \n> Acked-by: Author , which even publicly records that the\n> action was taken to bypass a check.\n\nI like that idea.\n\n> > Reviewed-by: Laurent Pinchart \n> \n> > > +\n> > > +\n> > > # ------------------------------------------------------------------------------\n> > > # Test stage - Run unit tests and hardware tests\n> > > # ------------------------------------------------------------------------------","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2D50DC3237\n\tfor ;\n\tSat, 15 Jun 2024 13:48:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 43ADB65489;\n\tSat, 15 Jun 2024 15:48:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24FDF65458\n\tfor ;\n\tSat, 15 Jun 2024 15:48:40 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6E949183;\n\tSat, 15 Jun 2024 15:48:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VlTeUUsD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718459304;\n\tbh=RuStJZeDK8QIfYQ9P1IspCzC8e0gz0znhuVYgymYiyE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VlTeUUsDWLIQjZkZ7apw9/G7hkPuo0gdd9yjkTSdbqT4TCbk78LFTQDrS8XQZ5wlc\n\txpVc4jDm3Cn7Df6a7pzC/Q2tZ9pJSLGv7NJhdomnM8OjWOAUZ3d7sMQOAzCeRup/ax\n\tW0XjF9wwi+ldpplyP40A8OBEUvKyl1wR/yX5ZXXQ=","Date":"Sat, 15 Jun 2024 16:48:18 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera devel ","Subject":"Re: [PATCH] lint: Add pre-merge checks using pre-push hook","Message-ID":"<20240615134818.GA22903@pendragon.ideasonboard.com>","References":"<20240614155718.10902-1-kieran.bingham@ideasonboard.com>\n\t<20240614182420.GC9171@pendragon.ideasonboard.com>\n\t<171845610235.1292894.5260303427480712043@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<171845610235.1292894.5260303427480712043@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]