thesofproject / sof

Sound Open Firmware
Other
531 stars 307 forks source link

[BUG] Checkpatch scans only last commit #1699

Closed jajanusz closed 4 years ago

jajanusz commented 5 years ago

Describe the bug When there are multiple commits in PR, then sof-ci/jenkins/pr-checkpatch scans only the last one.

F.e. in https://github.com/thesofproject/sof/pull/1611 it should show camel case error for each commit, not only for the last one.

To Reproduce Make PR with 2+ commits where each have checkpatch issues. Checkpatch errors will be reported only for the last one.

Reproduction Rate 100%

Expected behavior I expect to have report for each commit.

Impact Annoyance.

Environment N/A

xiulipan commented 5 years ago

@jajanusz It check all commits in log https://sof-ci.01.org/sofpr/PR1611/build2702/checkpatch/

The checkpatch will spilt check each commit.

I will try once locally to see if there is such issue

--------------------------------------
Commit 467d76ae16ea ("test: mux_copy")
--------------------------------------
CHECK: Avoid CamelCase: <CMUnitTest>
#467: FILE: test/cmocka/src/audio/mux/mux_copy.c:291:
+   struct CMUnitTest tests[ARRAY_SIZE(valid_formats) * ARRAY_SIZE(masks)];

total: 0 errors, 0 warnings, 1 checks, 517 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Commit 467d76ae16ea ("test: mux_copy") has style problems, please review.
----------------------------------------
Commit 1f2114cfebec ("test: demux_copy")
----------------------------------------
total: 0 errors, 0 warnings, 0 checks, 327 lines checked

Commit 1f2114cfebec ("test: demux_copy") has no obvious style problems and is ready for submission.
-----------------------------------------
Commit 64c1257e6120 ("test: mux_prepare")
-----------------------------------------
total: 0 errors, 0 warnings, 0 checks, 135 lines checked

Commit 64c1257e6120 ("test: mux_prepare") has no obvious style problems and is ready for submission.
---------------------------------------------------
Commit 1a78c9d00b8d ("test: mux calc_sample_s32le")
---------------------------------------------------
total: 0 errors, 0 warnings, 0 checks, 161 lines checked

Commit 1a78c9d00b8d ("test: mux calc_sample_s32le") has no obvious style problems and is ready for submission.
---------------------------------------------------
Commit 9d72e583efa3 ("test: mux calc_sample_s24le")
---------------------------------------------------
total: 0 errors, 0 warnings, 0 checks, 164 lines checked
jajanusz commented 5 years ago

@xiulipan I see it shows various commits IDs, but error is shown only for the one. Try to run it f.e. for demux_copy and mux_copy commit and both have issues, but it shows issue only for mux_copy.

xiulipan commented 5 years ago

@jajanusz I try to run ./scripts/checkpatch.pl --no-tree --codespell --strict -g 467d76ae16ea~..9d72e583efa3

locally and it indeed return the same logs.

but if I run

./scripts/checkpatch.pl --no-tree --codespell --strict -g 467d76ae16ea
./scripts/checkpatch.pl --no-tree --codespell --strict -g 1f2114cfebec 

There will be two different. I think that would be check patch issue. Will check it.

Also try with the latest checkpatch from Linux repo, the same issue here.

jajanusz commented 5 years ago

@xiulipan maybe it's checkpatch script issue and not CI issue if it works with kernel's checkpatch. So it may be caused by https://github.com/thesofproject/sof/issues/1649

xiulipan commented 5 years ago

@jajanusz I also tested with the latest checkpatch form kernel repo, it have the same issue. In one context, the CamelCase error will only report once. I think we should have some patch to the checkpatch.

mengdonglin commented 4 years ago

@xiulipan @jajanusz Is this issue fixed?

xiulipan commented 4 years ago

@mengdonglin No if we do not raise issue to Linux. Do not have time to learn and play with checkpatch scripts.

Maybe we should create a bug report for checkpatch, need to find out where to create an issue.

jajanusz commented 4 years ago

fixed