REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
As planed we will enable Ecc check for edk2 on open ci. And they are
ready now, but these are V2 series. So I expect that contributors in
edk2 community can try using this script when reviewing. And I appreciate
receiving feedback and comments if someone find errors or false positive
issues.

I created a pipline of EccCheck for my forked edk2.
https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10&_a=summary

The patch series are big, so the commits are also pushed into my forked tree.
https://github.com/shenglei10/edk2/commits/ECC

Patches
1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not
     a pipline or script issue. But it is listed here for people willing
     to try this tool.
2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run
     on azure servers for open ci, or locally. Its usage is like
     PatchCheck.py.
3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local
     use, we need to type command
     "py -3 -m pip install antlr4-python3-runtime" first.
4/5: Windows-EccCheck.yml is a yaml file to configure the newly added
     pipline. The azure uses this to create a pipline.
5/5: We consider some cases that will report out Ecc issues but they won't
     be fixed, like submodule and industry standard related things. So we
     add two configuration fields "Exception" and "IgnoreFiles" for people
     to use. The patch is a example and the contents in the fields will be
     empty in final version.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>

v2: Update 2/5, fix the bug that the script can't hanlde multiple commits.

Fan, Zhiju (1):
  BaseTools:ECC needs to update the contents of CParser4

Shenglei Zhang (4):
  BaseTools/Scripts: Add EccCheck.py
  pip-requirements.txt: Add Ecc required lib
  .azurepiplines: Add a pipline to check ECC issues for commits
  MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check

 .azurepipelines/Windows-EccCheck.yml          |  38 ++
 BaseTools/Scripts/EccCheck.py                 | 433 ++++++++++++++++++
 .../Source/Python/Ecc/CParser4/CLexer.py      |   6 +-
 .../Source/Python/Ecc/CParser4/CListener.py   |   4 +-
 .../Source/Python/Ecc/CParser4/CParser.py     |  38 +-
 MdeModulePkg/MdeModulePkg.ci.yaml             |   8 +
 pip-requirements.txt                          |   1 +
 7 files changed, 505 insertions(+), 23 deletions(-)
 create mode 100644 .azurepipelines/Windows-EccCheck.yml
 create mode 100644 BaseTools/Scripts/EccCheck.py

-- 
2.18.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60631): https://edk2.groups.io/g/devel/message/60631
Mute This Topic: https://groups.io/mt/74645921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 06/03/20 10:48, Zhang, Shenglei wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
> As planed we will enable Ecc check for edk2 on open ci. And they are
> ready now, but these are V2 series. So I expect that contributors in
> edk2 community can try using this script when reviewing. And I appreciate
> receiving feedback and comments if someone find errors or false positive
> issues.
> 
> I created a pipline of EccCheck for my forked edk2.
> https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10&_a=summary
> 
> The patch series are big, so the commits are also pushed into my forked tree.
> https://github.com/shenglei10/edk2/commits/ECC
> 
> Patches
> 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not
>      a pipline or script issue. But it is listed here for people willing
>      to try this tool.
> 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run
>      on azure servers for open ci, or locally. Its usage is like
>      PatchCheck.py.
> 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local
>      use, we need to type command
>      "py -3 -m pip install antlr4-python3-runtime" first.
> 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added
>      pipline. The azure uses this to create a pipline.
> 5/5: We consider some cases that will report out Ecc issues but they won't
>      be fixed, like submodule and industry standard related things. So we
>      add two configuration fields "Exception" and "IgnoreFiles" for people
>      to use. The patch is a example and the contents in the fields will be
>      empty in final version.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> 
> v2: Update 2/5, fix the bug that the script can't hanlde multiple commits.

Thanks for the ExceptionList / IgnoreFiles features; I think they are
really important. I've run ECC in the past, and in some cases it is
*way* too strict and opinionated, so I'm sure we'll end up "training"
the ExceptionList entry for OvmfPkg.

Can you please explain how (if?) ECC is restricted to new code added by
a patch series? Patch#2 seems related, but I don't fully understand. It
says,

"It can only handle the issues, whose line number in CSV report
accurately map with their code in source code files."

Does that mean that CI performs a full ECC check, but filters out all
warning / error messages that do not refer to code lines added in the
patch series?

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60665): https://edk2.groups.io/g/devel/message/60665
Mute This Topic: https://groups.io/mt/74645921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Laszlo,

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, June 3, 2020 10:27 PM
> To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@intel.com>
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean
> Brogan <sean.brogan@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for
> edk2 on open ci
> 
> On 06/03/20 10:48, Zhang, Shenglei wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
> > As planed we will enable Ecc check for edk2 on open ci. And they are
> > ready now, but these are V2 series. So I expect that contributors in
> > edk2 community can try using this script when reviewing. And I appreciate
> > receiving feedback and comments if someone find errors or false positive
> > issues.
> >
> > I created a pipline of EccCheck for my forked edk2.
> >
> https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10
> &_a=summary
> >
> > The patch series are big, so the commits are also pushed into my forked
> tree.
> > https://github.com/shenglei10/edk2/commits/ECC
> >
> > Patches
> > 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not
> >      a pipline or script issue. But it is listed here for people willing
> >      to try this tool.
> > 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run
> >      on azure servers for open ci, or locally. Its usage is like
> >      PatchCheck.py.
> > 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local
> >      use, we need to type command
> >      "py -3 -m pip install antlr4-python3-runtime" first.
> > 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added
> >      pipline. The azure uses this to create a pipline.
> > 5/5: We consider some cases that will report out Ecc issues but they won't
> >      be fixed, like submodule and industry standard related things. So we
> >      add two configuration fields "Exception" and "IgnoreFiles" for people
> >      to use. The patch is a example and the contents in the fields will be
> >      empty in final version.
> >
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> >
> > v2: Update 2/5, fix the bug that the script can't hanlde multiple commits.
> 
> Thanks for the ExceptionList / IgnoreFiles features; I think they are
> really important. I've run ECC in the past, and in some cases it is
> *way* too strict and opinionated, so I'm sure we'll end up "training"
> the ExceptionList entry for OvmfPkg.
> 
> Can you please explain how (if?) ECC is restricted to new code added by
> a patch series? Patch#2 seems related, but I don't fully understand. It
> says,
> 
> "It can only handle the issues, whose line number in CSV report
> accurately map with their code in source code files."
> 
> Does that mean that CI performs a full ECC check, but filters out all
> warning / error messages that do not refer to code lines added in the
> patch series?
> 

Yes, I think you get the point.
Since there are plenty of Ecc issues existing in edk2, we need to filter out the
issues that are not caused by the patch to be checked in. The actual implementation
is to create a dictionary for modified files, in which the key word is the modified file and
the content is the line number scope for added code. Then if the issues in CSV report can
be mapped with the dictionary, they are marked as real issues.

And the scanning scope is the folder that the change is located in. For example,
Someone modifies "FmpDevicePkg\Library\FmpDependencyLib\FmpDependencyLib.c".
Then the scan scope is "FmpDevicePkg\Library\FmpDependencyLib". It's not a full check
for edk2, because it will cost much time.

Thanks,
Shenglei

> Thanks,
> Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60711): https://edk2.groups.io/g/devel/message/60711
Mute This Topic: https://groups.io/mt/74645921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 06/04/20 07:38, Zhang, Shenglei wrote:
> Hi Laszlo,
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, June 3, 2020 10:27 PM
>> To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@intel.com>
>> Cc: Feng, Bob C <bob.c.feng@intel.com>; Bret Barkelew
>> <Bret.Barkelew@microsoft.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean
>> Brogan <sean.brogan@microsoft.com>
>> Subject: Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for
>> edk2 on open ci
>>
>> On 06/03/20 10:48, Zhang, Shenglei wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
>>> As planed we will enable Ecc check for edk2 on open ci. And they are
>>> ready now, but these are V2 series. So I expect that contributors in
>>> edk2 community can try using this script when reviewing. And I appreciate
>>> receiving feedback and comments if someone find errors or false positive
>>> issues.
>>>
>>> I created a pipline of EccCheck for my forked edk2.
>>>
>> https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10
>> &_a=summary
>>>
>>> The patch series are big, so the commits are also pushed into my forked
>> tree.
>>> https://github.com/shenglei10/edk2/commits/ECC
>>>
>>> Patches
>>> 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not
>>>      a pipline or script issue. But it is listed here for people willing
>>>      to try this tool.
>>> 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run
>>>      on azure servers for open ci, or locally. Its usage is like
>>>      PatchCheck.py.
>>> 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local
>>>      use, we need to type command
>>>      "py -3 -m pip install antlr4-python3-runtime" first.
>>> 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added
>>>      pipline. The azure uses this to create a pipline.
>>> 5/5: We consider some cases that will report out Ecc issues but they won't
>>>      be fixed, like submodule and industry standard related things. So we
>>>      add two configuration fields "Exception" and "IgnoreFiles" for people
>>>      to use. The patch is a example and the contents in the fields will be
>>>      empty in final version.
>>>
>>> Cc: Bob Feng <bob.c.feng@intel.com>
>>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Sean Brogan <sean.brogan@microsoft.com>
>>>
>>> v2: Update 2/5, fix the bug that the script can't hanlde multiple commits.
>>
>> Thanks for the ExceptionList / IgnoreFiles features; I think they are
>> really important. I've run ECC in the past, and in some cases it is
>> *way* too strict and opinionated, so I'm sure we'll end up "training"
>> the ExceptionList entry for OvmfPkg.
>>
>> Can you please explain how (if?) ECC is restricted to new code added by
>> a patch series? Patch#2 seems related, but I don't fully understand. It
>> says,
>>
>> "It can only handle the issues, whose line number in CSV report
>> accurately map with their code in source code files."
>>
>> Does that mean that CI performs a full ECC check, but filters out all
>> warning / error messages that do not refer to code lines added in the
>> patch series?
>>
> 
> Yes, I think you get the point.
> Since there are plenty of Ecc issues existing in edk2, we need to filter out the
> issues that are not caused by the patch to be checked in. The actual implementation
> is to create a dictionary for modified files, in which the key word is the modified file and
> the content is the line number scope for added code. Then if the issues in CSV report can
> be mapped with the dictionary, they are marked as real issues.
> 
> And the scanning scope is the folder that the change is located in. For example,
> Someone modifies "FmpDevicePkg\Library\FmpDependencyLib\FmpDependencyLib.c".
> Then the scan scope is "FmpDevicePkg\Library\FmpDependencyLib". It's not a full check
> for edk2, because it will cost much time.

Thank you for the explanation!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60961): https://edk2.groups.io/g/devel/message/60961
Mute This Topic: https://groups.io/mt/74645921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Shenglei:
  Seemly, ECC tool update is not related to this patch serial. Can you separate it? 
  Besides, please update package yaml file with new ECC checker. 

Thanks
Liming
> -----Original Message-----
> From: Zhang, Shenglei <shenglei.zhang@intel.com>
> Sent: Wednesday, June 3, 2020 4:48 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>
> Subject: [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
> As planed we will enable Ecc check for edk2 on open ci. And they are
> ready now, but these are V2 series. So I expect that contributors in
> edk2 community can try using this script when reviewing. And I appreciate
> receiving feedback and comments if someone find errors or false positive
> issues.
> 
> I created a pipline of EccCheck for my forked edk2.
> https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10&_a=summary
> 
> The patch series are big, so the commits are also pushed into my forked tree.
> https://github.com/shenglei10/edk2/commits/ECC
> 
> Patches
> 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not
>      a pipline or script issue. But it is listed here for people willing
>      to try this tool.
> 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run
>      on azure servers for open ci, or locally. Its usage is like
>      PatchCheck.py.
> 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local
>      use, we need to type command
>      "py -3 -m pip install antlr4-python3-runtime" first.
> 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added
>      pipline. The azure uses this to create a pipline.
> 5/5: We consider some cases that will report out Ecc issues but they won't
>      be fixed, like submodule and industry standard related things. So we
>      add two configuration fields "Exception" and "IgnoreFiles" for people
>      to use. The patch is a example and the contents in the fields will be
>      empty in final version.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> 
> v2: Update 2/5, fix the bug that the script can't hanlde multiple commits.
> 
> Fan, Zhiju (1):
>   BaseTools:ECC needs to update the contents of CParser4
> 
> Shenglei Zhang (4):
>   BaseTools/Scripts: Add EccCheck.py
>   pip-requirements.txt: Add Ecc required lib
>   .azurepiplines: Add a pipline to check ECC issues for commits
>   MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check
> 
>  .azurepipelines/Windows-EccCheck.yml          |  38 ++
>  BaseTools/Scripts/EccCheck.py                 | 433 ++++++++++++++++++
>  .../Source/Python/Ecc/CParser4/CLexer.py      |   6 +-
>  .../Source/Python/Ecc/CParser4/CListener.py   |   4 +-
>  .../Source/Python/Ecc/CParser4/CParser.py     |  38 +-
>  MdeModulePkg/MdeModulePkg.ci.yaml             |   8 +
>  pip-requirements.txt                          |   1 +
>  7 files changed, 505 insertions(+), 23 deletions(-)
>  create mode 100644 .azurepipelines/Windows-EccCheck.yml
>  create mode 100644 BaseTools/Scripts/EccCheck.py
> 
> --
> 2.18.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60965): https://edk2.groups.io/g/devel/message/60965
Mute This Topic: https://groups.io/mt/74645921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-