[{"id":3488,"web_url":"https://patchwork.libcamera.org/comment/3488/","msgid":"<20200117221115.GI1074550@oden.dyn.berto.se>","date":"2020-01-17T22:11:15","subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nicolas,\n\nThanks for your work.\n\nOn 2020-01-17 14:17:32 -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> \n> This introduce a new argument \"--amend\" and a new special type of\n> commit \"Amendment\". It will check the style of changes that are in\n> the index combined with the changes of the last commit. So this is\n> the changes that would be applied by \"git commit --amend\" hence the\n> name of the argument.\n> \n> This is needed to implement pre-commit hook.\n> \n> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  utils/checkstyle.py | 24 ++++++++++++++++++++++++\n>  1 file changed, 24 insertions(+)\n>  mode change 100644 => 100755 utils/checkstyle.py\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> old mode 100644\n> new mode 100755\n> index 8e456cd..7c2ce00\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -501,6 +501,26 @@ class Index(Commit):\n>                                stdout=subprocess.PIPE).stdout.decode('utf-8')\n>  \n>  \n> +class Amendment(Index):\n> +    def __init__(self):\n> +        Commit.__init__(self, None)\n> +\n> +    def det_info(self, top_level):\n> +        # Create a title using HEAD commit\n> +        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', 'HEAD'],\n> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +        title = 'Amendment of: ' + ret.splitlines()[0]\n> +        # Extract the list of modifier files\n> +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'],\n> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +        return title, ret.splitlines()\n> +\n> +    def get_diff(self, top_level, filename):\n> +        return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',\n> +                               '%s/%s' % (top_level, filename)],\n> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +\n>  def check_file(top_level, commit, filename):\n>      # Extract the line numbers touched by the commit.\n>      diff = commit.get_diff(top_level, filename)\n> @@ -633,6 +653,8 @@ def main(argv):\n>                          help='Code formatter. Default to clang-format if not specified.')\n>      parser.add_argument('--staged', '-s', action='store_true',\n>                          help='Include the changes in the index. Defaults to False')\n> +    parser.add_argument('--amend', '-a', action='store_true',\n> +                        help='Includes changes in the index and the previous patch combined. Defaults to False')\n>      parser.add_argument('revision_range', type=str, default=None, nargs='?',\n>                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')\n>      args = parser.parse_args(argv[1:])\n> @@ -671,6 +693,8 @@ def main(argv):\n>      revlist = []\n>      if args.staged:\n>          revlist.append(Index())\n> +    if args.amend:\n> +        revlist.append(Amendment())\n>  \n>      # If nothing of --staged or --amend was passed, defaults to HEAD\n>      if len(revlist) == 0 and not args.revision_range:\n> -- \n> 2.24.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9746260782\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 23:11:17 +0100 (CET)","by mail-lf1-x143.google.com with SMTP id m30so19452454lfp.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 14:11:17 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\ti4sm15263914lji.0.2020.01.17.14.11.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 17 Jan 2020 14:11:16 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=9yshoXeB/78NR3iqctVFXpk509cXp31FpZe7kAn3VcM=;\n\tb=2FeEdBd6EhRTZX6WIbsl+CJhYMN/K00MoIySt2WKc19e0BXP35ByOCc5Hz5Wq1eml4\n\t9gjBtMfsvPFWvV4K9B2uMsy8Bq1vOGzKwIiAmVjO3OL5npPx3hG49ir3nnam7aopZNvo\n\t6v50u5Kck7qjCz9mEEkTIMSuOQNfcjahUpL8bZk/mea7bjfdHCSOJsYd+mYq3+R0yHXb\n\t5RaN/uaoLoTZ23YEjgA6cmcWwDut6GJxRrD7C5vWmxzd/xKJ7/UPqkvhT/hiAUQPyHgF\n\th8IxEoSOIvF4myhCQHzr1ee+fKX1pCCGuUyiXh/Dt527ZXC2wiw9XEBKhmgUBPT/3FvV\n\tjfnQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=9yshoXeB/78NR3iqctVFXpk509cXp31FpZe7kAn3VcM=;\n\tb=fOWvdeX77Y8D7VGSQKDSY+kcCCjwUBKM9ifXt7Krzj4augIQznm60vzYidfgnWa5mz\n\t8cKSHjLUIhseB6vF/B5x7E24UC+Y6Ww1Zsl7ytRD7Ddwrl7Tp86OdW3HLhGXo62R0/eG\n\t6OAcmYhoIDukI4Dro/rBKd/yv1dljk/42kj8fAlFF/4I8DEWsEUi0Er+6GoOvkcqe+hY\n\tR84UsQFb4WivotvjQ/QvKV73y5baaZkayceGLxZxwDVHua5hABtsvNJmQHPpElNtKElM\n\t3zqHRqOwlNI94ZlFFyBE9QX9wpjotVuaVKGT20Ozfq0nzmm7ncX38xMH9HTxZYSov0nR\n\tXpYA==","X-Gm-Message-State":"APjAAAWqLXgiWaPcuTj7mDUpidVfcYIsew2WIMdCroA3z1KnalNjT+dx\n\tWuuCIKipSkdQHTsn05rxHWnsj/E5I1E=","X-Google-Smtp-Source":"APXvYqw0KYhjz39zgxiyo4JmLucbza3vWU3p8Q4J3ObYhTpY0pDGMWArMl4IjNcDaJTkswUdC6PG/w==","X-Received":"by 2002:ac2:465e:: with SMTP id\n\ts30mr6767757lfo.134.1579299077006; \n\tFri, 17 Jan 2020 14:11:17 -0800 (PST)","Date":"Fri, 17 Jan 2020 23:11:15 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne <nicolas.dufresne@collabora.com>","Message-ID":"<20200117221115.GI1074550@oden.dyn.berto.se>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-6-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200117191733.198897-6-nicolas@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 22:11:17 -0000"}},{"id":3492,"web_url":"https://patchwork.libcamera.org/comment/3492/","msgid":"<20200117222556.GQ5711@pendragon.ideasonboard.com>","date":"2020-01-17T22:25:56","subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the patch.\n\nOn Fri, Jan 17, 2020 at 02:17:32PM -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> \n> This introduce a new argument \"--amend\" and a new special type of\n> commit \"Amendment\". It will check the style of changes that are in\n> the index combined with the changes of the last commit. So this is\n> the changes that would be applied by \"git commit --amend\" hence the\n> name of the argument.\n> \n> This is needed to implement pre-commit hook.\n> \n> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> ---\n>  utils/checkstyle.py | 24 ++++++++++++++++++++++++\n>  1 file changed, 24 insertions(+)\n>  mode change 100644 => 100755 utils/checkstyle.py\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> old mode 100644\n> new mode 100755\n> index 8e456cd..7c2ce00\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -501,6 +501,26 @@ class Index(Commit):\n>                                stdout=subprocess.PIPE).stdout.decode('utf-8')\n>  \n>  \n> +class Amendment(Index):\n> +    def __init__(self):\n> +        Commit.__init__(self, None)\n> +\n> +    def det_info(self, top_level):\n\ns/det_info/get_info/\n\nThis code path has never been tested :-)\n\n> +        # Create a title using HEAD commit\n> +        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', 'HEAD'],\n> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +        title = 'Amendment of: ' + ret.splitlines()[0]\n> +        # Extract the list of modifier files\n> +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'],\n> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +        return title, ret.splitlines()\n> +\n> +    def get_diff(self, top_level, filename):\n> +        return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',\n> +                               '%s/%s' % (top_level, filename)],\n> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +\n>  def check_file(top_level, commit, filename):\n>      # Extract the line numbers touched by the commit.\n>      diff = commit.get_diff(top_level, filename)\n> @@ -633,6 +653,8 @@ def main(argv):\n>                          help='Code formatter. Default to clang-format if not specified.')\n>      parser.add_argument('--staged', '-s', action='store_true',\n>                          help='Include the changes in the index. Defaults to False')\n> +    parser.add_argument('--amend', '-a', action='store_true',\n> +                        help='Includes changes in the index and the previous patch combined. Defaults to False')\n\ns/Includes/Include/\n\n>      parser.add_argument('revision_range', type=str, default=None, nargs='?',\n>                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')\n>      args = parser.parse_args(argv[1:])\n> @@ -671,6 +693,8 @@ def main(argv):\n>      revlist = []\n>      if args.staged:\n>          revlist.append(Index())\n> +    if args.amend:\n> +        revlist.append(Amendment())\n\nI think you'll find out that it won't work correctly if you specify both\n--staged and --amend, as they duplicate each other to some extent.\nShould we at the very least ignore --staged if --amend is set ? Or use a\nsingle parameter that would take two different values so that both can't\nbe set together ?\n\nShould we also ignore the list of commits of --staged or --amend is set\n?\n\n>      # If nothing of --staged or --amend was passed, defaults to HEAD\n>      if len(revlist) == 0 and not args.revision_range:","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E409C60782\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 23:26:11 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6B0C59A1;\n\tFri, 17 Jan 2020 23:26:11 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579299971;\n\tbh=0hCti9ip2/x3lrRTWrKLh4R/tqzreYQCZ68jbQYEMKA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rJflQVlMPRHaTLSvcJX1NZi+97Qvziug5oEUOjc3ojsBu9BVMLyGov6Z1mxHQjOSF\n\td4sHNzE5ZuEio9sxUZmBBG8oP1Bz13+Q3BRukLNTvkIo625r90RthfEG7B24EMSSLE\n\tdYyFQbr17HXeBP5rvcLd7tuzSJpOBnyWVuJIhk8g=","Date":"Sat, 18 Jan 2020 00:25:56 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne <nicolas.dufresne@collabora.com>","Message-ID":"<20200117222556.GQ5711@pendragon.ideasonboard.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-6-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200117191733.198897-6-nicolas@ndufresne.ca>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 22:26:12 -0000"}},{"id":3499,"web_url":"https://patchwork.libcamera.org/comment/3499/","msgid":"<7898af858f3f3fdc176c412b7b34243da7727796.camel@collabora.com>","date":"2020-01-17T22:58:33","subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le samedi 18 janvier 2020 à 00:25 +0200, Laurent Pinchart a écrit :\n> Hi Nicolas,\n> \n> Thank you for the patch.\n> \n> On Fri, Jan 17, 2020 at 02:17:32PM -0500, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > \n> > This introduce a new argument \"--amend\" and a new special type of\n> > commit \"Amendment\". It will check the style of changes that are in\n> > the index combined with the changes of the last commit. So this is\n> > the changes that would be applied by \"git commit --amend\" hence the\n> > name of the argument.\n> > \n> > This is needed to implement pre-commit hook.\n> > \n> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > ---\n> >  utils/checkstyle.py | 24 ++++++++++++++++++++++++\n> >  1 file changed, 24 insertions(+)\n> >  mode change 100644 => 100755 utils/checkstyle.py\n> > \n> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > old mode 100644\n> > new mode 100755\n> > index 8e456cd..7c2ce00\n> > --- a/utils/checkstyle.py\n> > +++ b/utils/checkstyle.py\n> > @@ -501,6 +501,26 @@ class Index(Commit):\n> >                                stdout=subprocess.PIPE).stdout.decode('utf-8')\n> >  \n> >  \n> > +class Amendment(Index):\n> > +    def __init__(self):\n> > +        Commit.__init__(self, None)\n> > +\n> > +    def det_info(self, top_level):\n> \n> s/det_info/get_info/\n> \n> This code path has never been tested :-)\n\nArg, uncommited, sorry.\n\n> \n> > +        # Create a title using HEAD commit\n> > +        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', 'HEAD'],\n> > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > +        title = 'Amendment of: ' + ret.splitlines()[0]\n> > +        # Extract the list of modifier files\n> > +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'],\n> > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > +        return title, ret.splitlines()\n> > +\n> > +    def get_diff(self, top_level, filename):\n> > +        return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',\n> > +                               '%s/%s' % (top_level, filename)],\n> > +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > +\n> > +\n> >  def check_file(top_level, commit, filename):\n> >      # Extract the line numbers touched by the commit.\n> >      diff = commit.get_diff(top_level, filename)\n> > @@ -633,6 +653,8 @@ def main(argv):\n> >                          help='Code formatter. Default to clang-format if not specified.')\n> >      parser.add_argument('--staged', '-s', action='store_true',\n> >                          help='Include the changes in the index. Defaults to False')\n> > +    parser.add_argument('--amend', '-a', action='store_true',\n> > +                        help='Includes changes in the index and the previous patch combined. Defaults to False')\n> \n> s/Includes/Include/\n\nAck.\n\n> \n> >      parser.add_argument('revision_range', type=str, default=None, nargs='?',\n> >                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')\n> >      args = parser.parse_args(argv[1:])\n> > @@ -671,6 +693,8 @@ def main(argv):\n> >      revlist = []\n> >      if args.staged:\n> >          revlist.append(Index())\n> > +    if args.amend:\n> > +        revlist.append(Amendment())\n> \n> I think you'll find out that it won't work correctly if you specify both\n> --staged and --amend, as they duplicate each other to some extent.\n> Should we at the very least ignore --staged if --amend is set ? Or use a\n> single parameter that would take two different values so that both can't\n> be set together ?\n> \n> Should we also ignore the list of commits of --staged or --amend is set\n> ?\n\nJust tested, and there is bugs in there. Of course it's redundant, but that's a\nuser problem.\n\n./utils/checkstyle.py --staged --amend HEAD~\n--------------\nStaged changes\n--------------\nNo style issue detected\n\n--------------------------------------------------------------------------------\n---------------\nAmendment of: f4079ec6365ef6a5c23ff844e9a92aa93e986f01 checkstyle: Add a pre-\ncommit hook script\n--------------------------------------------------------------------------------\n---------------\nNo style issue detected\n\n--------------------------------------------------------------------------------\n-----------------\nb240df0a1857a653d7417f1bf6035f552d3815e8 checkstyle: Add support for checking\nstyle on amendments\n--------------------------------------------------------------------------------\n-----------------\nNo style issue detected\n\n> \n> >      # If nothing of --staged or --amend was passed, defaults to HEAD\n> >      if len(revlist) == 0 and not args.revision_range:","headers":{"Return-Path":"<nicolas.dufresne@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B450560792\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 23:58:42 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::127])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 900B929487B;\n\tFri, 17 Jan 2020 22:58:41 +0000 (GMT)"],"Message-ID":"<7898af858f3f3fdc176c412b7b34243da7727796.camel@collabora.com>","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 17 Jan 2020 17:58:33 -0500","In-Reply-To":"<20200117222556.GQ5711@pendragon.ideasonboard.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-6-nicolas@ndufresne.ca>\n\t<20200117222556.GQ5711@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 22:58:42 -0000"}},{"id":3504,"web_url":"https://patchwork.libcamera.org/comment/3504/","msgid":"<ced3881f116c519f6fc016faa6f06859bf895bf5.camel@collabora.com>","date":"2020-01-17T23:29:00","subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le vendredi 17 janvier 2020 à 17:58 -0500, Nicolas Dufresne a écrit :\n> Le samedi 18 janvier 2020 à 00:25 +0200, Laurent Pinchart a écrit :\n> > Hi Nicolas,\n> > \n> > Thank you for the patch.\n> > \n> > On Fri, Jan 17, 2020 at 02:17:32PM -0500, Nicolas Dufresne wrote:\n> > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > > \n> > > This introduce a new argument \"--amend\" and a new special type of\n> > > commit \"Amendment\". It will check the style of changes that are in\n> > > the index combined with the changes of the last commit. So this is\n> > > the changes that would be applied by \"git commit --amend\" hence the\n> > > name of the argument.\n> > > \n> > > This is needed to implement pre-commit hook.\n> > > \n> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > > ---\n> > >  utils/checkstyle.py | 24 ++++++++++++++++++++++++\n> > >  1 file changed, 24 insertions(+)\n> > >  mode change 100644 => 100755 utils/checkstyle.py\n> > > \n> > > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > > old mode 100644\n> > > new mode 100755\n> > > index 8e456cd..7c2ce00\n> > > --- a/utils/checkstyle.py\n> > > +++ b/utils/checkstyle.py\n> > > @@ -501,6 +501,26 @@ class Index(Commit):\n> > >                                stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > >  \n> > >  \n> > > +class Amendment(Index):\n> > > +    def __init__(self):\n> > > +        Commit.__init__(self, None)\n> > > +\n> > > +    def det_info(self, top_level):\n> > \n> > s/det_info/get_info/\n> > \n> > This code path has never been tested :-)\n> \n> Arg, uncommited, sorry.\n> \n> > > +        # Create a title using HEAD commit\n> > > +        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', 'HEAD'],\n> > > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > > +        title = 'Amendment of: ' + ret.splitlines()[0]\n> > > +        # Extract the list of modifier files\n> > > +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'],\n> > > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > > +        return title, ret.splitlines()\n> > > +\n> > > +    def get_diff(self, top_level, filename):\n> > > +        return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',\n> > > +                               '%s/%s' % (top_level, filename)],\n> > > +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > > +\n> > > +\n> > >  def check_file(top_level, commit, filename):\n> > >      # Extract the line numbers touched by the commit.\n> > >      diff = commit.get_diff(top_level, filename)\n> > > @@ -633,6 +653,8 @@ def main(argv):\n> > >                          help='Code formatter. Default to clang-format if not specified.')\n> > >      parser.add_argument('--staged', '-s', action='store_true',\n> > >                          help='Include the changes in the index. Defaults to False')\n> > > +    parser.add_argument('--amend', '-a', action='store_true',\n> > > +                        help='Includes changes in the index and the previous patch combined. Defaults to False')\n> > \n> > s/Includes/Include/\n> \n> Ack.\n> \n> > >      parser.add_argument('revision_range', type=str, default=None, nargs='?',\n> > >                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')\n> > >      args = parser.parse_args(argv[1:])\n> > > @@ -671,6 +693,8 @@ def main(argv):\n> > >      revlist = []\n> > >      if args.staged:\n> > >          revlist.append(Index())\n> > > +    if args.amend:\n> > > +        revlist.append(Amendment())\n> > \n> > I think you'll find out that it won't work correctly if you specify both\n> > --staged and --amend, as they duplicate each other to some extent.\n> > Should we at the very least ignore --staged if --amend is set ? Or use a\n> > single parameter that would take two different values so that both can't\n> > be set together ?\n> > \n> > Should we also ignore the list of commits of --staged or --amend is set\n> > ?\n> \n> Just tested, and there is bugs in there. Of course it's redundant, but that's a\n> user problem.\n\nTypo: there is *no* bugs.\n\n> \n> ./utils/checkstyle.py --staged --amend HEAD~\n> --------------\n> Staged changes\n> --------------\n> No style issue detected\n> \n> --------------------------------------------------------------------------------\n> ---------------\n> Amendment of: f4079ec6365ef6a5c23ff844e9a92aa93e986f01 checkstyle: Add a pre-\n> commit hook script\n> --------------------------------------------------------------------------------\n> ---------------\n> No style issue detected\n> \n> --------------------------------------------------------------------------------\n> -----------------\n> b240df0a1857a653d7417f1bf6035f552d3815e8 checkstyle: Add support for checking\n> style on amendments\n> --------------------------------------------------------------------------------\n> -----------------\n> No style issue detected\n> \n> > >      # If nothing of --staged or --amend was passed, defaults to HEAD\n> > >      if len(revlist) == 0 and not args.revision_range:\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<nicolas.dufresne@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D9E8460705\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 Jan 2020 00:29:08 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::127])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 5225C2947A8;\n\tFri, 17 Jan 2020 23:29:08 +0000 (GMT)"],"Message-ID":"<ced3881f116c519f6fc016faa6f06859bf895bf5.camel@collabora.com>","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 17 Jan 2020 18:29:00 -0500","In-Reply-To":"<7898af858f3f3fdc176c412b7b34243da7727796.camel@collabora.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-6-nicolas@ndufresne.ca>\n\t<20200117222556.GQ5711@pendragon.ideasonboard.com>\n\t<7898af858f3f3fdc176c412b7b34243da7727796.camel@collabora.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for\n\tchecking style on amendments","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 23:29:09 -0000"}}]