Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <bcba757ac112bab9287632400ce8b3508c955572.camel@perches.com>
Subject: Re: [PATCH v2] checkpatch.pl: warn on invalid commit id
From:   Joe Perches <joe@perches.com>
To:     Matteo Croce <mcroce@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Andy Whitcroft <apw@canonical.com>
Date:   Thu, 11 Jul 2019 02:04:11 -0700
In-Reply-To: <20190711001640.13398-1-mcroce@redhat.com>
References: <20190711001640.13398-1-mcroce@redhat.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.30.5-0ubuntu0.18.10.1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 2019-07-11 at 02:16 +0200, Matteo Croce wrote:
> It can happen that a commit message refers to an invalid commit id, because
> the referenced hash changed following a rebase, or simply by mistake.
> Add a check in checkpatch.pl which checks that an hash referenced by
> a Fixes tag, or just cited in the commit message, is a valid commit id.

Thanks Matteo, this seems sensible.

>     $ scripts/checkpatch.pl <<'EOF'
>     Subject: [PATCH] test commit
> 
>     Sample test commit to test checkpatch.pl
>     Commit 1da177e4c3f4 ("Linux-2.6.12-rc2") really exists,
>     commit 0bba044c4ce7 ("tree") is valid but not a commit,
>     while commit b4cc0b1c0cca ("unknown") is invalid.
> 
>     Fixes: f0cacc14cade ("unknown")
>     Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>     EOF
>     WARNING: Unknown commit id '0bba044c4ce7', maybe rebased or not pulled?
>     #8:
>     commit 0bba044c4ce7 ("tree") is valid but not a commit,
> 
>     WARNING: Unknown commit id 'b4cc0b1c0cca', maybe rebased or not pulled?
>     #9:
>     while commit b4cc0b1c0cca ("unknown") is invalid.
> 
>     WARNING: Unknown commit id 'f0cacc14cade', maybe rebased or not pulled?
>     #11:
>     Fixes: f0cacc14cade ("unknown")
> 
>     total: 0 errors, 3 warnings, 4 lines checked
> 
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
> ---
>  scripts/checkpatch.pl | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index a6d436809bf5..3b77279df13b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2898,6 +2898,17 @@ sub process {
>  			}
>  		}
>  
> +# check for invalid commit id
> +		if ($in_commit_log && $line =~ /(^fixes:|\bcommit)\s+([0-9a-f]{6,40})\b/i) {
> +			my $id;
> +			my $description;
> +			($id, $description) = git_commit_info($2, undef, undef);
> +			if (!defined($id)) {
> +				WARN("UNKNOWN_COMMIT_ID",
> +				     "Unknown commit id '$2', maybe rebased or not pulled?\n" . $herecurr);
> +			}
> +		}
> +
>  # ignore non-hunk lines and lines being removed
>  		next if (!$hunk_line || $line =~ /^-/);
>