New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 850856 link

Starred by 1 user
Status: Fixed
Owner:
vovoy@chromium.org
Closed: Jun 2018
Cc:
briannorris@chromium.org
cywang@chromium.org
vapier@chromium.org
cylee@chromium.org
█ wuchengli@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

checkpatch.pl shows false warning: Please use git commit description style...

Project Member Reported by vovoy@chromium.org, Jun 8 2018 
When I repo upload a patch with following commit message:
  Revert "ANDROID: proc: make oom adjustment files user read-only"
    
  This reverts commit c0ec3df8178404486ef4c377b7d1f00a2ba8b94c.
  ...

src/repohooks/checkpatch.pl shows the following error message:
  ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit c0ec3df81784 ("ANDROID: proc: make oom adjustment files user read-only")'
  #7: 
  This reverts commit c0ec3df8178404486ef4c377b7d1f00a2ba8b94c.

This false warning is fixed upstream: https://patchwork.kernel.org/patch/9772331/

It can be fixed by update src/repohooks/checkpatch.pl to the upstream kernel 4.14.48 version.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 8 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/repohooks/+/bb06fc9c260c32080f29cf6146cc3ab4f815e68e

commit bb06fc9c260c32080f29cf6146cc3ab4f815e68e
Author: Kuo-Hsin Yang <vovoy@chromium.org>
Date: Fri Jun 08 13:55:39 2018

checkpatch.pl: Update to latest from v4.17

Copied checkpatch and related files from the v4.17 kernel.org
repository.

BUG= chromium:850856 
TEST=repo upload and check warnings
Signed-off-by: Kuo-Hsin Yang <vovoy@chromium.org>

Change-Id: Ib3451fdb161d61cbf151742f9bf7900cd3023811
Reviewed-on: https://chromium-review.googlesource.com/1092557
Commit-Ready: Vovo Yang <vovoy@chromium.org>
Tested-by: Vovo Yang <vovoy@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/bb06fc9c260c32080f29cf6146cc3ab4f815e68e/checkpatch.pl
[modify] https://crrev.com/bb06fc9c260c32080f29cf6146cc3ab4f815e68e/spelling.txt

Comment 2 by briannorris@chromium.org, Jun 8 2018

Status: Fixed (was: Started)
Awesome! I complained to the maintainer about all the false positives previously and even tried to fix them myself. He rejected me at the time. And as a matter of fact, I just recently added this check to my local blacklist:

$ cat ~/.checkpatch.conf 
--ignore=GIT_COMMIT_ID

Unfortunately, even with your upgrade, I still hit several of the same false positives, because IIUC checkpatch doesn't know how to reconcile its own line-wrapping rules with splitting these references across lines.

Anyway, I think your particular report is fixed now. Thanks for the upgrade!

Comment 3 by vovoy@chromium.org, Jun 8 2018 

I think the false positives in general cannot be solved with RE.

Engineering a compile, 3.2.1 Why not regular expressions?
"In fact, we cannot write an RE that will match all expressions with balanced parentheses. ... In principle, Finite state machines (RE) cannot count."

Comment 4 by briannorris@chromium.org, Jun 8 2018 

That doesn't mean we can't fix false positives... I'd much rather err on the side of false negatives. As it is, almost every kernel patchset I upload hits this.

Anyway, I've fixed the problem for myself with .checkpatch.conf ;)

Comment 5 by vovoy@chromium.org, Jun 8 2018 

It's really painful to check these false warnings every time, that's why I filed this bug :D
►

Sign in to add a comment