public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.
@ 2020-09-23 10:57 Bob Feng
  2020-09-23 14:23 ` [edk2-devel] " Andrew Fish
  2020-09-24  6:29 ` Yuwei Chen
  0 siblings, 2 replies; 6+ messages in thread
From: Bob Feng @ 2020-09-23 10:57 UTC (permalink / raw)
  To: devel; +Cc: Mingyue Liang, Liming Gao, Yuwei Chen

From: Mingyue Liang <mingyuex.liang@intel.com>

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

Currently, When doing the Incremental build, the directory
macros extended to absolute path in output Makefile, which
is inconsistent with the output of Clean build.

When we do macro replacement, we can't replace macro due to
inconsistent path case, which results in inconsistent display
of incremental build and clean build in makefile.Therefore,
the path is converted to achieve the correct macro replacement.

Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
---
 BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0314d0ea34..b04d3f5436 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -786,8 +786,10 @@ cleanlib:
 
     def ReplaceMacro(self, str):
         for Macro in self.MacroList:
-            if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
-                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
+            if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
+                replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
+                    os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
+                str = str.replace(replace_dir, '$(' + Macro + ')')
         return str
 
     def CommandExceedLimit(self):
-- 
2.28.0.windows.1


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

* Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.
  2020-09-23 10:57 [PATCH] BaseTools: Normalize case of pathname when evaluating Macros Bob Feng
@ 2020-09-23 14:23 ` Andrew Fish
  2020-09-23 14:59   ` Bob Feng
  2020-09-24  6:29 ` Yuwei Chen
  1 sibling, 1 reply; 6+ messages in thread
From: Andrew Fish @ 2020-09-23 14:23 UTC (permalink / raw)
  To: devel, bob.c.feng; +Cc: Mingyue Liang, Liming Gao, Yuwei Chen

Does this work on case sensitive file systems?
> On Sep 23, 2020, at 3:58 AM, Bob Feng <bob.c.feng@intel.com> wrote:
> 
> From: Mingyue Liang <mingyuex.liang@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2880
> 
> Currently, When doing the Incremental build, the directory
> macros extended to absolute path in output Makefile, which
> is inconsistent with the output of Clean build.
> 
> When we do macro replacement, we can't replace macro due to
> inconsistent path case, which results in inconsistent display
> of incremental build and clean build in makefile.Therefore,
> the path is converted to achieve the correct macro replacement.
> 
> Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> ---
> BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
> index 0314d0ea34..b04d3f5436 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -786,8 +786,10 @@ cleanlib:
> 
>     def ReplaceMacro(self, str):
>         for Macro in self.MacroList:
> -            if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
> -                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
> +            if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
> +                replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
> +                    os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
> +                str = str.replace(replace_dir, '$(' + Macro + ')')
>         return str
> 
>     def CommandExceedLimit(self):
> -- 
> 2.28.0.windows.1
> 
> 
> 
> 
> 
> 

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

* Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.
  2020-09-23 14:23 ` [edk2-devel] " Andrew Fish
@ 2020-09-23 14:59   ` Bob Feng
  2020-09-23 15:31     ` Andrew Fish
  0 siblings, 1 reply; 6+ messages in thread
From: Bob Feng @ 2020-09-23 14:59 UTC (permalink / raw)
  To: Andrew Fish, devel@edk2.groups.io
  Cc: Liang, MingyueX, Liming Gao, Chen, Christine

Yes. we did test on Windows and Linux.

From the https://docs.python.org/3/library/os.path.html
os.path.normcase(path)
    Normalize the case of a pathname. On Windows, convert all characters in the pathname to lowercase, and also convert forward slashes to  backward slashes. On other operating systems, return the path unchanged.

Thanks,
Bob
-----Original Message-----
From: Andrew Fish <afish@apple.com> 
Sent: Wednesday, September 23, 2020 10:24 PM
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>
Cc: Liang, MingyueX <mingyuex.liang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Does this work on case sensitive file systems?
> On Sep 23, 2020, at 3:58 AM, Bob Feng <bob.c.feng@intel.com> wrote:
> 
> From: Mingyue Liang <mingyuex.liang@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2880
> 
> Currently, When doing the Incremental build, the directory macros 
> extended to absolute path in output Makefile, which is inconsistent 
> with the output of Clean build.
> 
> When we do macro replacement, we can't replace macro due to 
> inconsistent path case, which results in inconsistent display of 
> incremental build and clean build in makefile.Therefore, the path is 
> converted to achieve the correct macro replacement.
> 
> Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> ---
> BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
> b/BaseTools/Source/Python/AutoGen/GenMake.py
> index 0314d0ea34..b04d3f5436 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -786,8 +786,10 @@ cleanlib:
> 
>     def ReplaceMacro(self, str):
>         for Macro in self.MacroList:
> -            if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
> -                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
> +            if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
> +                replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
> +                    os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
> +                str = str.replace(replace_dir, '$(' + Macro + ')')
>         return str
> 
>     def CommandExceedLimit(self):
> --
> 2.28.0.windows.1
> 
> 
> 
> 
> 
> 

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

* Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.
  2020-09-23 14:59   ` Bob Feng
@ 2020-09-23 15:31     ` Andrew Fish
  2020-09-23 15:45       ` Bob Feng
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Fish @ 2020-09-23 15:31 UTC (permalink / raw)
  To: edk2-devel-groups-io, bob.c.feng
  Cc: Liang, MingyueX, Liming Gao, Chen, Christine

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

Bob,

Sorry I was confused by the commit comment `inconsistent path case`. So this was only a bug on case insensitive file systems? 

I’m paranoid as macOS support case and case insensitive file systems and we have had a lot of strange bugs in the past. 

Thanks,

Andrew Fish

> On Sep 23, 2020, at 7:59 AM, Bob Feng <bob.c.feng@intel.com> wrote:
> 
> Yes. we did test on Windows and Linux.
> 
> From the https://docs.python.org/3/library/os.path.html <https://docs.python.org/3/library/os.path.html>
> os.path.normcase(path)
>    Normalize the case of a pathname. On Windows, convert all characters in the pathname to lowercase, and also convert forward slashes to  backward slashes. On other operating systems, return the path unchanged.
> 
> Thanks,
> Bob
> -----Original Message-----
> From: Andrew Fish <afish@apple.com <mailto:afish@apple.com>> 
> Sent: Wednesday, September 23, 2020 10:24 PM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Feng, Bob C <bob.c.feng@intel.com <mailto:bob.c.feng@intel.com>>
> Cc: Liang, MingyueX <mingyuex.liang@intel.com <mailto:mingyuex.liang@intel.com>>; Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>; Chen, Christine <yuwei.chen@intel.com <mailto:yuwei.chen@intel.com>>
> Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.
> 
> Does this work on case sensitive file systems?
>> On Sep 23, 2020, at 3:58 AM, Bob Feng <bob.c.feng@intel.com <mailto:bob.c.feng@intel.com>> wrote:
>> 
>> From: Mingyue Liang <mingyuex.liang@intel.com <mailto:mingyuex.liang@intel.com>>
>> 
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2880 <https://bugzilla.tianocore.org/show_bug.cgi?id=2880>
>> 
>> Currently, When doing the Incremental build, the directory macros 
>> extended to absolute path in output Makefile, which is inconsistent 
>> with the output of Clean build.
>> 
>> When we do macro replacement, we can't replace macro due to 
>> inconsistent path case, which results in inconsistent display of 
>> incremental build and clean build in makefile.Therefore, the path is 
>> converted to achieve the correct macro replacement.
>> 
>> Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com <mailto:mingyuex.liang@intel.com>>
>> Cc: Bob Feng <bob.c.feng@intel.com <mailto:bob.c.feng@intel.com>>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>
>> Cc: Yuwei Chen <yuwei.chen@intel.com <mailto:yuwei.chen@intel.com>>
>> ---
>> BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
>> b/BaseTools/Source/Python/AutoGen/GenMake.py
>> index 0314d0ea34..b04d3f5436 100755
>> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
>> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
>> @@ -786,8 +786,10 @@ cleanlib:
>> 
>>    def ReplaceMacro(self, str):
>>        for Macro in self.MacroList:
>> -            if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
>> -                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
>> +            if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
>> +                replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
>> +                    os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
>> +                str = str.replace(replace_dir, '$(' + Macro + ')')
>>        return str
>> 
>>    def CommandExceedLimit(self):
>> --
>> 2.28.0.windows.1
>> 
>> 
>> 
>> 
>> 
>> 
> 
> 
> 


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

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

* Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.
  2020-09-23 15:31     ` Andrew Fish
@ 2020-09-23 15:45       ` Bob Feng
  0 siblings, 0 replies; 6+ messages in thread
From: Bob Feng @ 2020-09-23 15:45 UTC (permalink / raw)
  To: Andrew Fish, edk2-devel-groups-io
  Cc: Liang, MingyueX, Liming Gao, Chen, Christine

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

Yes. this bug only happens on case insensitive file systems.

Thanks,
Bob

From: Andrew Fish <afish@apple.com>
Sent: Wednesday, September 23, 2020 11:31 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Feng, Bob C <bob.c.feng@intel.com>
Cc: Liang, MingyueX <mingyuex.liang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Bob,

Sorry I was confused by the commit comment `inconsistent path case`. So this was only a bug on case insensitive file systems?

I’m paranoid as macOS support case and case insensitive file systems and we have had a lot of strange bugs in the past.

Thanks,

Andrew Fish


On Sep 23, 2020, at 7:59 AM, Bob Feng <bob.c.feng@intel.com<mailto:bob.c.feng@intel.com>> wrote:

Yes. we did test on Windows and Linux.

From the https://docs.python.org/3/library/os.path.html
os.path.normcase(path)
   Normalize the case of a pathname. On Windows, convert all characters in the pathname to lowercase, and also convert forward slashes to  backward slashes. On other operating systems, return the path unchanged.

Thanks,
Bob
-----Original Message-----
From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Wednesday, September 23, 2020 10:24 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Feng, Bob C <bob.c.feng@intel.com<mailto:bob.c.feng@intel.com>>
Cc: Liang, MingyueX <mingyuex.liang@intel.com<mailto:mingyuex.liang@intel.com>>; Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Chen, Christine <yuwei.chen@intel.com<mailto:yuwei.chen@intel.com>>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Does this work on case sensitive file systems?

On Sep 23, 2020, at 3:58 AM, Bob Feng <bob.c.feng@intel.com<mailto:bob.c.feng@intel.com>> wrote:

From: Mingyue Liang <mingyuex.liang@intel.com<mailto:mingyuex.liang@intel.com>>

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

Currently, When doing the Incremental build, the directory macros
extended to absolute path in output Makefile, which is inconsistent
with the output of Clean build.

When we do macro replacement, we can't replace macro due to
inconsistent path case, which results in inconsistent display of
incremental build and clean build in makefile.Therefore, the path is
converted to achieve the correct macro replacement.

Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com<mailto:mingyuex.liang@intel.com>>
Cc: Bob Feng <bob.c.feng@intel.com<mailto:bob.c.feng@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Yuwei Chen <yuwei.chen@intel.com<mailto:yuwei.chen@intel.com>>
---
BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0314d0ea34..b04d3f5436 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -786,8 +786,10 @@ cleanlib:

   def ReplaceMacro(self, str):
       for Macro in self.MacroList:
-            if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
-                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
+            if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
+                replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
+                    os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
+                str = str.replace(replace_dir, '$(' + Macro + ')')
       return str

   def CommandExceedLimit(self):
--
2.28.0.windows.1










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

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

* Re: [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.
  2020-09-23 10:57 [PATCH] BaseTools: Normalize case of pathname when evaluating Macros Bob Feng
  2020-09-23 14:23 ` [edk2-devel] " Andrew Fish
@ 2020-09-24  6:29 ` Yuwei Chen
  1 sibling, 0 replies; 6+ messages in thread
From: Yuwei Chen @ 2020-09-24  6:29 UTC (permalink / raw)
  To: Feng, Bob C, devel@edk2.groups.io; +Cc: Liang, MingyueX, Liming Gao

Reviewed-by: Yuwei Chen<yuwei.chen@intel.com>

> -----Original Message-----
> From: Feng, Bob C <bob.c.feng@intel.com>
> Sent: Wednesday, September 23, 2020 6:58 PM
> To: devel@edk2.groups.io
> Cc: Liang, MingyueX <mingyuex.liang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
> Subject: [PATCH] BaseTools: Normalize case of pathname when evaluating
> Macros.
> 
> From: Mingyue Liang <mingyuex.liang@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2880
> 
> Currently, When doing the Incremental build, the directory macros extended
> to absolute path in output Makefile, which is inconsistent with the output of
> Clean build.
> 
> When we do macro replacement, we can't replace macro due to inconsistent
> path case, which results in inconsistent display of incremental build and clean
> build in makefile.Therefore, the path is converted to achieve the correct
> macro replacement.
> 
> Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> ---
>  BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
> b/BaseTools/Source/Python/AutoGen/GenMake.py
> index 0314d0ea34..b04d3f5436 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -786,8 +786,10 @@ cleanlib:
> 
>      def ReplaceMacro(self, str):
>          for Macro in self.MacroList:
> -            if self._AutoGenObject.Macros[Macro] and
> self._AutoGenObject.Macros[Macro] in str:
> -                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro
> + ')')
> +            if self._AutoGenObject.Macros[Macro] and
> os.path.normcase(self._AutoGenObject.Macros[Macro]) in
> os.path.normcase(str):
> +                replace_dir =
> str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Ma
> cros[Macro])): os.path.normcase(str).index(
> +                    os.path.normcase(self._AutoGenObject.Macros[Macro])) +
> len(self._AutoGenObject.Macros[Macro])]
> +                str = str.replace(replace_dir, '$(' + Macro + ')')
>          return str
> 
>      def CommandExceedLimit(self):
> --
> 2.28.0.windows.1


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

end of thread, other threads:[~2020-09-24  6:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-23 10:57 [PATCH] BaseTools: Normalize case of pathname when evaluating Macros Bob Feng
2020-09-23 14:23 ` [edk2-devel] " Andrew Fish
2020-09-23 14:59   ` Bob Feng
2020-09-23 15:31     ` Andrew Fish
2020-09-23 15:45       ` Bob Feng
2020-09-24  6:29 ` Yuwei Chen

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