public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v10 00/16] Add a plugin to check Ecc issues for edk2 on open ci
@ 2020-08-14  7:43 Zhang, Shenglei
  2020-08-14  7:43 ` [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck Zhang, Shenglei
  0 siblings, 1 reply; 4+ messages in thread
From: Zhang, Shenglei @ 2020-08-14  7:43 UTC (permalink / raw)
  To: devel; +Cc: Bob Feng, Bret Barkelew, Michael D Kinney, Liming Gao,
	Sean Brogan

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 <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 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.

v8. Enable error code config section to ignore certain kinds of issues,
    which are always false positive in partial Ecc scaning.
    All patches except 2/16 have been R-B and are not updated in v8 series.
    To avoid making noise in community, I only send cover letter and 2/16 patch.

v9. Update 2/16, 3/16, 5/16 and 16/16.
    1. Enable directory path for "IgnoreFiles" section in xxxPkg.yaml. So that
       users can skip a certain directory and don't need to fill in with file names.
    2. Add submodule pathes in "IgnoreFiles" in MdeModulePkg.ci.yaml,
       CryptoPkg.ci.yaml and UnitTestFrameworkPkg.ci.yaml.

v10. Update 2/16 patch, adding a step to revert code when ecc plugin ends.
     All patches except 2/16 have been R-B and are not updated in v10 series.
     To avoid making noise in community, I only send cover letter and 2/16 patch.

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           | 309 ++++++++++++++++++
 .pytool/Plugin/EccCheck/EccCheck_plug_in.yaml |  11 +
 .pytool/Plugin/EccCheck/Readme.md             |  15 +
 ArmVirtPkg/ArmVirtPkg.ci.yaml                 |  12 +
 CryptoPkg/CryptoPkg.ci.yaml                   |  13 +
 EmulatorPkg/EmulatorPkg.ci.yaml               |  12 +
 FatPkg/FatPkg.ci.yaml                         |  12 +
 FmpDevicePkg/FmpDevicePkg.ci.yaml             |  12 +
 MdeModulePkg/MdeModulePkg.ci.yaml             |  14 +
 MdePkg/MdePkg.ci.yaml                         |  12 +
 NetworkPkg/NetworkPkg.ci.yaml                 |  12 +
 OvmfPkg/OvmfPkg.ci.yaml                       |  12 +
 PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml         |  13 +
 SecurityPkg/SecurityPkg.ci.yaml               |  12 +
 ShellPkg/ShellPkg.ci.yaml                     |  12 +
 UefiCpuPkg/UefiCpuPkg.ci.yaml                 |  12 +
 .../UnitTestFrameworkPkg.ci.yaml              |  12 +
 pip-requirements.txt                          |   1 +
 18 files changed, 508 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck
  2020-08-14  7:43 [PATCH v10 00/16] Add a plugin to check Ecc issues for edk2 on open ci Zhang, Shenglei
@ 2020-08-14  7:43 ` Zhang, Shenglei
  2020-08-14  8:11   ` Liming Gao
       [not found]   ` <162B1435DE941D73.20848@groups.io>
  0 siblings, 2 replies; 4+ messages in thread
From: Zhang, Shenglei @ 2020-08-14  7:43 UTC (permalink / raw)
  To: devel; +Cc: Sean Brogan, Bret Barkelew, Michael D Kinney, Liming Gao

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
EccCheck is a plugin to report Ecc issues for code in pull request
, which will be run on open ci.
But note not each kind of issue could be reported out.
It can only handle the issues, whose line number in CSV report
accurately map with their code in source code files. And Ecc issues
about comments can also be handled.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 .pytool/Plugin/EccCheck/EccCheck.py           | 309 ++++++++++++++++++
 .pytool/Plugin/EccCheck/EccCheck_plug_in.yaml |  11 +
 .pytool/Plugin/EccCheck/Readme.md             |  15 +
 3 files changed, 335 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

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
new file mode 100644
index 0000000000..eee1ff7a77
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -0,0 +1,309 @@
+# @file EccCheck.py
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+import os
+import shutil
+import re
+import csv
+import xml.dom.minidom
+from typing import List, Dict, Tuple
+import logging
+from io import StringIO
+from edk2toolext.environment import shell_environment
+from edk2toolext.environment.plugintypes.ci_build_plugin import ICiBuildPlugin
+from edk2toolext.environment.var_dict import VarDict
+from edk2toollib.utility_functions import RunCmd
+
+
+class EccCheck(ICiBuildPlugin):
+    """
+    A CiBuildPlugin that finds the Ecc issues of newly added code in pull request.
+
+    Configuration options:
+    "EccCheck": {
+        "ExceptionList": [],
+        "IgnoreFiles": []
+    },
+    """
+
+    ReModifyFile = re.compile(r'[B-Q,S-Z]+[\d]*\t(.*)')
+    FindModifyFile = re.compile(r'\+\+\+ b\/(.*)')
+    LineScopePattern = (r'@@ -\d*\,*\d* \+\d*\,*\d* @@.*')
+    LineNumRange = re.compile(r'@@ -\d*\,*\d* \+(\d*)\,*(\d*) @@.*')
+
+    def GetTestName(self, packagename: str, environment: VarDict) -> tuple:
+        """ Provide the testcase name and classname for use in reporting
+            testclassname: a descriptive string for the testcase can include whitespace
+            classname: should be patterned <packagename>.<plugin>.<optionally any unique condition>
+
+            Args:
+              packagename: string containing name of package to build
+              environment: The VarDict for the test to run in
+            Returns:
+                a tuple containing the testcase name and the classname
+                (testcasename, classname)
+        """
+        return ("Check for efi coding style for " + packagename, packagename + ".EccCheck")
+
+    ##
+    # External function of plugin.  This function is used to perform the task of the ci_build_plugin Plugin
+    #
+    #   - package is the edk2 path to package.  This means workspace/packagepath relative.
+    #   - edk2path object configured with workspace and packages path
+    #   - PkgConfig Object (dict) for the pkg
+    #   - EnvConfig Object
+    #   - Plugin Manager Instance
+    #   - Plugin Helper Obj Instance
+    #   - Junit Logger
+    #   - output_stream the StringIO output stream from this plugin via logging
+    def RunBuildPlugin(self, packagename, Edk2pathObj, pkgconfig, environment, PLM, PLMHelper, tc, output_stream=None):
+        edk2_path = Edk2pathObj.WorkspacePath
+        python_path = os.path.join(edk2_path, "BaseTools", "Source", "Python")
+        env = shell_environment.GetEnvironment()
+        env.set_shell_var('PYTHONPATH', python_path)
+        env.set_shell_var('WORKSPACE', edk2_path)
+        self.ECC_PASS = True
+        self.ApplyConfig(pkgconfig, edk2_path, packagename)
+        modify_dir_list = self.GetModifyDir(packagename)
+        patch = self.GetDiff(packagename)
+        ecc_diff_range = self.GetDiffRange(patch, packagename, edk2_path)
+        self.GenerateEccReport(modify_dir_list, ecc_diff_range, edk2_path)
+        ecc_log = os.path.join(edk2_path, "Ecc.log")
+        self.RevertCode()
+        if self.ECC_PASS:
+            tc.SetSuccess()
+            self.RemoveFile(ecc_log)
+            return 0
+        else:
+            with open(ecc_log, encoding='utf8') as output:
+                ecc_output = output.readlines()
+                for line in ecc_output:
+                    logging.error(line.strip())
+            self.RemoveFile(ecc_log)
+            tc.SetFailed("EccCheck failed for {0}".format(packagename), "Ecc detected issues")
+            return 1
+
+    def RevertCode(self) -> None:
+        submoudle_params = "submodule update --init"
+        RunCmd("git", submoudle_params)
+        reset_params = "reset HEAD --hard"
+        RunCmd("git", reset_params)
+
+    def GetDiff(self, pkg: str) -> List[str]:
+        return_buffer = StringIO()
+        params = "diff --unified=0 origin/master HEAD"
+        RunCmd("git", params, outstream=return_buffer)
+        p = return_buffer.getvalue().strip()
+        patch = p.split("\n")
+        return_buffer.close()
+
+        return patch
+
+    def RemoveFile(self, file: str) -> None:
+        if os.path.exists(file):
+            os.remove(file)
+        return
+
+    def GetModifyDir(self, pkg: str) -> List[str]:
+        return_buffer = StringIO()
+        params = "diff --name-status" + ' HEAD' + ' origin/master'
+        RunCmd("git", params, outstream=return_buffer)
+        p1 = return_buffer.getvalue().strip()
+        dir_list = p1.split("\n")
+        return_buffer.close()
+        modify_dir_list = []
+        for modify_dir in dir_list:
+            file_path = self.ReModifyFile.findall(modify_dir)
+            if file_path:
+                file_dir = os.path.dirname(file_path[0])
+            else:
+                continue
+            if pkg in file_dir and file_dir != pkg:
+                modify_dir_list.append('%s' % file_dir)
+            else:
+                continue
+
+        modify_dir_list = list(set(modify_dir_list))
+        return modify_dir_list
+
+    def GetDiffRange(self, patch_diff: List[str], pkg: str, workingdir: str) -> Dict[str, List[Tuple[int, int]]]:
+        IsDelete = True
+        StartCheck = False
+        range_directory: Dict[str, List[Tuple[int, int]]] = {}
+        for line in patch_diff:
+            modify_file = self.FindModifyFile.findall(line)
+            if modify_file and pkg in modify_file[0] and not StartCheck and os.path.isfile(modify_file[0]):
+                modify_file_comment_dic = self.GetCommentRange(modify_file[0], workingdir)
+                IsDelete = False
+                StartCheck = True
+                modify_file_dic = modify_file[0]
+                modify_file_dic = modify_file_dic.replace("/", os.sep)
+                range_directory[modify_file_dic] = []
+            elif line.startswith('--- '):
+                StartCheck = False
+            elif re.match(self.LineScopePattern, line, re.I) and not IsDelete and StartCheck:
+                start_line = self.LineNumRange.search(line).group(1)
+                line_range = self.LineNumRange.search(line).group(2)
+                if not line_range:
+                    line_range = '1'
+                range_directory[modify_file_dic].append((int(start_line), int(start_line) + int(line_range) - 1))
+                for i in modify_file_comment_dic:
+                    if int(i[0]) <= int(start_line) <= int(i[1]):
+                        range_directory[modify_file_dic].append(i)
+        return range_directory
+
+    def GetCommentRange(self, modify_file: str, workingdir: str) -> List[Tuple[int, int]]:
+        modify_file_path = os.path.join(workingdir, modify_file)
+        with open(modify_file_path) as f:
+            line_no = 1
+            comment_range: List[Tuple[int, int]] = []
+            Start = False
+            for line in f:
+                if line.startswith('/**'):
+                    start_no = line_no
+                    Start = True
+                if line.startswith('**/') and Start:
+                    end_no = line_no
+                    Start = False
+                    comment_range.append((int(start_no), int(end_no)))
+                line_no += 1
+
+        if comment_range and comment_range[0][0] == 1:
+            del comment_range[0]
+        return comment_range
+
+    def GenerateEccReport(self, modify_dir_list: List[str], ecc_diff_range: Dict[str, List[Tuple[int, int]]],
+                          edk2_path: str) -> None:
+        ecc_need = False
+        ecc_run = True
+        config = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "config.ini")
+        exception = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+        report = os.path.join(edk2_path, "Ecc.csv")
+        for modify_dir in modify_dir_list:
+            target = os.path.join(edk2_path, modify_dir)
+            logging.info('Run ECC tool for the commit in %s' % modify_dir)
+            ecc_need = True
+            ecc_params = "-c {0} -e {1} -t {2} -r {3}".format(config, exception, target, report)
+            return_code = RunCmd("Ecc", ecc_params, workingdir=edk2_path)
+            if return_code != 0:
+                ecc_run = False
+                break
+            if not ecc_run:
+                logging.error('Fail to run ECC tool')
+            self.ParseEccReport(ecc_diff_range, edk2_path)
+
+        if not ecc_need:
+            logging.info("Doesn't need run ECC check")
+
+        revert_params = "checkout -- {}".format(exception)
+        RunCmd("git", revert_params)
+        return
+
+    def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], edk2_path: str) -> None:
+        ecc_log = os.path.join(edk2_path, "Ecc.log")
+        ecc_csv = "Ecc.csv"
+        file = os.listdir(edk2_path)
+        row_lines = []
+        ignore_error_code = self.GetIgnoreErrorCode()
+        if ecc_csv in file:
+            with open(ecc_csv) as csv_file:
+                reader = csv.reader(csv_file)
+                for row in reader:
+                    for modify_file in ecc_diff_range:
+                        if modify_file in row[3]:
+                            for i in ecc_diff_range[modify_file]:
+                                line_no = int(row[4])
+                                if i[0] <= line_no <= i[1] and row[1] not in ignore_error_code:
+                                    row[0] = '\nEFI coding style error'
+                                    row[1] = 'Error code: ' + row[1]
+                                    row[3] = 'file: ' + row[3]
+                                    row[4] = 'Line number: ' + row[4]
+                                    row_line = '\n  *'.join(row)
+                                    row_lines.append(row_line)
+                                    break
+                            break
+        if row_lines:
+            self.ECC_PASS = False
+
+        with open(ecc_log, 'a') as log:
+            all_line = '\n'.join(row_lines)
+            all_line = all_line + '\n'
+            log.writelines(all_line)
+        return
+
+    def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, pkg: str) -> None:
+        if "IgnoreFiles" in pkgconfig:
+            for a in pkgconfig["IgnoreFiles"]:
+                a = os.path.join(edk2_path, pkg, a)
+                a = a.replace(os.sep, "/")
+
+                logging.info("Ignoring Files {0}".format(a))
+                if os.path.exists(a):
+                    if os.path.isfile(a):
+                        self.RemoveFile(a)
+                    elif os.path.isdir(a):
+                        shutil.rmtree(a)
+                else:
+                    logging.error("EccCheck.IgnoreInf -> {0} not found in filesystem.  Invalid ignore files".format(a))
+
+        if "ExceptionList" in pkgconfig:
+            exception_list = pkgconfig["ExceptionList"]
+            exception_xml = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+            try:
+                logging.info("Appending exceptions")
+                self.AppendException(exception_list, exception_xml)
+            except Exception as e:
+                logging.error("Fail to apply exceptions")
+                raise e
+        return
+
+    def AppendException(self, exception_list: List[str], exception_xml: str) -> None:
+        error_code_list = exception_list[::2]
+        keyword_list = exception_list[1::2]
+        dom_tree = xml.dom.minidom.parse(exception_xml)
+        root_node = dom_tree.documentElement
+        for error_code, keyword in zip(error_code_list, keyword_list):
+            customer_node = dom_tree.createElement("Exception")
+            keyword_node = dom_tree.createElement("KeyWord")
+            keyword_node_text_value = dom_tree.createTextNode(keyword)
+            keyword_node.appendChild(keyword_node_text_value)
+            customer_node.appendChild(keyword_node)
+            error_code_node = dom_tree.createElement("ErrorID")
+            error_code_text_value = dom_tree.createTextNode(error_code)
+            error_code_node.appendChild(error_code_text_value)
+            customer_node.appendChild(error_code_node)
+            root_node.appendChild(customer_node)
+        with open(exception_xml, 'w') as f:
+            dom_tree.writexml(f, indent='', addindent='', newl='\n', encoding='UTF-8')
+        return
+
+    def GetIgnoreErrorCode(self) -> set:
+        """
+        Below are kinds of error code that are accurate in ecc scanning of edk2 level.
+        But EccCheck plugin is partial scanning so they are always false positive issues.
+        The mapping relationship of error code and error message is listed BaseTools/Sourc/Python/Ecc/EccToolError.py
+        """
+        ignore_error_code = {
+                             "10000",
+                             "10001",
+                             "10002",
+                             "10003",
+                             "10004",
+                             "10005",
+                             "10006",
+                             "10007",
+                             "10008",
+                             "10009",
+                             "10010",
+                             "10011",
+                             "10012",
+                             "10013",
+                             "10015",
+                             "10016",
+                             "10017",
+                             "10022",
+                            }
+        return ignore_error_code
diff --git a/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml b/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml
new file mode 100644
index 0000000000..0d121ecb18
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml
@@ -0,0 +1,11 @@
+## @file
+# CiBuildPlugin used to check Ecc issues
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+{
+  "scope": "cibuild",
+  "name": "EccCheck Test",
+  "module": "EccCheck"
+}
diff --git a/.pytool/Plugin/EccCheck/Readme.md b/.pytool/Plugin/EccCheck/Readme.md
new file mode 100644
index 0000000000..85c439a5ed
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/Readme.md
@@ -0,0 +1,15 @@
+# EFI Coding style Check Plugin
+
+This CiBuildPlugin finds the Ecc issues of newly added code in pull request.
+
+## Configuration
+
+The plugin can be configured to ignore certain files and issues.
+
+"EccCheck": {
+        "ExceptionList": [],
+        "IgnoreFiles": []
+    },
+    """
+
+OPTIONAL List of file to ignore.
-- 
2.18.0.windows.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck
  2020-08-14  7:43 ` [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck Zhang, Shenglei
@ 2020-08-14  8:11   ` Liming Gao
       [not found]   ` <162B1435DE941D73.20848@groups.io>
  1 sibling, 0 replies; 4+ messages in thread
From: Liming Gao @ 2020-08-14  8:11 UTC (permalink / raw)
  To: Zhang, Shenglei, devel@edk2.groups.io
  Cc: Sean Brogan, Bret Barkelew, Kinney, Michael D

Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@intel.com> 
Sent: 2020年8月14日 15:44
To: devel@edk2.groups.io
Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
EccCheck is a plugin to report Ecc issues for code in pull request , which will be run on open ci.
But note not each kind of issue could be reported out.
It can only handle the issues, whose line number in CSV report accurately map with their code in source code files. And Ecc issues about comments can also be handled.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 .pytool/Plugin/EccCheck/EccCheck.py           | 309 ++++++++++++++++++
 .pytool/Plugin/EccCheck/EccCheck_plug_in.yaml |  11 +
 .pytool/Plugin/EccCheck/Readme.md             |  15 +
 3 files changed, 335 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

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
new file mode 100644
index 0000000000..eee1ff7a77
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -0,0 +1,309 @@
+# @file EccCheck.py
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> # 
+SPDX-License-Identifier: BSD-2-Clause-Patent ##
+
+import os
+import shutil
+import re
+import csv
+import xml.dom.minidom
+from typing import List, Dict, Tuple
+import logging
+from io import StringIO
+from edk2toolext.environment import shell_environment from 
+edk2toolext.environment.plugintypes.ci_build_plugin import 
+ICiBuildPlugin from edk2toolext.environment.var_dict import VarDict 
+from edk2toollib.utility_functions import RunCmd
+
+
+class EccCheck(ICiBuildPlugin):
+    """
+    A CiBuildPlugin that finds the Ecc issues of newly added code in pull request.
+
+    Configuration options:
+    "EccCheck": {
+        "ExceptionList": [],
+        "IgnoreFiles": []
+    },
+    """
+
+    ReModifyFile = re.compile(r'[B-Q,S-Z]+[\d]*\t(.*)')
+    FindModifyFile = re.compile(r'\+\+\+ b\/(.*)')
+    LineScopePattern = (r'@@ -\d*\,*\d* \+\d*\,*\d* @@.*')
+    LineNumRange = re.compile(r'@@ -\d*\,*\d* \+(\d*)\,*(\d*) @@.*')
+
+    def GetTestName(self, packagename: str, environment: VarDict) -> tuple:
+        """ Provide the testcase name and classname for use in reporting
+            testclassname: a descriptive string for the testcase can include whitespace
+            classname: should be patterned 
+ <packagename>.<plugin>.<optionally any unique condition>
+
+            Args:
+              packagename: string containing name of package to build
+              environment: The VarDict for the test to run in
+            Returns:
+                a tuple containing the testcase name and the classname
+                (testcasename, classname)
+        """
+        return ("Check for efi coding style for " + packagename, 
+ packagename + ".EccCheck")
+
+    ##
+    # External function of plugin.  This function is used to perform the task of the ci_build_plugin Plugin
+    #
+    #   - package is the edk2 path to package.  This means workspace/packagepath relative.
+    #   - edk2path object configured with workspace and packages path
+    #   - PkgConfig Object (dict) for the pkg
+    #   - EnvConfig Object
+    #   - Plugin Manager Instance
+    #   - Plugin Helper Obj Instance
+    #   - Junit Logger
+    #   - output_stream the StringIO output stream from this plugin via logging
+    def RunBuildPlugin(self, packagename, Edk2pathObj, pkgconfig, environment, PLM, PLMHelper, tc, output_stream=None):
+        edk2_path = Edk2pathObj.WorkspacePath
+        python_path = os.path.join(edk2_path, "BaseTools", "Source", "Python")
+        env = shell_environment.GetEnvironment()
+        env.set_shell_var('PYTHONPATH', python_path)
+        env.set_shell_var('WORKSPACE', edk2_path)
+        self.ECC_PASS = True
+        self.ApplyConfig(pkgconfig, edk2_path, packagename)
+        modify_dir_list = self.GetModifyDir(packagename)
+        patch = self.GetDiff(packagename)
+        ecc_diff_range = self.GetDiffRange(patch, packagename, edk2_path)
+        self.GenerateEccReport(modify_dir_list, ecc_diff_range, edk2_path)
+        ecc_log = os.path.join(edk2_path, "Ecc.log")
+        self.RevertCode()
+        if self.ECC_PASS:
+            tc.SetSuccess()
+            self.RemoveFile(ecc_log)
+            return 0
+        else:
+            with open(ecc_log, encoding='utf8') as output:
+                ecc_output = output.readlines()
+                for line in ecc_output:
+                    logging.error(line.strip())
+            self.RemoveFile(ecc_log)
+            tc.SetFailed("EccCheck failed for {0}".format(packagename), "Ecc detected issues")
+            return 1
+
+    def RevertCode(self) -> None:
+        submoudle_params = "submodule update --init"
+        RunCmd("git", submoudle_params)
+        reset_params = "reset HEAD --hard"
+        RunCmd("git", reset_params)
+
+    def GetDiff(self, pkg: str) -> List[str]:
+        return_buffer = StringIO()
+        params = "diff --unified=0 origin/master HEAD"
+        RunCmd("git", params, outstream=return_buffer)
+        p = return_buffer.getvalue().strip()
+        patch = p.split("\n")
+        return_buffer.close()
+
+        return patch
+
+    def RemoveFile(self, file: str) -> None:
+        if os.path.exists(file):
+            os.remove(file)
+        return
+
+    def GetModifyDir(self, pkg: str) -> List[str]:
+        return_buffer = StringIO()
+        params = "diff --name-status" + ' HEAD' + ' origin/master'
+        RunCmd("git", params, outstream=return_buffer)
+        p1 = return_buffer.getvalue().strip()
+        dir_list = p1.split("\n")
+        return_buffer.close()
+        modify_dir_list = []
+        for modify_dir in dir_list:
+            file_path = self.ReModifyFile.findall(modify_dir)
+            if file_path:
+                file_dir = os.path.dirname(file_path[0])
+            else:
+                continue
+            if pkg in file_dir and file_dir != pkg:
+                modify_dir_list.append('%s' % file_dir)
+            else:
+                continue
+
+        modify_dir_list = list(set(modify_dir_list))
+        return modify_dir_list
+
+    def GetDiffRange(self, patch_diff: List[str], pkg: str, workingdir: str) -> Dict[str, List[Tuple[int, int]]]:
+        IsDelete = True
+        StartCheck = False
+        range_directory: Dict[str, List[Tuple[int, int]]] = {}
+        for line in patch_diff:
+            modify_file = self.FindModifyFile.findall(line)
+            if modify_file and pkg in modify_file[0] and not StartCheck and os.path.isfile(modify_file[0]):
+                modify_file_comment_dic = self.GetCommentRange(modify_file[0], workingdir)
+                IsDelete = False
+                StartCheck = True
+                modify_file_dic = modify_file[0]
+                modify_file_dic = modify_file_dic.replace("/", os.sep)
+                range_directory[modify_file_dic] = []
+            elif line.startswith('--- '):
+                StartCheck = False
+            elif re.match(self.LineScopePattern, line, re.I) and not IsDelete and StartCheck:
+                start_line = self.LineNumRange.search(line).group(1)
+                line_range = self.LineNumRange.search(line).group(2)
+                if not line_range:
+                    line_range = '1'
+                range_directory[modify_file_dic].append((int(start_line), int(start_line) + int(line_range) - 1))
+                for i in modify_file_comment_dic:
+                    if int(i[0]) <= int(start_line) <= int(i[1]):
+                        range_directory[modify_file_dic].append(i)
+        return range_directory
+
+    def GetCommentRange(self, modify_file: str, workingdir: str) -> List[Tuple[int, int]]:
+        modify_file_path = os.path.join(workingdir, modify_file)
+        with open(modify_file_path) as f:
+            line_no = 1
+            comment_range: List[Tuple[int, int]] = []
+            Start = False
+            for line in f:
+                if line.startswith('/**'):
+                    start_no = line_no
+                    Start = True
+                if line.startswith('**/') and Start:
+                    end_no = line_no
+                    Start = False
+                    comment_range.append((int(start_no), int(end_no)))
+                line_no += 1
+
+        if comment_range and comment_range[0][0] == 1:
+            del comment_range[0]
+        return comment_range
+
+    def GenerateEccReport(self, modify_dir_list: List[str], ecc_diff_range: Dict[str, List[Tuple[int, int]]],
+                          edk2_path: str) -> None:
+        ecc_need = False
+        ecc_run = True
+        config = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "config.ini")
+        exception = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+        report = os.path.join(edk2_path, "Ecc.csv")
+        for modify_dir in modify_dir_list:
+            target = os.path.join(edk2_path, modify_dir)
+            logging.info('Run ECC tool for the commit in %s' % modify_dir)
+            ecc_need = True
+            ecc_params = "-c {0} -e {1} -t {2} -r {3}".format(config, exception, target, report)
+            return_code = RunCmd("Ecc", ecc_params, workingdir=edk2_path)
+            if return_code != 0:
+                ecc_run = False
+                break
+            if not ecc_run:
+                logging.error('Fail to run ECC tool')
+            self.ParseEccReport(ecc_diff_range, edk2_path)
+
+        if not ecc_need:
+            logging.info("Doesn't need run ECC check")
+
+        revert_params = "checkout -- {}".format(exception)
+        RunCmd("git", revert_params)
+        return
+
+    def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], edk2_path: str) -> None:
+        ecc_log = os.path.join(edk2_path, "Ecc.log")
+        ecc_csv = "Ecc.csv"
+        file = os.listdir(edk2_path)
+        row_lines = []
+        ignore_error_code = self.GetIgnoreErrorCode()
+        if ecc_csv in file:
+            with open(ecc_csv) as csv_file:
+                reader = csv.reader(csv_file)
+                for row in reader:
+                    for modify_file in ecc_diff_range:
+                        if modify_file in row[3]:
+                            for i in ecc_diff_range[modify_file]:
+                                line_no = int(row[4])
+                                if i[0] <= line_no <= i[1] and row[1] not in ignore_error_code:
+                                    row[0] = '\nEFI coding style error'
+                                    row[1] = 'Error code: ' + row[1]
+                                    row[3] = 'file: ' + row[3]
+                                    row[4] = 'Line number: ' + row[4]
+                                    row_line = '\n  *'.join(row)
+                                    row_lines.append(row_line)
+                                    break
+                            break
+        if row_lines:
+            self.ECC_PASS = False
+
+        with open(ecc_log, 'a') as log:
+            all_line = '\n'.join(row_lines)
+            all_line = all_line + '\n'
+            log.writelines(all_line)
+        return
+
+    def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, pkg: str) -> None:
+        if "IgnoreFiles" in pkgconfig:
+            for a in pkgconfig["IgnoreFiles"]:
+                a = os.path.join(edk2_path, pkg, a)
+                a = a.replace(os.sep, "/")
+
+                logging.info("Ignoring Files {0}".format(a))
+                if os.path.exists(a):
+                    if os.path.isfile(a):
+                        self.RemoveFile(a)
+                    elif os.path.isdir(a):
+                        shutil.rmtree(a)
+                else:
+                    logging.error("EccCheck.IgnoreInf -> {0} not found 
+ in filesystem.  Invalid ignore files".format(a))
+
+        if "ExceptionList" in pkgconfig:
+            exception_list = pkgconfig["ExceptionList"]
+            exception_xml = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+            try:
+                logging.info("Appending exceptions")
+                self.AppendException(exception_list, exception_xml)
+            except Exception as e:
+                logging.error("Fail to apply exceptions")
+                raise e
+        return
+
+    def AppendException(self, exception_list: List[str], exception_xml: str) -> None:
+        error_code_list = exception_list[::2]
+        keyword_list = exception_list[1::2]
+        dom_tree = xml.dom.minidom.parse(exception_xml)
+        root_node = dom_tree.documentElement
+        for error_code, keyword in zip(error_code_list, keyword_list):
+            customer_node = dom_tree.createElement("Exception")
+            keyword_node = dom_tree.createElement("KeyWord")
+            keyword_node_text_value = dom_tree.createTextNode(keyword)
+            keyword_node.appendChild(keyword_node_text_value)
+            customer_node.appendChild(keyword_node)
+            error_code_node = dom_tree.createElement("ErrorID")
+            error_code_text_value = dom_tree.createTextNode(error_code)
+            error_code_node.appendChild(error_code_text_value)
+            customer_node.appendChild(error_code_node)
+            root_node.appendChild(customer_node)
+        with open(exception_xml, 'w') as f:
+            dom_tree.writexml(f, indent='', addindent='', newl='\n', encoding='UTF-8')
+        return
+
+    def GetIgnoreErrorCode(self) -> set:
+        """
+        Below are kinds of error code that are accurate in ecc scanning of edk2 level.
+        But EccCheck plugin is partial scanning so they are always false positive issues.
+        The mapping relationship of error code and error message is listed BaseTools/Sourc/Python/Ecc/EccToolError.py
+        """
+        ignore_error_code = {
+                             "10000",
+                             "10001",
+                             "10002",
+                             "10003",
+                             "10004",
+                             "10005",
+                             "10006",
+                             "10007",
+                             "10008",
+                             "10009",
+                             "10010",
+                             "10011",
+                             "10012",
+                             "10013",
+                             "10015",
+                             "10016",
+                             "10017",
+                             "10022",
+                            }
+        return ignore_error_code
diff --git a/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml b/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml
new file mode 100644
index 0000000000..0d121ecb18
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml
@@ -0,0 +1,11 @@
+## @file
+# CiBuildPlugin used to check Ecc issues # # Copyright (c) 2020, Intel 
+Corporation. All rights reserved.<BR> # SPDX-License-Identifier: 
+BSD-2-Clause-Patent ## {
+  "scope": "cibuild",
+  "name": "EccCheck Test",
+  "module": "EccCheck"
+}
diff --git a/.pytool/Plugin/EccCheck/Readme.md b/.pytool/Plugin/EccCheck/Readme.md
new file mode 100644
index 0000000000..85c439a5ed
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/Readme.md
@@ -0,0 +1,15 @@
+# EFI Coding style Check Plugin
+
+This CiBuildPlugin finds the Ecc issues of newly added code in pull request.
+
+## Configuration
+
+The plugin can be configured to ignore certain files and issues.
+
+"EccCheck": {
+        "ExceptionList": [],
+        "IgnoreFiles": []
+    },
+    """
+
+OPTIONAL List of file to ignore.
--
2.18.0.windows.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck
       [not found]   ` <162B1435DE941D73.20848@groups.io>
@ 2020-08-17  4:10     ` Liming Gao
  0 siblings, 0 replies; 4+ messages in thread
From: Liming Gao @ 2020-08-17  4:10 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming, Zhang, Shenglei
  Cc: Sean Brogan, Bret Barkelew, Kinney, Michael D

This patch set have been merged 7f0b28415cb464832155d5b3ff6eb63612f58645..98946ef352238ac233dbf8df34510fc4e30d9e02.

Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao
Sent: 2020年8月14日 16:12
To: Zhang, Shenglei <shenglei.zhang@intel.com>; devel@edk2.groups.io
Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck

Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@intel.com>
Sent: 2020年8月14日 15:44
To: devel@edk2.groups.io
Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
EccCheck is a plugin to report Ecc issues for code in pull request , which will be run on open ci.
But note not each kind of issue could be reported out.
It can only handle the issues, whose line number in CSV report accurately map with their code in source code files. And Ecc issues about comments can also be handled.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 .pytool/Plugin/EccCheck/EccCheck.py           | 309 ++++++++++++++++++
 .pytool/Plugin/EccCheck/EccCheck_plug_in.yaml |  11 +
 .pytool/Plugin/EccCheck/Readme.md             |  15 +
 3 files changed, 335 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

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
new file mode 100644
index 0000000000..eee1ff7a77
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -0,0 +1,309 @@
+# @file EccCheck.py
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> #
+SPDX-License-Identifier: BSD-2-Clause-Patent ##
+
+import os
+import shutil
+import re
+import csv
+import xml.dom.minidom
+from typing import List, Dict, Tuple
+import logging
+from io import StringIO
+from edk2toolext.environment import shell_environment from 
+edk2toolext.environment.plugintypes.ci_build_plugin import 
+ICiBuildPlugin from edk2toolext.environment.var_dict import VarDict 
+from edk2toollib.utility_functions import RunCmd
+
+
+class EccCheck(ICiBuildPlugin):
+    """
+    A CiBuildPlugin that finds the Ecc issues of newly added code in pull request.
+
+    Configuration options:
+    "EccCheck": {
+        "ExceptionList": [],
+        "IgnoreFiles": []
+    },
+    """
+
+    ReModifyFile = re.compile(r'[B-Q,S-Z]+[\d]*\t(.*)')
+    FindModifyFile = re.compile(r'\+\+\+ b\/(.*)')
+    LineScopePattern = (r'@@ -\d*\,*\d* \+\d*\,*\d* @@.*')
+    LineNumRange = re.compile(r'@@ -\d*\,*\d* \+(\d*)\,*(\d*) @@.*')
+
+    def GetTestName(self, packagename: str, environment: VarDict) -> tuple:
+        """ Provide the testcase name and classname for use in reporting
+            testclassname: a descriptive string for the testcase can include whitespace
+            classname: should be patterned 
+ <packagename>.<plugin>.<optionally any unique condition>
+
+            Args:
+              packagename: string containing name of package to build
+              environment: The VarDict for the test to run in
+            Returns:
+                a tuple containing the testcase name and the classname
+                (testcasename, classname)
+        """
+        return ("Check for efi coding style for " + packagename, 
+ packagename + ".EccCheck")
+
+    ##
+    # External function of plugin.  This function is used to perform the task of the ci_build_plugin Plugin
+    #
+    #   - package is the edk2 path to package.  This means workspace/packagepath relative.
+    #   - edk2path object configured with workspace and packages path
+    #   - PkgConfig Object (dict) for the pkg
+    #   - EnvConfig Object
+    #   - Plugin Manager Instance
+    #   - Plugin Helper Obj Instance
+    #   - Junit Logger
+    #   - output_stream the StringIO output stream from this plugin via logging
+    def RunBuildPlugin(self, packagename, Edk2pathObj, pkgconfig, environment, PLM, PLMHelper, tc, output_stream=None):
+        edk2_path = Edk2pathObj.WorkspacePath
+        python_path = os.path.join(edk2_path, "BaseTools", "Source", "Python")
+        env = shell_environment.GetEnvironment()
+        env.set_shell_var('PYTHONPATH', python_path)
+        env.set_shell_var('WORKSPACE', edk2_path)
+        self.ECC_PASS = True
+        self.ApplyConfig(pkgconfig, edk2_path, packagename)
+        modify_dir_list = self.GetModifyDir(packagename)
+        patch = self.GetDiff(packagename)
+        ecc_diff_range = self.GetDiffRange(patch, packagename, edk2_path)
+        self.GenerateEccReport(modify_dir_list, ecc_diff_range, edk2_path)
+        ecc_log = os.path.join(edk2_path, "Ecc.log")
+        self.RevertCode()
+        if self.ECC_PASS:
+            tc.SetSuccess()
+            self.RemoveFile(ecc_log)
+            return 0
+        else:
+            with open(ecc_log, encoding='utf8') as output:
+                ecc_output = output.readlines()
+                for line in ecc_output:
+                    logging.error(line.strip())
+            self.RemoveFile(ecc_log)
+            tc.SetFailed("EccCheck failed for {0}".format(packagename), "Ecc detected issues")
+            return 1
+
+    def RevertCode(self) -> None:
+        submoudle_params = "submodule update --init"
+        RunCmd("git", submoudle_params)
+        reset_params = "reset HEAD --hard"
+        RunCmd("git", reset_params)
+
+    def GetDiff(self, pkg: str) -> List[str]:
+        return_buffer = StringIO()
+        params = "diff --unified=0 origin/master HEAD"
+        RunCmd("git", params, outstream=return_buffer)
+        p = return_buffer.getvalue().strip()
+        patch = p.split("\n")
+        return_buffer.close()
+
+        return patch
+
+    def RemoveFile(self, file: str) -> None:
+        if os.path.exists(file):
+            os.remove(file)
+        return
+
+    def GetModifyDir(self, pkg: str) -> List[str]:
+        return_buffer = StringIO()
+        params = "diff --name-status" + ' HEAD' + ' origin/master'
+        RunCmd("git", params, outstream=return_buffer)
+        p1 = return_buffer.getvalue().strip()
+        dir_list = p1.split("\n")
+        return_buffer.close()
+        modify_dir_list = []
+        for modify_dir in dir_list:
+            file_path = self.ReModifyFile.findall(modify_dir)
+            if file_path:
+                file_dir = os.path.dirname(file_path[0])
+            else:
+                continue
+            if pkg in file_dir and file_dir != pkg:
+                modify_dir_list.append('%s' % file_dir)
+            else:
+                continue
+
+        modify_dir_list = list(set(modify_dir_list))
+        return modify_dir_list
+
+    def GetDiffRange(self, patch_diff: List[str], pkg: str, workingdir: str) -> Dict[str, List[Tuple[int, int]]]:
+        IsDelete = True
+        StartCheck = False
+        range_directory: Dict[str, List[Tuple[int, int]]] = {}
+        for line in patch_diff:
+            modify_file = self.FindModifyFile.findall(line)
+            if modify_file and pkg in modify_file[0] and not StartCheck and os.path.isfile(modify_file[0]):
+                modify_file_comment_dic = self.GetCommentRange(modify_file[0], workingdir)
+                IsDelete = False
+                StartCheck = True
+                modify_file_dic = modify_file[0]
+                modify_file_dic = modify_file_dic.replace("/", os.sep)
+                range_directory[modify_file_dic] = []
+            elif line.startswith('--- '):
+                StartCheck = False
+            elif re.match(self.LineScopePattern, line, re.I) and not IsDelete and StartCheck:
+                start_line = self.LineNumRange.search(line).group(1)
+                line_range = self.LineNumRange.search(line).group(2)
+                if not line_range:
+                    line_range = '1'
+                range_directory[modify_file_dic].append((int(start_line), int(start_line) + int(line_range) - 1))
+                for i in modify_file_comment_dic:
+                    if int(i[0]) <= int(start_line) <= int(i[1]):
+                        range_directory[modify_file_dic].append(i)
+        return range_directory
+
+    def GetCommentRange(self, modify_file: str, workingdir: str) -> List[Tuple[int, int]]:
+        modify_file_path = os.path.join(workingdir, modify_file)
+        with open(modify_file_path) as f:
+            line_no = 1
+            comment_range: List[Tuple[int, int]] = []
+            Start = False
+            for line in f:
+                if line.startswith('/**'):
+                    start_no = line_no
+                    Start = True
+                if line.startswith('**/') and Start:
+                    end_no = line_no
+                    Start = False
+                    comment_range.append((int(start_no), int(end_no)))
+                line_no += 1
+
+        if comment_range and comment_range[0][0] == 1:
+            del comment_range[0]
+        return comment_range
+
+    def GenerateEccReport(self, modify_dir_list: List[str], ecc_diff_range: Dict[str, List[Tuple[int, int]]],
+                          edk2_path: str) -> None:
+        ecc_need = False
+        ecc_run = True
+        config = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "config.ini")
+        exception = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+        report = os.path.join(edk2_path, "Ecc.csv")
+        for modify_dir in modify_dir_list:
+            target = os.path.join(edk2_path, modify_dir)
+            logging.info('Run ECC tool for the commit in %s' % modify_dir)
+            ecc_need = True
+            ecc_params = "-c {0} -e {1} -t {2} -r {3}".format(config, exception, target, report)
+            return_code = RunCmd("Ecc", ecc_params, workingdir=edk2_path)
+            if return_code != 0:
+                ecc_run = False
+                break
+            if not ecc_run:
+                logging.error('Fail to run ECC tool')
+            self.ParseEccReport(ecc_diff_range, edk2_path)
+
+        if not ecc_need:
+            logging.info("Doesn't need run ECC check")
+
+        revert_params = "checkout -- {}".format(exception)
+        RunCmd("git", revert_params)
+        return
+
+    def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], edk2_path: str) -> None:
+        ecc_log = os.path.join(edk2_path, "Ecc.log")
+        ecc_csv = "Ecc.csv"
+        file = os.listdir(edk2_path)
+        row_lines = []
+        ignore_error_code = self.GetIgnoreErrorCode()
+        if ecc_csv in file:
+            with open(ecc_csv) as csv_file:
+                reader = csv.reader(csv_file)
+                for row in reader:
+                    for modify_file in ecc_diff_range:
+                        if modify_file in row[3]:
+                            for i in ecc_diff_range[modify_file]:
+                                line_no = int(row[4])
+                                if i[0] <= line_no <= i[1] and row[1] not in ignore_error_code:
+                                    row[0] = '\nEFI coding style error'
+                                    row[1] = 'Error code: ' + row[1]
+                                    row[3] = 'file: ' + row[3]
+                                    row[4] = 'Line number: ' + row[4]
+                                    row_line = '\n  *'.join(row)
+                                    row_lines.append(row_line)
+                                    break
+                            break
+        if row_lines:
+            self.ECC_PASS = False
+
+        with open(ecc_log, 'a') as log:
+            all_line = '\n'.join(row_lines)
+            all_line = all_line + '\n'
+            log.writelines(all_line)
+        return
+
+    def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, pkg: str) -> None:
+        if "IgnoreFiles" in pkgconfig:
+            for a in pkgconfig["IgnoreFiles"]:
+                a = os.path.join(edk2_path, pkg, a)
+                a = a.replace(os.sep, "/")
+
+                logging.info("Ignoring Files {0}".format(a))
+                if os.path.exists(a):
+                    if os.path.isfile(a):
+                        self.RemoveFile(a)
+                    elif os.path.isdir(a):
+                        shutil.rmtree(a)
+                else:
+                    logging.error("EccCheck.IgnoreInf -> {0} not found 
+ in filesystem.  Invalid ignore files".format(a))
+
+        if "ExceptionList" in pkgconfig:
+            exception_list = pkgconfig["ExceptionList"]
+            exception_xml = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+            try:
+                logging.info("Appending exceptions")
+                self.AppendException(exception_list, exception_xml)
+            except Exception as e:
+                logging.error("Fail to apply exceptions")
+                raise e
+        return
+
+    def AppendException(self, exception_list: List[str], exception_xml: str) -> None:
+        error_code_list = exception_list[::2]
+        keyword_list = exception_list[1::2]
+        dom_tree = xml.dom.minidom.parse(exception_xml)
+        root_node = dom_tree.documentElement
+        for error_code, keyword in zip(error_code_list, keyword_list):
+            customer_node = dom_tree.createElement("Exception")
+            keyword_node = dom_tree.createElement("KeyWord")
+            keyword_node_text_value = dom_tree.createTextNode(keyword)
+            keyword_node.appendChild(keyword_node_text_value)
+            customer_node.appendChild(keyword_node)
+            error_code_node = dom_tree.createElement("ErrorID")
+            error_code_text_value = dom_tree.createTextNode(error_code)
+            error_code_node.appendChild(error_code_text_value)
+            customer_node.appendChild(error_code_node)
+            root_node.appendChild(customer_node)
+        with open(exception_xml, 'w') as f:
+            dom_tree.writexml(f, indent='', addindent='', newl='\n', encoding='UTF-8')
+        return
+
+    def GetIgnoreErrorCode(self) -> set:
+        """
+        Below are kinds of error code that are accurate in ecc scanning of edk2 level.
+        But EccCheck plugin is partial scanning so they are always false positive issues.
+        The mapping relationship of error code and error message is listed BaseTools/Sourc/Python/Ecc/EccToolError.py
+        """
+        ignore_error_code = {
+                             "10000",
+                             "10001",
+                             "10002",
+                             "10003",
+                             "10004",
+                             "10005",
+                             "10006",
+                             "10007",
+                             "10008",
+                             "10009",
+                             "10010",
+                             "10011",
+                             "10012",
+                             "10013",
+                             "10015",
+                             "10016",
+                             "10017",
+                             "10022",
+                            }
+        return ignore_error_code
diff --git a/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml b/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml
new file mode 100644
index 0000000000..0d121ecb18
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/EccCheck_plug_in.yaml
@@ -0,0 +1,11 @@
+## @file
+# CiBuildPlugin used to check Ecc issues # # Copyright (c) 2020, Intel 
+Corporation. All rights reserved.<BR> # SPDX-License-Identifier:
+BSD-2-Clause-Patent ## {
+  "scope": "cibuild",
+  "name": "EccCheck Test",
+  "module": "EccCheck"
+}
diff --git a/.pytool/Plugin/EccCheck/Readme.md b/.pytool/Plugin/EccCheck/Readme.md
new file mode 100644
index 0000000000..85c439a5ed
--- /dev/null
+++ b/.pytool/Plugin/EccCheck/Readme.md
@@ -0,0 +1,15 @@
+# EFI Coding style Check Plugin
+
+This CiBuildPlugin finds the Ecc issues of newly added code in pull request.
+
+## Configuration
+
+The plugin can be configured to ignore certain files and issues.
+
+"EccCheck": {
+        "ExceptionList": [],
+        "IgnoreFiles": []
+    },
+    """
+
+OPTIONAL List of file to ignore.
--
2.18.0.windows.1





^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-08-17  4:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-14  7:43 [PATCH v10 00/16] Add a plugin to check Ecc issues for edk2 on open ci Zhang, Shenglei
2020-08-14  7:43 ` [PATCH v10 02/16] .pytool/Plugin: Add a plugin EccCheck Zhang, Shenglei
2020-08-14  8:11   ` Liming Gao
     [not found]   ` <162B1435DE941D73.20848@groups.io>
2020-08-17  4:10     ` [edk2-devel] " Liming Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox