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.sh)
. 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:
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):
Of course there are many more.
/George
From: TF-M <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.com>; Christopher Brand <chris.brand@cypress.com>;
tf-m@lists.trustedfirmware.org
Cc: nd <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.org>
On Behalf Of Karl Zhang via TF-M
Sent: 02 September 2020 01:56
To: tf-m@lists.trustedfirmware.org; Christopher Brand <chris.brand@cypress.com>
Cc: nd <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.
|
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
|
-1 |
|
|
-1 |
|
|
-1 |
|
|
-1 |
....
|
-1 |
|
|
1 |
|
|
1 |
|
|
1 |
|
|
-1 |
|
|
-1 |
|
|
-1 |
|
|
1 |
Thanks,
Karl Zhang
From: TF-M <tf-m-bounces@lists.trustedfirmware.org> on behalf of Christopher Brand via TF-M <tf-m@lists.trustedfirmware.org>
Sent: Tuesday, September 1, 2020 12:35 AM
To: tf-m@lists.trustedfirmware.org <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/tf-m-static/466/pipeline 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.com
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.