[PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

[PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

[Kubacki, Michael A]

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2232

Commit 0075ab2cec introduced an issue that causes an exception
when multiple workspace packages paths are specified. For example,
if edk2-platforms is used, the root directory will contain an edk
and edk2-platforms directory representing the respective
repositories.

In GenMake, the path to the package DEC file for a module is
discovered by getting the relative path of the INF to the
workspace root directory. Each directory in the relative path
is incrementally joined to the WORKSPACE directory. The file
list in the joined path is searched for a DEC file.

As an example, if the build command is used on a package outside
the edk2 repository, the INF file path is relative to the
edk2-platforms directory not edk2. This causes directory paths
to be built that do not exist. Commit 0075ab2cec replaced the
os.path.exists() call with a try except block that always fails
when os.listdir() is invoked to enumerate the list of files in
the built directory path on packages outside edk2.

This commit restores the original conditional statement which
avoids calling os.listdir() with an invalid directory path.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
 BaseTools/Source/Python/AutoGen/GenMake.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
index 584156dab9..97ba158ff2 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -637,13 +637,11 @@ cleanlib:
         while not found and os.sep in package_rel_dir:
             index = package_rel_dir.index(os.sep)
             current_dir = mws.join(current_dir, package_rel_dir[:index])
-            try:
+            if os.path.exists(current_dir):
                 for fl in os.listdir(current_dir):
                     if fl.endswith('.dec'):
                         found = True
                         break
-            except:
-                EdkLogger.error('build', FILE_NOT_FOUND, "WORKSPACE does not exist.")
             package_rel_dir = package_rel_dir[index + 1:]
 
         MakefileTemplateDict = {
-- 
2.16.2.windows.1

Re: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

[Michael D Kinney]

Hi Michael,

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

This look really important.  Do you want me to push today?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Kubacki, Michael A
> Sent: Tuesday, October 1, 2019 3:58 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix
> GenMake multi-workspace failure
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2232
> 
> Commit 0075ab2cec introduced an issue that causes an
> exception when multiple workspace packages paths are
> specified. For example, if edk2-platforms is used, the
> root directory will contain an edk and edk2-platforms
> directory representing the respective repositories.
> 
> In GenMake, the path to the package DEC file for a
> module is discovered by getting the relative path of the
> INF to the workspace root directory. Each directory in
> the relative path is incrementally joined to the
> WORKSPACE directory. The file list in the joined path is
> searched for a DEC file.
> 
> As an example, if the build command is used on a package
> outside the edk2 repository, the INF file path is
> relative to the edk2-platforms directory not edk2. This
> causes directory paths to be built that do not exist.
> Commit 0075ab2cec replaced the
> os.path.exists() call with a try except block that
> always fails when os.listdir() is invoked to enumerate
> the list of files in the built directory path on
> packages outside edk2.
> 
> This commit restores the original conditional statement
> which avoids calling os.listdir() with an invalid
> directory path.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Michael Kubacki
> <michael.a.kubacki@intel.com>
> ---
>  BaseTools/Source/Python/AutoGen/GenMake.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
> b/BaseTools/Source/Python/AutoGen/GenMake.py
> index 584156dab9..97ba158ff2 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -637,13 +637,11 @@ cleanlib:
>          while not found and os.sep in package_rel_dir:
>              index = package_rel_dir.index(os.sep)
>              current_dir = mws.join(current_dir,
> package_rel_dir[:index])
> -            try:
> +            if os.path.exists(current_dir):
>                  for fl in os.listdir(current_dir):
>                      if fl.endswith('.dec'):
>                          found = True
>                          break
> -            except:
> -                EdkLogger.error('build',
> FILE_NOT_FOUND, "WORKSPACE does not exist.")
>              package_rel_dir = package_rel_dir[index +
> 1:]
> 
>          MakefileTemplateDict = {
> --
> 2.16.2.windows.1
> 
> 
> 

Re: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

[Bob Feng]

Thanks for the fix.

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


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
Sent: Friday, October 4, 2019 5:14 AM
To: devel@edk2.groups.io; Kubacki, Michael A <michael.a.kubacki@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

Hi Michael,

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

This look really important.  Do you want me to push today?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> Kubacki, Michael A
> Sent: Tuesday, October 1, 2019 3:58 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming 
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake 
> multi-workspace failure
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2232
> 
> Commit 0075ab2cec introduced an issue that causes an exception when 
> multiple workspace packages paths are specified. For example, if 
> edk2-platforms is used, the root directory will contain an edk and 
> edk2-platforms directory representing the respective repositories.
> 
> In GenMake, the path to the package DEC file for a module is 
> discovered by getting the relative path of the INF to the workspace 
> root directory. Each directory in the relative path is incrementally 
> joined to the WORKSPACE directory. The file list in the joined path is 
> searched for a DEC file.
> 
> As an example, if the build command is used on a package outside the 
> edk2 repository, the INF file path is relative to the edk2-platforms 
> directory not edk2. This causes directory paths to be built that do 
> not exist.
> Commit 0075ab2cec replaced the
> os.path.exists() call with a try except block that always fails when 
> os.listdir() is invoked to enumerate the list of files in the built 
> directory path on packages outside edk2.
> 
> This commit restores the original conditional statement which avoids 
> calling os.listdir() with an invalid directory path.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Michael Kubacki
> <michael.a.kubacki@intel.com>
> ---
>  BaseTools/Source/Python/AutoGen/GenMake.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
> b/BaseTools/Source/Python/AutoGen/GenMake.py
> index 584156dab9..97ba158ff2 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -637,13 +637,11 @@ cleanlib:
>          while not found and os.sep in package_rel_dir:
>              index = package_rel_dir.index(os.sep)
>              current_dir = mws.join(current_dir,
> package_rel_dir[:index])
> -            try:
> +            if os.path.exists(current_dir):
>                  for fl in os.listdir(current_dir):
>                      if fl.endswith('.dec'):
>                          found = True
>                          break
> -            except:
> -                EdkLogger.error('build',
> FILE_NOT_FOUND, "WORKSPACE does not exist.")
>              package_rel_dir = package_rel_dir[index + 1:]
> 
>          MakefileTemplateDict = {
> --
> 2.16.2.windows.1
> 
> 
> 

Re: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

[Bob Feng]

Pushed at SHA-1: 61af5f249495b18f45ca164376c871081448c0e4

Thanks,
Bob

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bob Feng
Sent: Friday, October 4, 2019 11:04 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Kubacki, Michael A <michael.a.kubacki@intel.com>
Cc: Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

Thanks for the fix.

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


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
Sent: Friday, October 4, 2019 5:14 AM
To: devel@edk2.groups.io; Kubacki, Michael A <michael.a.kubacki@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake multi-workspace failure

Hi Michael,

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

This look really important.  Do you want me to push today?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> Kubacki, Michael A
> Sent: Tuesday, October 1, 2019 3:58 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming 
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH V1 1/1] BaseTools: Fix GenMake 
> multi-workspace failure
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2232
> 
> Commit 0075ab2cec introduced an issue that causes an exception when 
> multiple workspace packages paths are specified. For example, if 
> edk2-platforms is used, the root directory will contain an edk and 
> edk2-platforms directory representing the respective repositories.
> 
> In GenMake, the path to the package DEC file for a module is 
> discovered by getting the relative path of the INF to the workspace 
> root directory. Each directory in the relative path is incrementally 
> joined to the WORKSPACE directory. The file list in the joined path is 
> searched for a DEC file.
> 
> As an example, if the build command is used on a package outside the
> edk2 repository, the INF file path is relative to the edk2-platforms 
> directory not edk2. This causes directory paths to be built that do 
> not exist.
> Commit 0075ab2cec replaced the
> os.path.exists() call with a try except block that always fails when
> os.listdir() is invoked to enumerate the list of files in the built 
> directory path on packages outside edk2.
> 
> This commit restores the original conditional statement which avoids 
> calling os.listdir() with an invalid directory path.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Michael Kubacki
> <michael.a.kubacki@intel.com>
> ---
>  BaseTools/Source/Python/AutoGen/GenMake.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
> b/BaseTools/Source/Python/AutoGen/GenMake.py
> index 584156dab9..97ba158ff2 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -637,13 +637,11 @@ cleanlib:
>          while not found and os.sep in package_rel_dir:
>              index = package_rel_dir.index(os.sep)
>              current_dir = mws.join(current_dir,
> package_rel_dir[:index])
> -            try:
> +            if os.path.exists(current_dir):
>                  for fl in os.listdir(current_dir):
>                      if fl.endswith('.dec'):
>                          found = True
>                          break
> -            except:
> -                EdkLogger.error('build',
> FILE_NOT_FOUND, "WORKSPACE does not exist.")
>              package_rel_dir = package_rel_dir[index + 1:]
> 
>          MakefileTemplateDict = {
> --
> 2.16.2.windows.1
> 
> 
>