From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.5889.1591705473819106600 for ; Tue, 09 Jun 2020 05:24:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=OK9sxOep; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1591705473; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=B5Y0IvBVGz5/avw1n5NV6g+ymqbWGlTCDyWQV/DtOx8=; b=OK9sxOep+x/+YQZVlpQ7OmvMqSxacZFpN6x9cEFKZ0miXSATQJkyKokdMZvbt4QAgIq5xq bGyl5RxfMTRTHo3Kd/xCU3kdaA4HhZYSWjS112D/G1ShEPguhG5tldjlqj8/tCL+dIP5ZP Gu88+e1e281p9WXeU0ieVa18P7zf/bQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-27-25biHrF0ODK38slPravs4w-1; Tue, 09 Jun 2020 08:24:18 -0400 X-MC-Unique: 25biHrF0ODK38slPravs4w-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D5CE2107ACCA; Tue, 9 Jun 2020 12:24:16 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-127.ams2.redhat.com [10.36.113.127]) by smtp.corp.redhat.com (Postfix) with ESMTP id E27F082025; Tue, 9 Jun 2020 12:24:14 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci To: "Zhang, Shenglei" , "devel@edk2.groups.io" Cc: "Feng, Bob C" , Bret Barkelew , "Kinney, Michael D" , "Gao, Liming" , Sean Brogan References: <20200603084807.24484-1-shenglei.zhang@intel.com> From: "Laszlo Ersek" Message-ID: <7ac1918a-b153-fcb2-2aa6-46ec89534db6@redhat.com> Date: Tue, 9 Jun 2020 14:24:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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 >> Cc: Feng, Bob C ; Bret Barkelew >> ; Kinney, Michael D >> ; Gao, Liming ; Sean >> Brogan >> 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 >>> Cc: Bret Barkelew >>> Cc: Michael D Kinney >>> Cc: Liming Gao >>> Cc: Sean Brogan >>> >>> 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