Message ID | 20230129123431.1282427-1-j.neuschaefer@gmx.net |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp1727329wrn; Sun, 29 Jan 2023 04:37:50 -0800 (PST) X-Google-Smtp-Source: AK7set9fG+pA5u1TL50yMMPspzr2VFUuzqeHjt2exrae/8KQGTlLhlFvCCovpHGSiPIYBFe56j/d X-Received: by 2002:a17:906:b310:b0:878:52cd:9006 with SMTP id n16-20020a170906b31000b0087852cd9006mr14338730ejz.69.1674995869817; Sun, 29 Jan 2023 04:37:49 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674995869; cv=none; d=google.com; s=arc-20160816; b=c6XVvf+Hmyr+RCpPaAECnq9hOAbMXJ5B03ifGyzeVQrF4epOk2mj19K+Lzbr+7qsLj 7efq73uaG5TiqhJleBIxaVVx8gkNAP2H+0jPSIQAehD0u1IFM5wn6NYIuKJejiSq80h7 2F6J607EL2DODCu7RoRgzB310nN7f0go4ih6QYGgN5DtebUgtpZL06ehE5zlvvCPCtR9 uvBAEkEOmMW5tzDJDPrE4z1jnIbCsO2UBbS67VRNAEUdPUa76DXC+fUS5h6M0/vQonN/ OtpfqVnEJHlx+XGnZqUi8lgRDZQ2ph3n4DJLuYz6cohgRPSsvW+bAyGHL8YBLWUGgj27 /iTw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:ui-outboundreport:content-transfer-encoding :mime-version:message-id:date:subject:cc:to:from:dkim-signature; bh=8JVR003H/CTyyYzKfG6Cl5xk+z3T9FBTR7QdQbCzz+w=; b=qcSd9yHzUqCczwfMPDFzqxL98WLsT7yIbWBwpVI8ml6BckNEsulUTlPF8PohPAk8hf dsOO11s3IB0R9iCEW3wZAAXCtQGbBWS5wp/t5Q8v4g2cl9dwQ//Vmuizc3gMyQl1jy/P Ukaz7UOXNFJThROtGTe5zKDeICCtVhRyRpkDc1aGBhXMqiPPRSw4jmoG7FJNLH2pOWAK 9MFenztCYONkm+uJ3Gwq7nBb+0QyGq8GT5yOE7muZKGlkkO8cT0CGeJiKa4WSHhwZed3 FZRqBrLNE0UJRDYcR2P49BAiVjnOvgS91F7dZKpA/CggluDTLD9tJ7NscwMxFqZHmhYU 0CKw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmx.net header.s=s31663417 header.b=SLpHvAsj; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmx.net Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id gi30-20020a1709070c9e00b008610c1703cesi10677449ejc.228.2023.01.29.04.37.25; Sun, 29 Jan 2023 04:37:49 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmx.net header.s=s31663417 header.b=SLpHvAsj; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmx.net Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232698AbjA2Meu (ORCPT <rfc822;n2h9z4@gmail.com> + 99 others); Sun, 29 Jan 2023 07:34:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37392 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230240AbjA2Meq (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 29 Jan 2023 07:34:46 -0500 Received: from mout.gmx.net (mout.gmx.net [212.227.17.22]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4668422001 for <linux-kernel@vger.kernel.org>; Sun, 29 Jan 2023 04:34:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=s31663417; t=1674995677; bh=gQt9VX1KvePgNEJAJdcWhPr0Jz5WN6TRoAAoW9znZzs=; h=X-UI-Sender-Class:From:To:Cc:Subject:Date; b=SLpHvAsjJ3SNOYugkehJl3+gRnBB6gZXIN7L7DX9xyIxkNHxYXOROtXbqfQpEYBud PkdAKQr7DxjmGg9CuZJaVWDYC/GvBLlTk+cd6jmMEDOsdkrwzoSf2Tj1o+Z8GuY59d y0qfBWJRgOWnnAvC4sc9FF3P9L+6wBRouWiXM6psRUteq1nd6tHjwrACPQJ93p/MCo RGvH+ylI7jvMq5iSanUQ8LSD/6/2PlKmU6Wsykks4ILXAlKm0OS10+xq+5vBcmi6km kJjDlKJ48oM09EE6iH68fwGlvYzeQdLzFVW1DqS7YSkV/bvnxaCy0jfxSw2FliuQKf wm/QDZCETm5pg== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from probook ([95.223.44.193]) by mail.gmx.net (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1Mz9Ux-1oQikB3vOt-00wE9X; Sun, 29 Jan 2023 13:34:37 +0100 From: =?utf-8?q?Jonathan_Neusch=C3=A4fer?= <j.neuschaefer@gmx.net> To: linux-kernel@vger.kernel.org Cc: =?utf-8?q?Jonathan_Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>, Andy Whitcroft <apw@canonical.com>, Joe Perches <joe@perches.com>, Dwaipayan Ray <dwaipayanray1@gmail.com>, Lukas Bulwahn <lukas.bulwahn@gmail.com> Subject: [PATCH] checkpatch.pl: Relax commit ID check to allow more than 12 chars Date: Sun, 29 Jan 2023 13:34:31 +0100 Message-Id: <20230129123431.1282427-1-j.neuschaefer@gmx.net> X-Mailer: git-send-email 2.39.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:To8aw2cJRT9HKi99D3nDKzAF7HB6D3EVKkUCpVvmnu4s8/G9skO 9B70RmrjYJKUhi7Cq1oIIPOgavukG58D2bG4pjslG1ylrMYIRtczTCW+uFzmhRB7m89+F+x t5OOy7w5QfcO5ocAofHzlv3/ZuHksA3+pGvzEWGYuRHnx8V0vg5ZlvEjiIxfEsPBmKMgbo+ BQTa7776vfouTVrzMUHEQ== UI-OutboundReport: notjunk:1;M01:P0:Y8aGokvwFWc=;kCLUfMKfd2LdofXYZe7mqHXvUga QJi5uKVtbXRUMmtsqDWJIdRfTkz+KrHFbMt8aigVlWkAvPwvEhwsIRH1LGDM9YFB5fDSGO/Yg PZ8R3eaT4agmYz+spbIPlGLrQBmwL1Yw6Jm7jWrhUECL0+mycgd4+j3EJpZWzvCWTS7Kp+Xye cSVJ0uA9RCTIF1q/upBId6PUvpJxVF2YF8s+mJHDUywSVTwNyBNC8KgDyoEXHsKGD5Ry35SDX o1Q47g8gcXwzO5EitHiyzZlDiMFsPXWJMLrXPWE6aA0cKnSNZEm8+ZbdPJR+9NK3R2sTHC0T6 uvoByI7d988cJmv9d4pNLTVi0fChdgKA7WTsthT3T3z4cu4iX9KMlPBEQa9wNIrijLM2I5dpj 7gk+ge+xtAbR1jczRJG3L0LSM1jP54da1r/OoH+he4/1k1awIdrauwsf+KH6yAAA3JN/6AQhn cGFBKW/EriloXIMOKFBFt844knPm1cCLJfbmCYiIvKQqQP+Z1jWebux1FT5pnxxL8eSGwFrdl KAzZq3N7BzN9IcX8a+sIh5W+qt+psNJzsJlADv9LoUwM53eq8CbekmdqTHorckQ3kwzUZPg8h DUVP9nDjLHIOQ/wsV/mxA7BD86J6Ox1GZgbrpaajlEKh74tOLBKO6ABpqWnu6m7Af++0BCWCx UN/uv6iNiEZ5St7Ma+RmT5ra7aP8iNCIGvCOm1A+YWudRwzSNywTqEYD6bVyrprUhvN/Qx1QB gRt7hnXQsj+p4Y0nKs12uJGKYkF5r2TVMifQmsyF8jlteup07sy8vyqWC9zp/BkKlRFufvhdQ 1pyt89R9G7f+lXl2tcFC/DKiZDtIkKWNgD02jerZzjLg1KeT5XW6roouhalvfWdblY0Md5DfJ R2CLHFWJKs1QnBwNXP5r7LBPPeMr/J2jzb9WX9AtFHodqKzdK6W7FlTZ9JF49v30f3PYwkCXz OsPPf7xnyFuC+R9ndPPpa6XGbmg= X-Spam-Status: No, score=0.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756360469143699135?= X-GMAIL-MSGID: =?utf-8?q?1756360469143699135?= |
Series |
checkpatch.pl: Relax commit ID check to allow more than 12 chars
|
|
Commit Message
Jonathan Neuschäfer
Jan. 29, 2023, 12:34 p.m. UTC
By now, `git log --pretty=%h` (on my copy of linux.git) prints commit
hashes with 13 digits, because of the number of objects.
Relax the rule in checkpatch.pl to allow a few more digits (up to 16).
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
scripts/checkpatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.39.0
Comments
On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: > By now, `git log --pretty=%h` (on my copy of linux.git) prints commit > hashes with 13 digits, because of the number of objects. > > Relax the rule in checkpatch.pl to allow a few more digits (up to 16). NAK without updating the process docs first. Documentation/process/submitting-patches.rst-If your patch fixes a bug in a specific commit, e.g. you found an issue using Documentation/process/submitting-patches.rst:``git bisect``, please use the 'Fixes:' tag with the first 12 characters of Documentation/process/submitting-patches.rst-the SHA-1 ID, and the one line summary. Do not split the tag across multiple Documentation/process/submitting-patches.rst-lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify Documentation/process/submitting-patches.rst-parsing scripts. For example:: Documentation/process/submitting-patches.rst- Documentation/process/submitting-patches.rst- Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually fr> Documentation/process/submitting-patches.rst- Documentation/process/submitting-patches.rst-The following ``git config`` settings can be used to add a pretty format for Documentation/process/submitting-patches.rst-outputting the above style in the ``git log`` or ``git show`` commands:: Documentation/process/submitting-patches.rst- Documentation/process/submitting-patches.rst- [core] Documentation/process/submitting-patches.rst: abbrev = 12 Documentation/process/submitting-patches.rst- [pretty] Documentation/process/submitting-patches.rst- fixes = Fixes: %h (\"%s\") > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -3177,7 +3177,7 @@ sub process { > $tag_case = 0 if $tag eq "Fixes:"; > $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i); > > - $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i); > + $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12,16}$/i); > $id_case = 0 if ($orig_commit !~ /[A-F]/); > > # Always strip leading/trailing parens then double quotes if existing > @@ -3194,7 +3194,7 @@ sub process { > if ($ctitle ne $title || $tag_case || $tag_space || > $id_length || $id_case || !$title_has_quotes) { > if (WARN("BAD_FIXES_TAG", > - "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && > + "Please use correct Fixes: style 'Fixes: <12-16 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && > $fix) { > $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")"; > } > -- > 2.39.0 >
On Sun, Jan 29, 2023 at 09:52:38AM -0800, Joe Perches wrote: > On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: > > By now, `git log --pretty=%h` (on my copy of linux.git) prints commit > > hashes with 13 digits, because of the number of objects. > > > > Relax the rule in checkpatch.pl to allow a few more digits (up to 16). > > NAK without updating the process docs first. Good point, I'll do that. Thanks, Jonathan > > Documentation/process/submitting-patches.rst-If your patch fixes a bug in a specific commit, e.g. you found an issue using > Documentation/process/submitting-patches.rst:``git bisect``, please use the 'Fixes:' tag with the first 12 characters of > Documentation/process/submitting-patches.rst-the SHA-1 ID, and the one line summary. Do not split the tag across multiple > Documentation/process/submitting-patches.rst-lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify > Documentation/process/submitting-patches.rst-parsing scripts. For example:: > Documentation/process/submitting-patches.rst- > Documentation/process/submitting-patches.rst- Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually fr> > Documentation/process/submitting-patches.rst- > Documentation/process/submitting-patches.rst-The following ``git config`` settings can be used to add a pretty format for > Documentation/process/submitting-patches.rst-outputting the above style in the ``git log`` or ``git show`` commands:: > Documentation/process/submitting-patches.rst- > Documentation/process/submitting-patches.rst- [core] > Documentation/process/submitting-patches.rst: abbrev = 12 > Documentation/process/submitting-patches.rst- [pretty] > Documentation/process/submitting-patches.rst- fixes = Fixes: %h (\"%s\") > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -3177,7 +3177,7 @@ sub process { > > $tag_case = 0 if $tag eq "Fixes:"; > > $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i); > > > > - $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i); > > + $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12,16}$/i); > > $id_case = 0 if ($orig_commit !~ /[A-F]/); > > > > # Always strip leading/trailing parens then double quotes if existing > > @@ -3194,7 +3194,7 @@ sub process { > > if ($ctitle ne $title || $tag_case || $tag_space || > > $id_length || $id_case || !$title_has_quotes) { > > if (WARN("BAD_FIXES_TAG", > > - "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && > > + "Please use correct Fixes: style 'Fixes: <12-16 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && > > $fix) { > > $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")"; > > } > > -- > > 2.39.0 > > >
On Sun, 2023-01-29 at 09:52 -0800, Joe Perches wrote: > On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: > > By now, `git log --pretty=%h` (on my copy of linux.git) prints commit > > hashes with 13 digits, because of the number of objects. > > > > Relax the rule in checkpatch.pl to allow a few more digits (up to 16). > > NAK without updating the process docs first. btw: it looks like 12 will still be sufficient for awhile yet $ git count total 1154908 $ git -c core.abbrev=5 log --pretty=format:%h | \ perl -nE 'chomp;say length' | sort | uniq -c | sort -n -k2 198 5 664613 6 450955 7 36667 8 2312 9 155 10 8 11
On Sat, Feb 04, 2023 at 08:57:59AM -0800, Joe Perches wrote: > On Sun, 2023-01-29 at 09:52 -0800, Joe Perches wrote: > > On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: > > > By now, `git log --pretty=%h` (on my copy of linux.git) prints commit > > > hashes with 13 digits, because of the number of objects. > > > > > > Relax the rule in checkpatch.pl to allow a few more digits (up to 16). > > > > NAK without updating the process docs first. > > btw: it looks like 12 will still be sufficient for awhile yet > > $ git count > total 1154908 > $ git -c core.abbrev=5 log --pretty=format:%h | \ > perl -nE 'chomp;say length' | sort | uniq -c | sort -n -k2 > 198 5 > 664613 6 > 450955 7 > 36667 8 > 2312 9 > 155 10 > 8 11 Ok, I get similar stats on my tree (which includes linux-next and a few other remotes). However, git's default heuristic for %h length uses 13 digits here, so I think other people might get 13 digits as well. I could force git to use less digits than it naturally would, by setting core.abbrev=12 (and document this idea in the documentation), but that doesn't seem nice. Therefore, I still think allowing a few more digits is a good idea. Thanks, Jonathan
On 2/5/23 02:40, Jonathan Neuschäfer wrote: > On Sat, Feb 04, 2023 at 08:57:59AM -0800, Joe Perches wrote: >> On Sun, 2023-01-29 at 09:52 -0800, Joe Perches wrote: >>> On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: >>>> By now, `git log --pretty=%h` (on my copy of linux.git) prints commit >>>> hashes with 13 digits, because of the number of objects. >>>> >>>> Relax the rule in checkpatch.pl to allow a few more digits (up to 16). >>> >>> NAK without updating the process docs first. >> >> btw: it looks like 12 will still be sufficient for awhile yet >> >> $ git count >> total 1154908 >> $ git -c core.abbrev=5 log --pretty=format:%h | \ >> perl -nE 'chomp;say length' | sort | uniq -c | sort -n -k2 >> 198 5 >> 664613 6 >> 450955 7 >> 36667 8 >> 2312 9 >> 155 10 >> 8 11 > > Ok, I get similar stats on my tree (which includes linux-next and a few > other remotes). > > However, git's default heuristic for %h length uses 13 digits here, so I > think other people might get 13 digits as well. I could force git to use > less digits than it naturally would, by setting core.abbrev=12 (and > document this idea in the documentation), but that doesn't seem nice. > Therefore, I still think allowing a few more digits is a good idea. I have core.abbrev=12 and I still get 13 "digits" often. Then I just chop it off at 12 to satisfy checkpatch...
On Sat, Feb 4, 2023 at 8:58 AM Joe Perches <joe@perches.com> wrote: > > btw: it looks like 12 will still be sufficient for awhile yet To be honest, that's actually closer to the 12-digit limit than I was expecting. The git heuristics are pretty good, and it sounds like 13 hex digits is already starting to happen, so maybe we should relax things. That said, "up to 16" does sound questionable. We're talking exponential growth by number of digits, so saying "let's go from 12 to 16" is a *huge* jump. And I'd like to keep people doing fewer digits just because these things get used in free-flowing prose, and we have the whole line wrapping issue and things just get uglier at some point. So we're closing in on two decades of git use, and we are not that far from having 10 million objects in our git database (for the base tree). Sure, that's a lot of objects, but to a close approximation the object count grows _largely_ linearly with time. Considering that git is actually pretty good at handling the ambiguous case anyway, I'd say go up at *most* to 14 digits. I just checked my current tip-of-tree, and I needed to go down to *five* digits to have git start complaining about ambiguous object names: [torvalds@ryzen linux]$ git show c608f error: short object ID c608f is ambiguous hint: The candidates are: hint: c608f6b58f30 commit 2023-02-05 - Merge tag 'usb-6.2-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb hint: c608f14fb0ee tree hint: c608fccf692f tree hint: c608f76e5753 blob hint: c608fa168fe6 blob hint: c608fd96771c blob and maybe that was pure luck, but looking at your stats it does look like "6 digits is still unique for most objects", I really think that we're better off with shorter and visually easier numbers than going overboard. Note above how even with just 5 digits, it's still unique in actual commits, so from a *practical* standpoint even five digits are fine (because normal human communication doesn't talk about the blob or tree commits). If this was some case of "when you hit the limit, things break horribly badly", that would be one thing. But that not even being true means that things like line wrapping and just visuals matter. So I think 12 digits likely still work just fine for another decade or two, but yes, we're at the point where we might want to start thinking about 13 or 14. Linus
Hi Joe, On Sat, Feb 4, 2023 at 5:59 PM Joe Perches <joe@perches.com> wrote: > On Sun, 2023-01-29 at 09:52 -0800, Joe Perches wrote: > > On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: > > > By now, `git log --pretty=%h` (on my copy of linux.git) prints commit > > > hashes with 13 digits, because of the number of objects. > > > > > > Relax the rule in checkpatch.pl to allow a few more digits (up to 16). > > > > NAK without updating the process docs first. > > btw: it looks like 12 will still be sufficient for awhile yet > > $ git count > total 1154908 Hmm, Ubuntu git too old? > $ git -c core.abbrev=5 log --pretty=format:%h | \ > perl -nE 'chomp;say length' | sort | uniq -c | sort -n -k2 > 198 5 > 664613 6 > 450955 7 > 36667 8 > 2312 9 > 155 10 > 8 11 I'm already at twelve: 433752 6 640819 7 62759 8 3998 9 261 10 12 11 1 12 I've been using core.abbrev=16 for a while, and some maintainers reject my patches with Fixes: tags because of that... Is it really worthwhile to save on the number of hexits, making lookup of some commits more inconvenient? Note that while "git show edb9b8" suggests edb9b8f[...], gitweb says bad object id: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=edb9b8 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, 2023-02-06 at 09:38 +0100, Geert Uytterhoeven wrote: > Hi Joe, > > On Sat, Feb 4, 2023 at 5:59 PM Joe Perches <joe@perches.com> wrote: > > On Sun, 2023-01-29 at 09:52 -0800, Joe Perches wrote: > > > On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: > > > > By now, `git log --pretty=%h` (on my copy of linux.git) prints commit > > > > hashes with 13 digits, because of the number of objects. > > > > > > > > Relax the rule in checkpatch.pl to allow a few more digits (up to 16). > > > > > > NAK without updating the process docs first. > > > > btw: it looks like 12 will still be sufficient for awhile yet > > > > $ git count > > total 1154908 > > Hmm, Ubuntu git too old? Don't think so $ git --version git version 2.39.1 More likely just using Linus' tree and not a development tree with a bunch of branches. I've got a -next tree with history back to next-20151106 with a bunch of missing dates because I don't fetch it every day. It has: $ git tag | grep next | wc -l 1134 There I get: $ git -c core.abbrev=5 log --pretty=format:%h | \ perl -nE 'chomp;say length' | sort | uniq -c | sort -n -k2 6 5 542082 6 568573 7 51124 8 3249 9 217 10 14 11 1 12 > I've been using core.abbrev=16 for a while, and some maintainers > reject my patches with Fixes: tags because of that... Perhaps because that's not the documented format? > Is it really worthwhile to save on the number of hexits, making lookup > of some commits more inconvenient? > > Note that while "git show edb9b8" suggests edb9b8f[...], > gitweb says bad object id: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=edb9b8 hmm. Not here. $ git show edb9b8 tree edb9b8 Kconfig Makefile fmvj18x_cs.c
Hi Joe, On Mon, Feb 6, 2023 at 12:09 PM Joe Perches <joe@perches.com> wrote: > On Mon, 2023-02-06 at 09:38 +0100, Geert Uytterhoeven wrote: > > On Sat, Feb 4, 2023 at 5:59 PM Joe Perches <joe@perches.com> wrote: > > > On Sun, 2023-01-29 at 09:52 -0800, Joe Perches wrote: > > > > On Sun, 2023-01-29 at 13:34 +0100, Jonathan Neuschäfer wrote: > > > > > By now, `git log --pretty=%h` (on my copy of linux.git) prints commit > > > > > hashes with 13 digits, because of the number of objects. > > > > > > > > > > Relax the rule in checkpatch.pl to allow a few more digits (up to 16). > > > > > > > > NAK without updating the process docs first. > > > > > > btw: it looks like 12 will still be sufficient for awhile yet > > > > > > $ git count > > > total 1154908 > > > > Hmm, Ubuntu git too old? > > Don't think so > > $ git --version > git version 2.39.1 Exactly, Ubuntu 22.04LTS only has $ git --version git version 2.34.1 i.e. no git count. > > I've been using core.abbrev=16 for a while, and some maintainers > > reject my patches with Fixes: tags because of that... > > Perhaps because that's not the documented format? Right. I look a lot at history, and don't want to become slowed down by ambiguous Fixes: tags anytime soon (or later). > > Is it really worthwhile to save on the number of hexits, making lookup > > of some commits more inconvenient? > > > > Note that while "git show edb9b8" suggests edb9b8f[...], > > gitweb says bad object id: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=edb9b8 > > hmm. Not here. > > $ git show edb9b8 > tree edb9b8 Yeah, I also have that tree object. But I don't want to see the tree object; I want to see the commit object, which is in v6.2-rc7: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=edb9b8f Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, 2023-02-06 at 12:54 +0100, Geert Uytterhoeven wrote:
> Hi Joe,
rehi Geert
maybe:
---
scripts/checkpatch.pl | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c98..55267ee6b1190 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -36,6 +36,8 @@ my $showfile = 0;
my $file = 0;
my $git = 0;
my %git_commits = ();
+my $git_sha1_min = 12;
+my $git_sha1_max = 14;
my $check = 0;
my $check_orig = 0;
my $summary = 1;
@@ -1230,7 +1232,13 @@ sub git_commit_info {
$lines[0] =~ /^fatal: bad object $commit/) {
$id = undef;
} else {
- $id = substr($lines[0], 0, 12);
+ my $len = length($commit);
+ if ($len < $git_sha1_min) {
+ $len = $git_sha1_min;
+ } elsif ($len > $git_sha1_max) {
+ $len = $git_sha1_max;
+ }
+ $id = substr($lines[0], 0, $len);
$desc = substr($lines[0], 41);
}
@@ -1297,7 +1305,7 @@ for my $filename (@ARGV) {
if ($filename eq '-') {
$vname = 'Your patch';
} elsif ($git) {
- $vname = "Commit " . substr($filename, 0, 12) . ' ("' . $git_commits{$filename} . '")';
+ $vname = "Commit " . substr($filename, 0, $git_sha1_min) . ' ("' . $git_commits{$filename} . '")';
} else {
$vname = $filename;
}
@@ -3191,7 +3199,7 @@ sub process {
$tag_case = 0 if $tag eq "Fixes:";
$tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i);
- $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i);
+ $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{$git_sha1_min,$git_sha1_max}$/i);
$id_case = 0 if ($orig_commit !~ /[A-F]/);
# Always strip leading/trailing parens then double quotes if existing
@@ -3208,7 +3216,7 @@ sub process {
if ($ctitle ne $title || $tag_case || $tag_space ||
$id_length || $id_case || !$title_has_quotes) {
if (WARN("BAD_FIXES_TAG",
- "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) &&
+ "Please use correct Fixes: style 'Fixes: <$git_sha1_min to $git_sha1_max chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")";
}
@@ -3300,9 +3308,9 @@ sub process {
$line !~ /^This reverts commit [0-9a-f]{7,40}/ &&
(($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i ||
($line =~ /\bcommit\s*$/i && defined($rawlines[$linenr]) && $rawlines[$linenr] =~ /^\s*[0-9a-f]{5,}\b/i)) ||
- ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i &&
- $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i &&
- $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) {
+ ($line =~ /(?:\s|^)[0-9a-f]{$git_sha1_min,40}(?:[\s"'\(\[]|$)/i &&
+ $line !~ /[\<\[][0-9a-f]{$git_sha1_min,40}[\>\]]/i &&
+ $line !~ /\bfixes:\s*[0-9a-f]{$git_sha1_min,40}/i))) {
my $init_char = "c";
my $orig_commit = "";
my $short = 1;
@@ -3340,11 +3348,11 @@ sub process {
if ($input =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
$init_char = $1;
$orig_commit = lc($2);
- $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i);
+ $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{$git_sha1_min,40}/i);
$long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i);
$space = 0 if ($input =~ /\bcommit [0-9a-f]/i);
$case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
- } elsif ($input =~ /\b([0-9a-f]{12,40})\b/i) {
+ } elsif ($input =~ /\b([0-9a-f]{$git_sha1_min,40})\b/i) {
$orig_commit = lc($1);
}
@@ -3355,7 +3363,7 @@ sub process {
($short || $long || $space || $case || ($orig_desc ne $description) || !$has_quotes) &&
$last_git_commit_id_linenr != $linenr - 1) {
ERROR("GIT_COMMIT_ID",
- "Please use git commit description style 'commit <12+ chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herectx);
+ "Please use git commit description style 'commit <$git_sha1_min to $git_sha1_max chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herectx);
}
#don't report the next line if this line ends in commit and the sha1 hash is the next line
$last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
On Mon, Feb 06, 2023 at 04:25:19AM -0800, Joe Perches wrote: > On Mon, 2023-02-06 at 12:54 +0100, Geert Uytterhoeven wrote: > > Hi Joe, > > rehi Geert > > maybe: > --- At a quick glance, this looks reasonable to me. Feel free to take over the patch and send a real v2. Thanks, Jonathan > scripts/checkpatch.pl | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index bd44d12965c98..55267ee6b1190 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -36,6 +36,8 @@ my $showfile = 0; > my $file = 0; > my $git = 0; > my %git_commits = (); > +my $git_sha1_min = 12; > +my $git_sha1_max = 14; > my $check = 0; > my $check_orig = 0; > my $summary = 1; > @@ -1230,7 +1232,13 @@ sub git_commit_info { > $lines[0] =~ /^fatal: bad object $commit/) { > $id = undef; > } else { > - $id = substr($lines[0], 0, 12); > + my $len = length($commit); > + if ($len < $git_sha1_min) { > + $len = $git_sha1_min; > + } elsif ($len > $git_sha1_max) { > + $len = $git_sha1_max; > + } > + $id = substr($lines[0], 0, $len); > $desc = substr($lines[0], 41); > } > > @@ -1297,7 +1305,7 @@ for my $filename (@ARGV) { > if ($filename eq '-') { > $vname = 'Your patch'; > } elsif ($git) { > - $vname = "Commit " . substr($filename, 0, 12) . ' ("' . $git_commits{$filename} . '")'; > + $vname = "Commit " . substr($filename, 0, $git_sha1_min) . ' ("' . $git_commits{$filename} . '")'; > } else { > $vname = $filename; > } > @@ -3191,7 +3199,7 @@ sub process { > $tag_case = 0 if $tag eq "Fixes:"; > $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i); > > - $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i); > + $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{$git_sha1_min,$git_sha1_max}$/i); > $id_case = 0 if ($orig_commit !~ /[A-F]/); > > # Always strip leading/trailing parens then double quotes if existing > @@ -3208,7 +3216,7 @@ sub process { > if ($ctitle ne $title || $tag_case || $tag_space || > $id_length || $id_case || !$title_has_quotes) { > if (WARN("BAD_FIXES_TAG", > - "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && > + "Please use correct Fixes: style 'Fixes: <$git_sha1_min to $git_sha1_max chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && > $fix) { > $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")"; > } > @@ -3300,9 +3308,9 @@ sub process { > $line !~ /^This reverts commit [0-9a-f]{7,40}/ && > (($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || > ($line =~ /\bcommit\s*$/i && defined($rawlines[$linenr]) && $rawlines[$linenr] =~ /^\s*[0-9a-f]{5,}\b/i)) || > - ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i && > - $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i && > - $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) { > + ($line =~ /(?:\s|^)[0-9a-f]{$git_sha1_min,40}(?:[\s"'\(\[]|$)/i && > + $line !~ /[\<\[][0-9a-f]{$git_sha1_min,40}[\>\]]/i && > + $line !~ /\bfixes:\s*[0-9a-f]{$git_sha1_min,40}/i))) { > my $init_char = "c"; > my $orig_commit = ""; > my $short = 1; > @@ -3340,11 +3348,11 @@ sub process { > if ($input =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) { > $init_char = $1; > $orig_commit = lc($2); > - $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i); > + $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{$git_sha1_min,40}/i); > $long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i); > $space = 0 if ($input =~ /\bcommit [0-9a-f]/i); > $case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); > - } elsif ($input =~ /\b([0-9a-f]{12,40})\b/i) { > + } elsif ($input =~ /\b([0-9a-f]{$git_sha1_min,40})\b/i) { > $orig_commit = lc($1); > } > > @@ -3355,7 +3363,7 @@ sub process { > ($short || $long || $space || $case || ($orig_desc ne $description) || !$has_quotes) && > $last_git_commit_id_linenr != $linenr - 1) { > ERROR("GIT_COMMIT_ID", > - "Please use git commit description style 'commit <12+ chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herectx); > + "Please use git commit description style 'commit <$git_sha1_min to $git_sha1_max chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herectx); > } > #don't report the next line if this line ends in commit and the sha1 hash is the next line > $last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 78cc595b98ce1..3a2c8b5426191 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3177,7 +3177,7 @@ sub process { $tag_case = 0 if $tag eq "Fixes:"; $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i); - $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i); + $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12,16}$/i); $id_case = 0 if ($orig_commit !~ /[A-F]/); # Always strip leading/trailing parens then double quotes if existing @@ -3194,7 +3194,7 @@ sub process { if ($ctitle ne $title || $tag_case || $tag_space || $id_length || $id_case || !$title_has_quotes) { if (WARN("BAD_FIXES_TAG", - "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && + "Please use correct Fixes: style 'Fixes: <12-16 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && $fix) { $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")"; }