On 06/04/20 07:38, Zhang, Shenglei wrote: > Hi Laszlo, > >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Wednesday, June 3, 2020 10:27 PM >> To: [email protected]; Zhang, Shenglei <[email protected]> >> Cc: Feng, Bob C <[email protected]>; Bret Barkelew >> <[email protected]>; Kinney, Michael D >> <[email protected]>; Gao, Liming <[email protected]>; Sean >> Brogan <[email protected]> >> 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 <[email protected]> >>> Cc: Bret Barkelew <[email protected]> >>> Cc: Michael D Kinney <[email protected]> >>> Cc: Liming Gao <[email protected]> >>> Cc: Sean Brogan <[email protected]> >>> >>> 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/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
