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

[PATCH v2 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.

The changes can be seen at: https://github.com/PierreARM/edk2/commits/1628_Ecc_environment_paths_update_v2

v2:
 - Use ';' path separator for Windows host [Liming]  

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

Notes:
    v2:
     - Use ';' path separator for Windows host [Liming]

 .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..2d0612269b2e 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', os.pathsep.join(Edk2pathObj.PackagePathList))
         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 v2 4/4] .pytool/EccCheck: Set PACKAGES_PATH env var in Ecc

[Bob Feng]

Reviewed-by: Bob Feng <bob.c.feng@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of PierreGondois
Sent: Wednesday, July 7, 2021 4:56 AM
To: devel@edk2.groups.io; Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Sami Mujawar <sami.mujawar@arm.com>
Subject: [edk2-devel] [PATCH v2 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>
---

Notes:
    v2:
     - Use ';' path separator for Windows host [Liming]

 .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..2d0612269b2e 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', 
+ os.pathsep.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 v2 0/4] Allow EccCheck to run on other repositories

[gaoliming]

This version is good to me. Reviewed-by: Liming Gao
<gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: Pierre.Gondois@arm.com <Pierre.Gondois@arm.com>
> 发送时间: 2021年7月7日 4:56
> 收件人: 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 v2 0/4] Allow EccCheck to run on other repositories
> 
> 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.
> 
> The changes can be seen at:
> https://github.com/PierreARM/edk2/commits/1628_Ecc_environment_paths
> _update_v2
> 
> v2:
>  - Use ';' path separator for Windows host [Liming]
> 
> 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

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

[PierreGondois]

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

Hi Sean and Bret,
Do you have any comments on the patchset ?

Regards,
Pierre

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

回复: [edk2-devel] [PATCH v2 0/4] Allow EccCheck to run on other repositories

[gaoliming]

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

Pierre:

 I have merged this patch set at 40a9066439cbab235933525810f46f03806c6ef1..fb5b6220a9718fc28ae67f317d3611214a05589c

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 PierreGondois
发送时间: 2021年7月21日 16:20
收件人: PierreGondois <pierre.gondois@arm.com>; devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH v2 0/4] Allow EccCheck to run on other repositories

 

Hi Sean and Bret,
Do you have any comments on the patchset ?

Regards,
Pierre




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