threads / patch / 37324

patchRe: [PATCH] checkpatch: Add test for commit id formatting style in commit log

Subject: Re: [PATCH] checkpatch: Add test for commit id formatting style in commit log

## tl;dr

4 messages between Aug 10, 2014 and Aug 10, 2014. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Geert Uytterhoeven· Aug 10, 2014, 21:08 UTC · lore
Hi Joe,
On Thu, Jul 3, 2014 at 12:00 AM, Joe Perches <joe@perches.com> wrote:
Show 7 quoted lines
> Commit logs have various forms of commit id references.
>
> Try to standardize on a 12 character long lower case
> commit id along with a description of parentheses and
> the quoted subject line
>
> ie: commit 0123456789ab ("commit description")

Now this is in mainline, checkpatch starts complaining about my "too long" (40 chars) commit IDs in commit messages :-(

40 chars may be too long (but it's quick to copy-and-paste, as "git show" shows that by default), but 12 sounds a bit short, as that's only 48 bits.

According to the Birthday Paradox (en.wikipedia.org/wiki/Birthday_problem), there's a probability of 50% of a collision if you use 48 bits IDs in a repository with ca. 16 milion (2^24) objects. A Linux kernel repository counts ca. 4 million objects, so we're getting close...

So soon we'll get "error: short SHA1 is ambiguous".

BTW, is there actually an easy way to make "git show" show all options for an ambiguous SHA1?

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
Joe Perches· Aug 10, 2014, 21:28 UTC · re: Geert Uytterhoeven · lore
On Sun, 2014-08-10 at 23:08 +0200, Geert Uytterhoeven wrote:
> Hi Joe,
Hi Geert.
Show 14 quoted lines
> On Thu, Jul 3, 2014 at 12:00 AM, Joe Perches <joe@perches.com> wrote:
> > Commit logs have various forms of commit id references.
> >
> > Try to standardize on a 12 character long lower case
> > commit id along with a description of parentheses and
> > the quoted subject line
> >
> > ie: commit 0123456789ab ("commit description")
> 
> Now this is in mainline, checkpatch starts complaining about my "too long"
> (40 chars) commit IDs in commit messages :-(
> 
> 40 chars may be too long (but it's quick to copy-and-paste, as "git show"
> shows that by default), but 12 sounds a bit short, as that's only 48 bits.

Right now, this test allows 12 to 16 byte length commit ids without emitting a warning.

Andrew wanted this test, I don't care how long the commit id is in the commit log.

Show 9 quoted lines
> According to the Birthday Paradox (en.wikiipedia.org/wiki/Birthday_problem),
> there's a probability of 50% of a collision if you use 48 bits IDs in a
> repository with ca. 16 milion (2^24) objects. A Linux kernel repository
> counts ca. 4 million objects, so we're getting close...
> 
> So soon we'll get "error: short SHA1 is ambiguous".
> 
> BTW, is there actually an easy way to make "git show" show all options for
> an ambiguous SHA1?
Not so far as I know, but I'm nothing like a git expert.
The script I used before adding this to checkpatch was:

$ cat format_commit.sh #!/bin/bash

regex1="^error: short SHA1 $1 is ambiguous\." regex2="fatal: ambiguous argument '$1': unknown revision or path not in the working tree\."

tmp=$(mktemp --tmpdir format_commit.XXXXXXXXXXXXX)
git log --format='%H ("%s")' -1 $1 > $tmp 2>&1
read line < $tmp
rm -f $tmp
if [[ $line =~ $regex1 ]] ; then
    echo "checking commits $1..."
    git rev-list --remotes | grep -i "^$1" |
    while read line ; do
        git log --format='%H ("%s")' -1 $line | 
        echo "commit $(cut -c 1-12,41-)"
    done
elif [[ $line =~ $regex2 ]] ; then
    echo "No matching commit"
    exit 1
else
    echo "commit $(echo $line | cut -c1-12,41-)"
fi

exit 0 $

so that using "$ format_commit.sh 1234" looks at _all_ the commit references by using git rev-list then greps that output for the matches, but it is darn slow...

$ time ./format_commit.sh 1234 checking commits 1234... commit 1234351cba95 ("xfs: introduce xlog_copy_iovec") commit 1234471e2d11 ("perf header: Fix numa topology printing") commit 1234f4bada54 ("hwrng: Kconfig: remove dependency for atmel-rng driver") commit 12340313cf94 ("MAINTAINERS: add new cgroup list to CC notice") commit 12346037a718 ("UBIFS: dump more in the lprops debugging check") commit 12342c475f5d ("iwlwifi: proper monitor support") commit 1234010684bb ("Add notation that the Asus W5F laptop has a short cable instead of 80-wire.") commit 123411f2d0da ("[CPUFREQ] dprintf format fixes in cpufreq/speedstep-centrino.c")

real 0m24.535s user 0m21.668s sys 0m5.332s

Andrew Morton· Aug 10, 2014, 21:35 UTC · re: Joe Perches · lore
On Sun, 10 Aug 2014 14:28:01 -0700 Joe Perches <joe@perches.com> wrote:
Show 20 quoted lines
> > On Thu, Jul 3, 2014 at 12:00 AM, Joe Perches <joe@perches.com> wrote:
> > > Commit logs have various forms of commit id references.
> > >
> > > Try to standardize on a 12 character long lower case
> > > commit id along with a description of parentheses and
> > > the quoted subject line
> > >
> > > ie: commit 0123456789ab ("commit description")
> > 
> > Now this is in mainline, checkpatch starts complaining about my "too long"
> > (40 chars) commit IDs in commit messages :-(
> > 
> > 40 chars may be too long (but it's quick to copy-and-paste, as "git show"
> > shows that by default), but 12 sounds a bit short, as that's only 48 bits.
> 
> Right now, this test allows 12 to 16 byte length commit ids
> without emitting a warning.
> 
> Andrew wanted this test, I don't care how long the commit id
> is in the commit log.

Well, I mainly wanted to stop having to add "commit description" when people forget it. The length check was perhaps a bit anal. How about we make it "12 or more"?

Joe Perches· Aug 10, 2014, 21:41 UTC · re: Andrew Morton · lore
On Sun, 2014-08-10 at 14:35 -0700, Andrew Morton wrote:
Show 25 quoted lines
> On Sun, 10 Aug 2014 14:28:01 -0700 Joe Perches <joe@perches.com> wrote:
> > > On Thu, Jul 3, 2014 at 12:00 AM, Joe Perches <joe@perches.com> wrote:
> > > > Commit logs have various forms of commit id references.
> > > >
> > > > Try to standardize on a 12 character long lower case
> > > > commit id along with a description of parentheses and
> > > > the quoted subject line
> > > >
> > > > ie: commit 0123456789ab ("commit description")
> > > 
> > > Now this is in mainline, checkpatch starts complaining about my "too long"
> > > (40 chars) commit IDs in commit messages :-(
> > > 
> > > 40 chars may be too long (but it's quick to copy-and-paste, as "git show"
> > > shows that by default), but 12 sounds a bit short, as that's only 48 bits.
> > 
> > Right now, this test allows 12 to 16 byte length commit ids
> > without emitting a warning.
> > 
> > Andrew wanted this test, I don't care how long the commit id
> > is in the commit log.
> 
> Well, I mainly wanted to stop having to add "commit description" when
> people forget it.  The length check was perhaps a bit anal.  How about
> we make it "12 or more"?
Fine by me, just change the 16 to 40
---
 scripts/checkpatch.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to scripts/checkpatch.pl +2 −2
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 31a731e..b385bcb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2133,7 +2133,7 @@ sub process {
 # Check for improperly formed commit descriptions
 		if ($in_commit_log &&
 		    $line =~ /\bcommit\s+[0-9a-f]{5,}/i &&
-		    $line !~ /\b[Cc]ommit [0-9a-f]{12,16} \("/) {
+		    $line !~ /\b[Cc]ommit [0-9a-f]{12,40} \("/) {
 			$line =~ /\b(c)ommit\s+([0-9a-f]{5,})/i;
 			my $init_char = $1;
 			my $orig_commit = lc($2);
@@ -2141,7 +2141,7 @@ sub process {
 			my $desc = 'commit description';
 		        ($id, $desc) = git_commit_info($orig_commit, $id, $desc);
 			ERROR("GIT_COMMIT_ID",
-			      "Please use 12 to 16 chars for the git commit ID like: '${init_char}ommit $id (\"$desc\")'\n" . $herecurr);
+			      "Please use 12 or more chars for the git commit ID like: '${init_char}ommit $id (\"$desc\")'\n" . $herecurr);
 		}
 
 # Check for added, moved or deleted files

← back to recent threads