Thanks Gyorgy for your inputs.

My only concern with git hooks is that they are triggered when patches are pushed for review whereas checkpatch is something needs to be frequently run for non-trivial patchsets. The other issue is I am not sure whether we can pass different arguments to githooks whereas build system integration allows that (for example check the entire tree vs only the changed lines). Also having it part of regular build allows easier integration with work flow. Hence many OSS projects integrate this into regular build for this reason.

But as you say, perhaps the first solution is to download the script and run locally and I don't have a strong opinion against git hooks either. Whatever works best can be used.

Best Regards Soby Mathew

From: TF-M tf-m-bounces@lists.trustedfirmware.org On Behalf Of Gyorgy Szing via TF-M Sent: 02 September 2020 15:25 To: tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: Re: [TF-M] CI Checkpatch

Hi,

My two cents on the topic.

Sort term Sort term it is possible to clone the ci-scripts repository and to run the check-patch script locally (https://git.trustedfirmware.org/ci/tf-m-ci-scripts.git/tree/run-checkpatch.s...) . The script has some in-line documentation.

Long term I think this is an automation topic which may have a connection to the build system, but the right place for a solution in not there. When it comes to automation the best is to differentiate two use cases:

1. Centralized. This is where the automation happens on a server, and the main purpose is to keep authentic records about quality, or to drive delivery (i.e. push documents to hosting provider.) For this we have Jenkins. 2. De-centralized. This is when the automation is executed on the developers machine. How the user interacts with this system can be an implementation detail. The same scripts could be executed by git hooks, manually, or by the build system. I think the best solution would be to use git-hooks for decentralized automation, as this is there already, and has a well defined and standard interface towards the developer. The main problem with git hooks is, git as a policy leaves hook management as the responsibility of the developer, and there is no built-in way to deploy hooks to the developers machine. (This is due to security considerations.) As a solution to this issue multiple "hook frameworks" have been developed. I suggest investigating these and to build a decentralize automation solution on top of one of them. Ideally the same scripts executed by Jenkins could be executed by the framework too. Some contenders (and the language thy are developed with):

* https://github.com/icefox/git-hooks - bash * https://github.com/git-hooks/git-hooks -golang * https://github.com/gnustavo/Git-Hooks - perl

Of course there are many more.

/George

From: TF-M <tf-m-bounces@lists.trustedfirmware.orgmailto:tf-m-bounces@lists.trustedfirmware.org> On Behalf Of Soby Mathew via TF-M Sent: 02 September 2020 15:18 To: Karl Zhang <Karl.Zhang@arm.commailto:Karl.Zhang@arm.com>; Christopher Brand <chris.brand@cypress.commailto:chris.brand@cypress.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: Re: [TF-M] CI Checkpatch

Hi, Just my view on this, One of the things that will be helpful is to have is the checkpatch script imported into the project and have a `make checkpatch` build target. This will help to pipe clean check-patch errors from developer side before pushing patch for review. We could also make it a git hook but then I feel it is less convenient than having a regular build target.

Best Regards Soby Mathew

From: TF-M <tf-m-bounces@lists.trustedfirmware.orgmailto:tf-m-bounces@lists.trustedfirmware.org> On Behalf Of Karl Zhang via TF-M Sent: 02 September 2020 01:56 To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org; Christopher Brand <chris.brand@cypress.commailto:chris.brand@cypress.com> Cc: nd <nd@arm.commailto:nd@arm.com> Subject: Re: [TF-M] CI Checkpatch

Hi Chris,

The CI job was trigged from https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/3521, you could find a list of all jobs and status of each job on Gerrit page that triggered by that job, job list example can be found at the end of this email. Another way is from the link you mentioned, that is a pipeline of how CI jobs start from one stage to another stage, if you click on the node of each stage, "Triggered Builds" will list the related job and you can check the details of it.

The Open CI for TF-M is under development of Linaro, development plan and status can be found from https://developer.trustedfirmware.org/w/collaboration/openci/ , it is not stable at this moment that we are continuously addressing on. There is no latest document for detail introduction of current Open CI yet. Open CI - developer.trustedfirmware.orghttps://developer.trustedfirmware.org/w/collaboration/openci/ Milestone Deliverables Target Completion Status; M1: Planning, Handover and Deployment SOW and project plan Hand over from OCE to Developer Services developer.trustedfirmware.org For the checkpatch job, it is a part of the static check stage, the error from this stage won't impact the final CI score, we need more investigation before active all static checks. The CI jobs were not able to trigger manually. There is a request to Linaro for the requirement that already working on, hope it can be deployed to the public Open CI soon.

Job list example on Gerrit after CI job:

Passed: 4, Failed: 18, Not done: 0 AN519_GNUARM_ConfigRegressionIPCTfmLevel2_Release_BL2http://ci.trustedfirmware.org/job/tf-m-build-config/29147/ -1 AN519_GNUARM_ConfigRegressionIPC_Release_BL2http://ci.trustedfirmware.org/job/tf-m-build-config/29146/ -1 AN519_GNUARM_ConfigRegressionProfileS_Release_BL2http://ci.trustedfirmware.org/job/tf-m-build-config/29152/ -1 AN519_GNUARM_ConfigRegression_Release_BL2http://ci.trustedfirmware.org/job/tf-m-build-config/29144/ -1 .... checkpatchhttp://ci.trustedfirmware.org/job/tf-m-checkpatch/472/ -1 cppcheckhttp://ci.trustedfirmware.org/job/tf-m-cppcheck/472/ 1 lava_boothttp://ci.trustedfirmware.org/job/tf-m-build-and-test/474/ 1 lava_testhttp://ci.trustedfirmware.org/job/tf-m-build-and-test/474/ 1 psoc64_GNUARM_ConfigRegressionIPCTfmLevel2_Release_NOBL2http://ci.trustedfirmware.org/job/tf-m-build-config/29157/ -1 psoc64_GNUARM_ConfigRegressionIPC_Release_NOBL2http://ci.trustedfirmware.org/job/tf-m-build-config/29154/ -1 tf-m-buildhttp://ci.trustedfirmware.org/job/tf-m-build-and-test/479/ -1 tf-m-build-docshttp://ci.trustedfirmware.org/job/tf-m-build-docs/647/ 1

Thanks, Karl Zhang ________________________________ From: TF-M <tf-m-bounces@lists.trustedfirmware.orgmailto:tf-m-bounces@lists.trustedfirmware.org> on behalf of Christopher Brand via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Sent: Tuesday, September 1, 2020 12:35 AM To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Subject: [TF-M] CI Checkpatch

So I see the CI system runs checkpatch, but I don't see any mention of checkpatch under the "docs" directory, or in any of the gerrit reviews, or even on the mailing list. The output in the CI system, as far as I can see, is not particularly useful (I followed the link posted on my review to https://ci.trustedfirmware.org/blue/organizations/jenkins/tf-m-static/detail... but could find anything indicating what issue was actually found).

Is there any documentation on how we can run checkptach manually? Or on how to see what the CI version is actually complaining about? Should I just be ignoring the CI checkpatch errors?

Thanks,

Chris Brand

Sr Prin Software Engr, MCD: WIRELESS

Cypress Semiconductor Corp.

An Infineon Technologies Company

#320-13700 International Place, Richmond, British Columbia V6V 2X8 Canada

www.infineon.comhttp://www.infineon.com www.cypress.comhttp://www.cypress.com

This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.