[PATCH v1 0/4] Allow EccCheck to run on other repositories

[PATCH v1 0/4] Allow EccCheck to run on other repositories

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

EccCheck currently makes some assumptions on its working
environment that prevent it from running it in other repositories.
For instance, the workspace is assumed to be pointing to the edk2
repository path, which can be wrong.
This patch-set aims to allow the EccCheck tool to run on the
edk2-platforms repository, with edk2 being a submodule of
edk2-platforms.

Pierre Gondois (4):
  .pytool/EccCheck: Locate BaseTools dir with EDK_TOOLS_PATH
  .pytool/EccCheck: Rename edk2_path as workspace_path
  .pytool/EccCheck: Check ecc_csv exists
  .pytool/EccCheck: Set PACKAGES_PATH env var in Ecc

 .pytool/Plugin/EccCheck/EccCheck.py | 46 +++++++++++++++--------------
 1 file changed, 24 insertions(+), 22 deletions(-)

-- 
2.17.1

[PATCH v1 1/4] .pytool/EccCheck: Locate BaseTools dir with EDK_TOOLS_PATH

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

The BaseTools directory is currently being located as a
sub-directory of the WORKSPACE env var. This might not be
true in other environments. Cf EDKII Build Specification,
s4.1.3 "Build Process Restrictions":
  There is no restriction on the location of the EDK_TOOLS_PATH,
  it may be located within a directory identified as the
  WORKSPACE directory, or in any other location that is
  accessible on the development workstation.

Locate the BaseTools directory using EDK_TOOLS_PATH instead.

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 <gaoliming@byosoft.com.cn>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .pytool/Plugin/EccCheck/EccCheck.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index eee1ff7a77b5..bb93446441de 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -1,5 +1,6 @@
 # @file EccCheck.py
 #
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
 # Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
@@ -61,16 +62,17 @@ class EccCheck(ICiBuildPlugin):
     #   - 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")
+        basetools_path = environment.GetValue("EDK_TOOLS_PATH")
+        python_path = os.path.join(basetools_path, "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)
+        self.ApplyConfig(pkgconfig, edk2_path, basetools_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)
+        self.GenerateEccReport(modify_dir_list, ecc_diff_range, edk2_path, basetools_path)
         ecc_log = os.path.join(edk2_path, "Ecc.log")
         self.RevertCode()
         if self.ECC_PASS:
@@ -176,11 +178,11 @@ class EccCheck(ICiBuildPlugin):
         return comment_range
 
     def GenerateEccReport(self, modify_dir_list: List[str], ecc_diff_range: Dict[str, List[Tuple[int, int]]],
-                          edk2_path: str) -> None:
+                           edk2_path: str, basetools_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")
+        config = os.path.join(basetools_path, "Source", "Python", "Ecc", "config.ini")
+        exception = os.path.join(basetools_path, "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)
@@ -234,7 +236,7 @@ class EccCheck(ICiBuildPlugin):
             log.writelines(all_line)
         return
 
-    def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, pkg: str) -> None:
+    def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, basetools_path: str, pkg: str) -> None:
         if "IgnoreFiles" in pkgconfig:
             for a in pkgconfig["IgnoreFiles"]:
                 a = os.path.join(edk2_path, pkg, a)
@@ -251,7 +253,7 @@ class EccCheck(ICiBuildPlugin):
 
         if "ExceptionList" in pkgconfig:
             exception_list = pkgconfig["ExceptionList"]
-            exception_xml = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+            exception_xml = os.path.join(basetools_path, "Source", "Python", "Ecc", "exception.xml")
             try:
                 logging.info("Appending exceptions")
                 self.AppendException(exception_list, exception_xml)
-- 
2.17.1

[PATCH v1 2/4] .pytool/EccCheck: Rename edk2_path as workspace_path

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

The edk2 path and the workspace path are identical when running
Ecc on edk2. When running Ecc on another repository
(e.g.: edk2-platforms with edk2 as a submodule of edk2-platforms),
these directories are different. Indeed, in the latter configuration,
Ecc must run git commands on the tested repository, i.e. the workspace
directory, edk2-platforms.

Thus, rename edk2_path as workspace_path.

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 <gaoliming@byosoft.com.cn>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .pytool/Plugin/EccCheck/EccCheck.py | 32 ++++++++++++++---------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index bb93446441de..fff317f23110 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -61,19 +61,19 @@ class EccCheck(ICiBuildPlugin):
     #   - 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
+        workspace_path = Edk2pathObj.WorkspacePath
         basetools_path = environment.GetValue("EDK_TOOLS_PATH")
         python_path = os.path.join(basetools_path, "Source", "Python")
         env = shell_environment.GetEnvironment()
         env.set_shell_var('PYTHONPATH', python_path)
-        env.set_shell_var('WORKSPACE', edk2_path)
+        env.set_shell_var('WORKSPACE', workspace_path)
         self.ECC_PASS = True
-        self.ApplyConfig(pkgconfig, edk2_path, basetools_path, packagename)
+        self.ApplyConfig(pkgconfig, workspace_path, basetools_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, basetools_path)
-        ecc_log = os.path.join(edk2_path, "Ecc.log")
+        ecc_diff_range = self.GetDiffRange(patch, packagename, workspace_path)
+        self.GenerateEccReport(modify_dir_list, ecc_diff_range, workspace_path, basetools_path)
+        ecc_log = os.path.join(workspace_path, "Ecc.log")
         self.RevertCode()
         if self.ECC_PASS:
             tc.SetSuccess()
@@ -178,24 +178,24 @@ class EccCheck(ICiBuildPlugin):
         return comment_range
 
     def GenerateEccReport(self, modify_dir_list: List[str], ecc_diff_range: Dict[str, List[Tuple[int, int]]],
-                           edk2_path: str, basetools_path: str) -> None:
+                           workspace_path: str, basetools_path: str) -> None:
         ecc_need = False
         ecc_run = True
         config = os.path.join(basetools_path, "Source", "Python", "Ecc", "config.ini")
         exception = os.path.join(basetools_path, "Source", "Python", "Ecc", "exception.xml")
-        report = os.path.join(edk2_path, "Ecc.csv")
+        report = os.path.join(workspace_path, "Ecc.csv")
         for modify_dir in modify_dir_list:
-            target = os.path.join(edk2_path, modify_dir)
+            target = os.path.join(workspace_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)
+            return_code = RunCmd("Ecc", ecc_params, workingdir=workspace_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)
+            self.ParseEccReport(ecc_diff_range, workspace_path)
 
         if not ecc_need:
             logging.info("Doesn't need run ECC check")
@@ -204,10 +204,10 @@ class EccCheck(ICiBuildPlugin):
         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")
+    def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], workspace_path: str) -> None:
+        ecc_log = os.path.join(workspace_path, "Ecc.log")
         ecc_csv = "Ecc.csv"
-        file = os.listdir(edk2_path)
+        file = os.listdir(workspace_path)
         row_lines = []
         ignore_error_code = self.GetIgnoreErrorCode()
         if ecc_csv in file:
@@ -236,10 +236,10 @@ class EccCheck(ICiBuildPlugin):
             log.writelines(all_line)
         return
 
-    def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, basetools_path: str, pkg: str) -> None:
+    def ApplyConfig(self, pkgconfig: Dict[str, List[str]], workspace_path: str, basetools_path: str, pkg: str) -> None:
         if "IgnoreFiles" in pkgconfig:
             for a in pkgconfig["IgnoreFiles"]:
-                a = os.path.join(edk2_path, pkg, a)
+                a = os.path.join(workspace_path, pkg, a)
                 a = a.replace(os.sep, "/")
 
                 logging.info("Ignoring Files {0}".format(a))
-- 
2.17.1

[PATCH v1 3/4] .pytool/EccCheck: Check ecc_csv exists

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

'workspace_path' being an absolute path leads to 'ecc_csv' being
an absolute path. Then it won't be found among 'file' as they
are relative paths.

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 <gaoliming@byosoft.com.cn>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .pytool/Plugin/EccCheck/EccCheck.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index fff317f23110..87f0e65a140f 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -206,11 +206,10 @@ class EccCheck(ICiBuildPlugin):
 
     def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], workspace_path: str) -> None:
         ecc_log = os.path.join(workspace_path, "Ecc.log")
-        ecc_csv = "Ecc.csv"
-        file = os.listdir(workspace_path)
+        ecc_csv = os.path.join(workspace_path, "Ecc.csv")
         row_lines = []
         ignore_error_code = self.GetIgnoreErrorCode()
-        if ecc_csv in file:
+        if os.path.exists(ecc_csv):
             with open(ecc_csv) as csv_file:
                 reader = csv.reader(csv_file)
                 for row in reader:
-- 
2.17.1

[PATCH v1 4/4] .pytool/EccCheck: Set PACKAGES_PATH env var in Ecc

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

When running Ecc on other repositories (e.g.: edk2-platforms with
edk2 as a submodule), edk2 modules are referenced.
E.g.: MdePkg/..
The PACKAGES_PATH env var can be used to reference other directories
containing packages. Set it so that Ecc can find these packages.

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 <gaoliming@byosoft.com.cn>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .pytool/Plugin/EccCheck/EccCheck.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index 87f0e65a140f..49d143b32f91 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -67,6 +67,7 @@ class EccCheck(ICiBuildPlugin):
         env = shell_environment.GetEnvironment()
         env.set_shell_var('PYTHONPATH', python_path)
         env.set_shell_var('WORKSPACE', workspace_path)
+        env.set_shell_var('PACKAGES_PATH', ":".join(Edk2pathObj.PackagePathList))
         self.ECC_PASS = True
         self.ApplyConfig(pkgconfig, workspace_path, basetools_path, packagename)
         modify_dir_list = self.GetModifyDir(packagename)
-- 
2.17.1

回复: [PATCH v1 4/4] .pytool/EccCheck: Set PACKAGES_PATH env var in Ecc

[gaoliming]

Pierre:

> -----邮件原件-----
> 发件人: Pierre.Gondois@arm.com <Pierre.Gondois@arm.com>
> 发送时间: 2021年6月23日 15:23
> 收件人: devel@edk2.groups.io; Sean Brogan <sean.brogan@microsoft.com>;
> Bret Barkelew <Bret.Barkelew@microsoft.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Sami Mujawar <sami.mujawar@arm.com>
> 主题: [PATCH v1 4/4] .pytool/EccCheck: Set PACKAGES_PATH env var in Ecc
> 
> From: Pierre Gondois <Pierre.Gondois@arm.com>
> 
> When running Ecc on other repositories (e.g.: edk2-platforms with
> edk2 as a submodule), edk2 modules are referenced.
> E.g.: MdePkg/..
> The PACKAGES_PATH env var can be used to reference other directories
> containing packages. Set it so that Ecc can find these packages.
> 
> 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 <gaoliming@byosoft.com.cn>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>  .pytool/Plugin/EccCheck/EccCheck.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.pytool/Plugin/EccCheck/EccCheck.py
> b/.pytool/Plugin/EccCheck/EccCheck.py
> index 87f0e65a140f..49d143b32f91 100644
> --- a/.pytool/Plugin/EccCheck/EccCheck.py
> +++ b/.pytool/Plugin/EccCheck/EccCheck.py
> @@ -67,6 +67,7 @@ class EccCheck(ICiBuildPlugin):
>          env = shell_environment.GetEnvironment()
>          env.set_shell_var('PYTHONPATH', python_path)
>          env.set_shell_var('WORKSPACE', workspace_path)
> +        env.set_shell_var('PACKAGES_PATH',
> ":".join(Edk2pathObj.PackagePathList))
":" is Linux OS path separator. Windows OS path separator is ";". 

Thanks
Liming
>          self.ECC_PASS = True
>          self.ApplyConfig(pkgconfig, workspace_path, basetools_path,
> packagename)
>          modify_dir_list = self.GetModifyDir(packagename)
> --
> 2.17.1

Re: [edk2-devel] [PATCH v1 0/4] Allow EccCheck to run on other repositories

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

The modifications can be seen at: https://github.com/PierreARM/edk2/tree/1628_Ecc_environment_paths_update_v1
The result of the CI ca be seen at: https://github.com/tianocore/edk2/pull/1743
Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 222 bytes --]