Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues for edk2 on open ci
Shenglei: I have no other comment. This patch set is good to me. Reviewed-by: Liming Gao If no other comment, I suggest to merge this patch set before end of this week. Thanks Liming -Original Message- From: Zhang, Shenglei Sent: 2020年7月8日 9:57 To: Leif Lindholm Cc: devel@edk2.groups.io; Feng, Bob C ; Bret Barkelew ; Kinney, Michael D ; Gao, Liming ; Sean Brogan Subject: RE: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues for edk2 on open ci Comments below. > -Original Message- > From: Leif Lindholm > Sent: Tuesday, July 7, 2020 7:57 PM > To: Zhang, Shenglei > Cc: devel@edk2.groups.io; Feng, Bob C ; Bret > Barkelew ; Kinney, Michael D > ; Gao, Liming ; Sean > Brogan > Subject: Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc > issues for edk2 on open ci > > On Tue, Jul 07, 2020 at 02:24:14 +, Zhang, Shenglei wrote: > > Comments below. > > > > > -Original Message- > > > From: devel@edk2.groups.io On Behalf Of > > > Leif Lindholm > > > Sent: Monday, July 6, 2020 4:55 PM > > > To: devel@edk2.groups.io; Zhang, Shenglei > > > > > > Cc: Feng, Bob C ; Bret Barkelew > > > ; Kinney, Michael D > > > ; Gao, Liming ; > Sean > > > Brogan > > > Subject: Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check > > > Ecc > issues > > > for edk2 on open ci > > > > > > On Mon, Jul 06, 2020 at 16:48:30 +0800, 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. I appreciate receiving feedback and comments if > > > > someone find errors or false positive issues. > > > > > > > > I created a pipline of EccCheck for my forked edk2. Welcome > > > > everyone > to > > > > create pull request to test the quality of this plugin. > > > > My forked tree: https://github.com/shenglei10/edk2 > > > > > > > > And I also created some test cases for ECC plugin. Below are test cases. > > > > https://github.com/shenglei10/edk2/tree/ECC > > > > Results can be view in below azure server. > > > > > > > > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId= > 12 > > > &_a=summary > > > > > > > > Patches > > > > 1/16: It's a lib necessary for py3 to run Ecc on azure servers. > > > > > > > > 2/16: EccCheck.py is a plugin to report Ecc issues for commits. > > > > It can be > run > > > > on azure servers for open ci, or a local virtual environment. > > > > > > > > 3/16~16/16: 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. These patches add configuration in yaml files for Ecc > > > > check. > > > > > > > > Cc: Bob Feng > > > > Cc: Bret Barkelew > > > > Cc: Michael D Kinney > > > > Cc: Liming Gao > > > > Cc: Sean Brogan > > > > > > > > v2: Update 1/17, fix the bug that the script can't hanlde > > > > multiple > commits. > > > > > > > > v3: Update 1/17, set the only workalbe workspace is edk2 root directory. > > > > Update 2/17, designate the version of antlr4 is 4.7.1. > > > > Add 4/17~17/17. > > > > > > > > v4. Update 1/17, remove the function EdksetupRebuild(), instead add > > > > function SetupEnvironment(). Update variables' format and > > > > type > hints > > > > to pass flake8 and mypy. > > > > > > > > v5. Conver the former method to plugin solution, to align with > > > > other check points on open ci. > > > > > > > > v6. The 1/16 patch is missed in v5 series. Now add it in v6. > > > > > > > > v7. Fix a bug that Ecc plugin can not be run correctly under Linux OS. > > > > > > What is the bug? Where is it fixed? > > > > V6 plugin pipeline link: > > > https://dev.azure.com/shengleizhang/shengleizhang/_build/results?build > Id > =282&view=logs&j=9b8d87a5-bd93-
Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues for edk2 on open ci
Comments below. > -Original Message- > From: Leif Lindholm > Sent: Tuesday, July 7, 2020 7:57 PM > To: Zhang, Shenglei > Cc: devel@edk2.groups.io; Feng, Bob C ; Bret > Barkelew ; Kinney, Michael D > ; Gao, Liming ; Sean > Brogan > Subject: Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues > for edk2 on open ci > > On Tue, Jul 07, 2020 at 02:24:14 +, Zhang, Shenglei wrote: > > Comments below. > > > > > -Original Message- > > > From: devel@edk2.groups.io On Behalf Of Leif > > > Lindholm > > > Sent: Monday, July 6, 2020 4:55 PM > > > To: devel@edk2.groups.io; Zhang, Shenglei > > > Cc: Feng, Bob C ; Bret Barkelew > > > ; Kinney, Michael D > > > ; Gao, Liming ; > Sean > > > Brogan > > > Subject: Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc > issues > > > for edk2 on open ci > > > > > > On Mon, Jul 06, 2020 at 16:48:30 +0800, 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. I appreciate receiving feedback and comments if someone > > > > find errors or false positive issues. > > > > > > > > I created a pipline of EccCheck for my forked edk2. Welcome everyone > to > > > > create pull request to test the quality of this plugin. > > > > My forked tree: https://github.com/shenglei10/edk2 > > > > > > > > And I also created some test cases for ECC plugin. Below are test cases. > > > > https://github.com/shenglei10/edk2/tree/ECC > > > > Results can be view in below azure server. > > > > > > > > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=12 > > > &_a=summary > > > > > > > > Patches > > > > 1/16: It's a lib necessary for py3 to run Ecc on azure servers. > > > > > > > > 2/16: EccCheck.py is a plugin to report Ecc issues for commits. It can > > > > be > run > > > > on azure servers for open ci, or a local virtual environment. > > > > > > > > 3/16~16/16: 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. These patches add configuration in yaml files for Ecc > > > > check. > > > > > > > > Cc: Bob Feng > > > > Cc: Bret Barkelew > > > > Cc: Michael D Kinney > > > > Cc: Liming Gao > > > > Cc: Sean Brogan > > > > > > > > v2: Update 1/17, fix the bug that the script can't hanlde multiple > commits. > > > > > > > > v3: Update 1/17, set the only workalbe workspace is edk2 root directory. > > > > Update 2/17, designate the version of antlr4 is 4.7.1. > > > > Add 4/17~17/17. > > > > > > > > v4. Update 1/17, remove the function EdksetupRebuild(), instead add > > > > function SetupEnvironment(). Update variables' format and type > hints > > > > to pass flake8 and mypy. > > > > > > > > v5. Conver the former method to plugin solution, to align with > > > > other check points on open ci. > > > > > > > > v6. The 1/16 patch is missed in v5 series. Now add it in v6. > > > > > > > > v7. Fix a bug that Ecc plugin can not be run correctly under Linux OS. > > > > > > What is the bug? Where is it fixed? > > > > V6 plugin pipeline link: > > > https://dev.azure.com/shengleizhang/shengleizhang/_build/results?buildId > =282&view=logs&j=9b8d87a5-bd93-5358-8ffa-588e312c4376&t=282a77bb- > da29-5801-6299-2bbe15d5aabb > > v7 plugin pipeline link: > > > https://dev.azure.com/shengleizhang/shengleizhang/_build/results?buildId > =316&view=logs&j=5e3952f2-9c8a-5394-82c1-a803bcab3ca3&t=7ea9e85e- > 5c3d-5fcf-865e-8449f16cc745 > > > > For the same pull request, v6 plugin can't report out the Ecc issues under > Linux OS. > > Right. So *what* was the bug and *where* (which patch or patches) was > it fixed? This is information that should (in an abbreviated form) be > provided in the cover letter. > > &q
Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues for edk2 on open ci
On Tue, Jul 07, 2020 at 02:24:14 +, Zhang, Shenglei wrote: > Comments below. > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of Leif > > Lindholm > > Sent: Monday, July 6, 2020 4:55 PM > > To: devel@edk2.groups.io; Zhang, Shenglei > > Cc: Feng, Bob C ; Bret Barkelew > > ; Kinney, Michael D > > ; Gao, Liming ; Sean > > Brogan > > Subject: Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues > > for edk2 on open ci > > > > On Mon, Jul 06, 2020 at 16:48:30 +0800, 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. I appreciate receiving feedback and comments if someone > > > find errors or false positive issues. > > > > > > I created a pipline of EccCheck for my forked edk2. Welcome everyone to > > > create pull request to test the quality of this plugin. > > > My forked tree: https://github.com/shenglei10/edk2 > > > > > > And I also created some test cases for ECC plugin. Below are test cases. > > > https://github.com/shenglei10/edk2/tree/ECC > > > Results can be view in below azure server. > > > > > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=12 > > &_a=summary > > > > > > Patches > > > 1/16: It's a lib necessary for py3 to run Ecc on azure servers. > > > > > > 2/16: EccCheck.py is a plugin to report Ecc issues for commits. It can be > > > run > > > on azure servers for open ci, or a local virtual environment. > > > > > > 3/16~16/16: 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. These patches add configuration in yaml files for Ecc check. > > > > > > Cc: Bob Feng > > > Cc: Bret Barkelew > > > Cc: Michael D Kinney > > > Cc: Liming Gao > > > Cc: Sean Brogan > > > > > > v2: Update 1/17, fix the bug that the script can't hanlde multiple > > > commits. > > > > > > v3: Update 1/17, set the only workalbe workspace is edk2 root directory. > > > Update 2/17, designate the version of antlr4 is 4.7.1. > > > Add 4/17~17/17. > > > > > > v4. Update 1/17, remove the function EdksetupRebuild(), instead add > > > function SetupEnvironment(). Update variables' format and type hints > > > to pass flake8 and mypy. > > > > > > v5. Conver the former method to plugin solution, to align with > > > other check points on open ci. > > > > > > v6. The 1/16 patch is missed in v5 series. Now add it in v6. > > > > > > v7. Fix a bug that Ecc plugin can not be run correctly under Linux OS. > > > > What is the bug? Where is it fixed? > > V6 plugin pipeline link: > https://dev.azure.com/shengleizhang/shengleizhang/_build/results?buildId=282&view=logs&j=9b8d87a5-bd93-5358-8ffa-588e312c4376&t=282a77bb-da29-5801-6299-2bbe15d5aabb > v7 plugin pipeline link: > https://dev.azure.com/shengleizhang/shengleizhang/_build/results?buildId=316&view=logs&j=5e3952f2-9c8a-5394-82c1-a803bcab3ca3&t=7ea9e85e-5c3d-5fcf-865e-8449f16cc745 > > For the same pull request, v6 plugin can't report out the Ecc issues under > Linux OS. Right. So *what* was the bug and *where* (which patch or patches) was it fixed? This is information that should (in an abbreviated form) be provided in the cover letter. "Fix a bug" is data, not information. The same is true for "can not be run correctly". And neither tells me which patch I should look at. If this has been covered in a specific discussion thread, a link to the thread *and* mentioning which patch(es) it affects is sufficient. Regards, Leif > > Thanks, > Shenglei > > > > > / > > Leif > > > > > Shenglei Zhang (16): > > > pip-requirements.txt: Add Ecc required lib > > > .pytool/Plugin: Add a plugin EccCheck > > > MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check > > > ArmVirtPkg/ArmVirtPkg.ci.yaml: Add configuration for Ecc check > > > CryptoPkg/CryptoPkg.ci.yaml: Add configuration for Ecc check > > > EmulatorPkg/EmulatorPkg.ci.yaml: Add configuration
Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues for edk2 on open ci
Comments below. > -Original Message- > From: devel@edk2.groups.io On Behalf Of Leif > Lindholm > Sent: Monday, July 6, 2020 4:55 PM > To: devel@edk2.groups.io; Zhang, Shenglei > Cc: Feng, Bob C ; Bret Barkelew > ; Kinney, Michael D > ; Gao, Liming ; Sean > Brogan > Subject: Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues > for edk2 on open ci > > On Mon, Jul 06, 2020 at 16:48:30 +0800, 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. I appreciate receiving feedback and comments if someone > > find errors or false positive issues. > > > > I created a pipline of EccCheck for my forked edk2. Welcome everyone to > > create pull request to test the quality of this plugin. > > My forked tree: https://github.com/shenglei10/edk2 > > > > And I also created some test cases for ECC plugin. Below are test cases. > > https://github.com/shenglei10/edk2/tree/ECC > > Results can be view in below azure server. > > > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=12 > &_a=summary > > > > Patches > > 1/16: It's a lib necessary for py3 to run Ecc on azure servers. > > > > 2/16: EccCheck.py is a plugin to report Ecc issues for commits. It can be > > run > > on azure servers for open ci, or a local virtual environment. > > > > 3/16~16/16: 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. These patches add configuration in yaml files for Ecc check. > > > > Cc: Bob Feng > > Cc: Bret Barkelew > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Sean Brogan > > > > v2: Update 1/17, fix the bug that the script can't hanlde multiple commits. > > > > v3: Update 1/17, set the only workalbe workspace is edk2 root directory. > > Update 2/17, designate the version of antlr4 is 4.7.1. > > Add 4/17~17/17. > > > > v4. Update 1/17, remove the function EdksetupRebuild(), instead add > > function SetupEnvironment(). Update variables' format and type hints > > to pass flake8 and mypy. > > > > v5. Conver the former method to plugin solution, to align with > > other check points on open ci. > > > > v6. The 1/16 patch is missed in v5 series. Now add it in v6. > > > > v7. Fix a bug that Ecc plugin can not be run correctly under Linux OS. > > What is the bug? Where is it fixed? V6 plugin pipeline link: https://dev.azure.com/shengleizhang/shengleizhang/_build/results?buildId=282&view=logs&j=9b8d87a5-bd93-5358-8ffa-588e312c4376&t=282a77bb-da29-5801-6299-2bbe15d5aabb v7 plugin pipeline link: https://dev.azure.com/shengleizhang/shengleizhang/_build/results?buildId=316&view=logs&j=5e3952f2-9c8a-5394-82c1-a803bcab3ca3&t=7ea9e85e-5c3d-5fcf-865e-8449f16cc745 For the same pull request, v6 plugin can't report out the Ecc issues under Linux OS. Thanks, Shenglei > > / > Leif > > > Shenglei Zhang (16): > > pip-requirements.txt: Add Ecc required lib > > .pytool/Plugin: Add a plugin EccCheck > > MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check > > ArmVirtPkg/ArmVirtPkg.ci.yaml: Add configuration for Ecc check > > CryptoPkg/CryptoPkg.ci.yaml: Add configuration for Ecc check > > EmulatorPkg/EmulatorPkg.ci.yaml: Add configuration for Ecc check > > FatPkg/FatPkg.ci.yaml: Add configuration for Ecc check > > FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration for Ecc check > > MdePkg/MdePkg.ci.yaml: Add configuration for Ecc check > > NetworkPkg/NetworkPkg.ci.yaml: Add configuration for Ecc check > > OvmfPkg/OvmfPkg.ci.yaml: Add configuration for Ecc check > > PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml: Add configuration for Ecc check > > SecurityPkg/SecurityPkg.ci.yaml: Add configuration for Ecc check > > ShellPkg/ShellPkg.ci.yaml: Add configuration for Ecc check > > UefiCpuPkg/UefiCpuPkg.ci.yaml: Add configuration for Ecc check > > UnitTestFrameworkPkg: Add configuration for Ecc check in yaml file > > > > .pytool/Plugin/EccCheck/EccCheck.py | 267 ++ > > .pytool/Plugin/EccCheck/EccCheck_plug_in.yaml | 11 + > > .pytool/Plugin/EccCheck/Readme.md | 15 + > > ArmVirtPkg/ArmVirtPkg.ci.yaml | 11 + > >
Re: [edk2-devel] [PATCH v7 00/16] Add a plugin to check Ecc issues for edk2 on open ci
On Mon, Jul 06, 2020 at 16:48:30 +0800, 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. I appreciate receiving feedback and comments if someone > find errors or false positive issues. > > I created a pipline of EccCheck for my forked edk2. Welcome everyone to > create pull request to test the quality of this plugin. > My forked tree: https://github.com/shenglei10/edk2 > > And I also created some test cases for ECC plugin. Below are test cases. > https://github.com/shenglei10/edk2/tree/ECC > Results can be view in below azure server. > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=12&_a=summary > > Patches > 1/16: It's a lib necessary for py3 to run Ecc on azure servers. > > 2/16: EccCheck.py is a plugin to report Ecc issues for commits. It can be run > on azure servers for open ci, or a local virtual environment. > > 3/16~16/16: 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. These patches add configuration in yaml files for Ecc check. > > Cc: Bob Feng > Cc: Bret Barkelew > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Sean Brogan > > v2: Update 1/17, fix the bug that the script can't hanlde multiple commits. > > v3: Update 1/17, set the only workalbe workspace is edk2 root directory. > Update 2/17, designate the version of antlr4 is 4.7.1. > Add 4/17~17/17. > > v4. Update 1/17, remove the function EdksetupRebuild(), instead add > function SetupEnvironment(). Update variables' format and type hints > to pass flake8 and mypy. > > v5. Conver the former method to plugin solution, to align with > other check points on open ci. > > v6. The 1/16 patch is missed in v5 series. Now add it in v6. > > v7. Fix a bug that Ecc plugin can not be run correctly under Linux OS. What is the bug? Where is it fixed? / Leif > Shenglei Zhang (16): > pip-requirements.txt: Add Ecc required lib > .pytool/Plugin: Add a plugin EccCheck > MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check > ArmVirtPkg/ArmVirtPkg.ci.yaml: Add configuration for Ecc check > CryptoPkg/CryptoPkg.ci.yaml: Add configuration for Ecc check > EmulatorPkg/EmulatorPkg.ci.yaml: Add configuration for Ecc check > FatPkg/FatPkg.ci.yaml: Add configuration for Ecc check > FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration for Ecc check > MdePkg/MdePkg.ci.yaml: Add configuration for Ecc check > NetworkPkg/NetworkPkg.ci.yaml: Add configuration for Ecc check > OvmfPkg/OvmfPkg.ci.yaml: Add configuration for Ecc check > PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml: Add configuration for Ecc check > SecurityPkg/SecurityPkg.ci.yaml: Add configuration for Ecc check > ShellPkg/ShellPkg.ci.yaml: Add configuration for Ecc check > UefiCpuPkg/UefiCpuPkg.ci.yaml: Add configuration for Ecc check > UnitTestFrameworkPkg: Add configuration for Ecc check in yaml file > > .pytool/Plugin/EccCheck/EccCheck.py | 267 ++ > .pytool/Plugin/EccCheck/EccCheck_plug_in.yaml | 11 + > .pytool/Plugin/EccCheck/Readme.md | 15 + > ArmVirtPkg/ArmVirtPkg.ci.yaml | 11 + > CryptoPkg/CryptoPkg.ci.yaml | 11 + > EmulatorPkg/EmulatorPkg.ci.yaml | 11 + > FatPkg/FatPkg.ci.yaml | 11 + > FmpDevicePkg/FmpDevicePkg.ci.yaml | 11 + > MdeModulePkg/MdeModulePkg.ci.yaml | 11 + > MdePkg/MdePkg.ci.yaml | 11 + > NetworkPkg/NetworkPkg.ci.yaml | 11 + > OvmfPkg/OvmfPkg.ci.yaml | 11 + > PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml | 11 + > SecurityPkg/SecurityPkg.ci.yaml | 11 + > ShellPkg/ShellPkg.ci.yaml | 11 + > UefiCpuPkg/UefiCpuPkg.ci.yaml | 11 + > .../UnitTestFrameworkPkg.ci.yaml | 10 + > pip-requirements.txt | 1 + > 18 files changed, 447 insertions(+) > create mode 100644 .pytool/Plugin/EccCheck/EccCheck.py > create mode 100644 .pytool/Plugin/EccCheck/EccCheck_plug_in.yaml > create mode 100644 .pytool/Plugin/EccCheck/Readme.md > > -- > 2.18.0.windows.1 > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62088): https://edk2.groups.io/g/devel/message/62088 Mute This Topic: https://groups.io/mt/75329189/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
