[PATCH] BaseTools: Skip .mcb file module in Binary Cache

[PATCH] BaseTools: Skip .mcb file module in Binary Cache

[Steven Shi]

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

Current Kabylake open platform will build fail if enabled
to consume the binary cache, because the binary cache doesn't
support to recovery the .mcb microcode file,
e.g. m80406E8_00000026.mcb, in a platform level folder which
is outside of the module output folder. In normal build
without cache, the .mcb file is copied through OS copy/move
commands defined in build rules which are not supported by
Binary Cache.
Change the Binary Cache to skip the .mcb file type module and
always rebuild the module to apply the full build rules if
it contains .mcb file.
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..6b596c8a65 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
         # If library or Module is binary do not skip by hash
         if self.IsBinaryModule:
             return False
-        # .inc is contains binary information so do not skip by hash as well
+        # .inc and .mcb is contains binary information so do not skip by hash as well
         for f_ext in self.SourceFileList:
-            if '.inc' in str(f_ext):
+            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
                 return False
         FileDir = path.join(GlobalData.gBinCacheSource, self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain, self.Arch, self.SourceDir, self.MetaFile.BaseName)
         HashFile = path.join(FileDir, self.Name + '.hash')
@@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
         # If library or Module is binary do not skip by hash
         if self.IsBinaryModule:
             return False
-        # .inc is contains binary information so do not skip by hash as well
+        # .inc or '.mcb' is contains binary information so do not skip by hash as well
         for f_ext in self.SourceFileList:
-            if '.inc' in str(f_ext):
+            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
                 return False
         if GlobalData.gUseHashCache:
             # If there is a valid hash or function generated a valid hash; function will return False
-- 
2.17.1.windows.2

Re: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache

[Yao, Jiewen]

Hi
Can we have better way to describe the binary cache?

I have 2 concern:
1) we have other type of binary besides MCB. How can we indicate that without update base tool source code ?
2) .inc might be text file, instead of binary. How can we support that?

Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Steven Shi
> Sent: Wednesday, May 8, 2019 10:00 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Rodriguez, Christian
> <christian.rodriguez@intel.com>; Johnson, Michael
> <michael.johnson@intel.com>
> Subject: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> Cache
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1723
> 
> Current Kabylake open platform will build fail if enabled
> to consume the binary cache, because the binary cache doesn't
> support to recovery the .mcb microcode file,
> e.g. m80406E8_00000026.mcb, in a platform level folder which
> is outside of the module output folder. In normal build
> without cache, the .mcb file is copied through OS copy/move
> commands defined in build rules which are not supported by
> Binary Cache.
> Change the Binary Cache to skip the .mcb file type module and
> always rebuild the module to apply the full build rules if
> it contains .mcb file.
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index 31721a6f9f..6b596c8a65 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
>          # If library or Module is binary do not skip by hash
>          if self.IsBinaryModule:
>              return False
> -        # .inc is contains binary information so do not skip by hash as well
> +        # .inc and .mcb is contains binary information so do not skip by
> hash as well
>          for f_ext in self.SourceFileList:
> -            if '.inc' in str(f_ext):
> +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
>                  return False
>          FileDir = path.join(GlobalData.gBinCacheSource,
> self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain, self.Arch,
> self.SourceDir, self.MetaFile.BaseName)
>          HashFile = path.join(FileDir, self.Name + '.hash')
> @@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
>          # If library or Module is binary do not skip by hash
>          if self.IsBinaryModule:
>              return False
> -        # .inc is contains binary information so do not skip by hash as well
> +        # .inc or '.mcb' is contains binary information so do not skip by
> hash as well
>          for f_ext in self.SourceFileList:
> -            if '.inc' in str(f_ext):
> +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
>                  return False
>          if GlobalData.gUseHashCache:
>              # If there is a valid hash or function generated a valid hash;
> function will return False
> --
> 2.17.1.windows.2
> 
> 
> 

Re: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache

[Liming Gao]

Jiewen:

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, May 9, 2019 1:13 PM
> To: devel@edk2.groups.io; Shi, Steven <steven.shi@intel.com>
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Rodriguez, Christian <christian.rodriguez@intel.com>;
> Johnson, Michael <michael.johnson@intel.com>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache
> 
> Hi
> Can we have better way to describe the binary cache?
Yes. Steven provides some wiki pages for it. 
> 
> I have 2 concern:
> 1) we have other type of binary besides MCB. How can we indicate that without update base tool source code ?
MCB will be combined to single bin file. The problem is that the platform uses the bin file from output IA32 or X64 
directory instead of the module output directory. Binary cache is for the module output directory. 
I suggest to update the platform to consume the module output file. But, I meet with one tool issue to support 
RAW FILE type, because Microcode uses RAW file type. BZ https://bugzilla.tianocore.org/show_bug.cgi?id=1765 is submitted. 
I suggest to fix this tool issue first. 

> 2) .inc might be text file, instead of binary. How can we support that?
.inc is similar to .c source file. Its output is .mcb, then be combined to .bin file. .bin file will be cached. 
.inc is not required to be cached. On next build, build tool will compare the hash value of source file, if 
hash value is not changed. Previous cached .bin file will be used. 

> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Steven Shi
> > Sent: Wednesday, May 8, 2019 10:00 PM
> > To: devel@edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Rodriguez, Christian
> > <christian.rodriguez@intel.com>; Johnson, Michael
> > <michael.johnson@intel.com>
> > Subject: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> > Cache
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1723
> >
> > Current Kabylake open platform will build fail if enabled
> > to consume the binary cache, because the binary cache doesn't
> > support to recovery the .mcb microcode file,
> > e.g. m80406E8_00000026.mcb, in a platform level folder which
> > is outside of the module output folder. In normal build
> > without cache, the .mcb file is copied through OS copy/move
> > commands defined in build rules which are not supported by
> > Binary Cache.
> > Change the Binary Cache to skip the .mcb file type module and
> > always rebuild the module to apply the full build rules if
> > it contains .mcb file.
> > ---
> >  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > index 31721a6f9f..6b596c8a65 100644
> > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > @@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
> >          # If library or Module is binary do not skip by hash
> >          if self.IsBinaryModule:
> >              return False
> > -        # .inc is contains binary information so do not skip by hash as well
> > +        # .inc and .mcb is contains binary information so do not skip by
> > hash as well
> >          for f_ext in self.SourceFileList:
> > -            if '.inc' in str(f_ext):
> > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> >                  return False
> >          FileDir = path.join(GlobalData.gBinCacheSource,
> > self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain, self.Arch,
> > self.SourceDir, self.MetaFile.BaseName)
> >          HashFile = path.join(FileDir, self.Name + '.hash')
> > @@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
> >          # If library or Module is binary do not skip by hash
> >          if self.IsBinaryModule:
> >              return False
> > -        # .inc is contains binary information so do not skip by hash as well
> > +        # .inc or '.mcb' is contains binary information so do not skip by
> > hash as well
> >          for f_ext in self.SourceFileList:
> > -            if '.inc' in str(f_ext):
> > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> >                  return False
> >          if GlobalData.gUseHashCache:
> >              # If there is a valid hash or function generated a valid hash;
> > function will return False
> > --
> > 2.17.1.windows.2
> >
> >
> > 

Re: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache

[Steven Shi]

> > Can we have better way to describe the binary cache?
> Yes. Steven provides some wiki pages for it.
https://raw.githubusercontent.com/shijunjing/WikiDoc/master/BuildCache/MultipleBuildCache.jpg 

I agree it sounds a bit confusing.  Binary Cache is the legacy name for the current model level build cache implementation. The current implementation try only save and recovery the module binary files (e.g. only cache .efi but no .obj, no .lib) which are necessary for the FV generation. But if we add .lib cache and .fv cache support in the future, we could rename it as Build Cache directly.


Thanks
Steven Shi


> -----Original Message-----
> From: Gao, Liming
> Sent: Thursday, May 9, 2019 1:47 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Shi, Steven
> <steven.shi@intel.com>
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> <christian.rodriguez@intel.com>; Johnson, Michael
> <michael.johnson@intel.com>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> Cache
> 
> Jiewen:
> 
> > -----Original Message-----
> > From: Yao, Jiewen
> > Sent: Thursday, May 9, 2019 1:13 PM
> > To: devel@edk2.groups.io; Shi, Steven <steven.shi@intel.com>
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Rodriguez, Christian
> <christian.rodriguez@intel.com>;
> > Johnson, Michael <michael.johnson@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> Cache
> >
> > Hi
> > Can we have better way to describe the binary cache?
> Yes. Steven provides some wiki pages for it.
> >
> > I have 2 concern:
> > 1) we have other type of binary besides MCB. How can we indicate that
> without update base tool source code ?
> MCB will be combined to single bin file. The problem is that the platform uses
> the bin file from output IA32 or X64
> directory instead of the module output directory. Binary cache is for the
> module output directory.
> I suggest to update the platform to consume the module output file. But, I
> meet with one tool issue to support
> RAW FILE type, because Microcode uses RAW file type. BZ
> https://bugzilla.tianocore.org/show_bug.cgi?id=1765 is submitted.
> I suggest to fix this tool issue first.
> 
> > 2) .inc might be text file, instead of binary. How can we support that?
> .inc is similar to .c source file. Its output is .mcb, then be combined to .bin
> file. .bin file will be cached.
> .inc is not required to be cached. On next build, build tool will compare the
> hash value of source file, if
> hash value is not changed. Previous cached .bin file will be used.
> 
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > > Steven Shi
> > > Sent: Wednesday, May 8, 2019 10:00 PM
> > > To: devel@edk2.groups.io
> > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Rodriguez, Christian
> > > <christian.rodriguez@intel.com>; Johnson, Michael
> > > <michael.johnson@intel.com>
> > > Subject: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> > > Cache
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1723
> > >
> > > Current Kabylake open platform will build fail if enabled
> > > to consume the binary cache, because the binary cache doesn't
> > > support to recovery the .mcb microcode file,
> > > e.g. m80406E8_00000026.mcb, in a platform level folder which
> > > is outside of the module output folder. In normal build
> > > without cache, the .mcb file is copied through OS copy/move
> > > commands defined in build rules which are not supported by
> > > Binary Cache.
> > > Change the Binary Cache to skip the .mcb file type module and
> > > always rebuild the module to apply the full build rules if
> > > it contains .mcb file.
> > > ---
> > >  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > index 31721a6f9f..6b596c8a65 100644
> > > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > @@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
> > >          # If library or Module is binary do not skip by hash
> > >          if self.IsBinaryModule:
> > >              return False
> > > -        # .inc is contains binary information so do not skip by hash as well
> > > +        # .inc and .mcb is contains binary information so do not skip by
> > > hash as well
> > >          for f_ext in self.SourceFileList:
> > > -            if '.inc' in str(f_ext):
> > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > >                  return False
> > >          FileDir = path.join(GlobalData.gBinCacheSource,
> > > self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain, self.Arch,
> > > self.SourceDir, self.MetaFile.BaseName)
> > >          HashFile = path.join(FileDir, self.Name + '.hash')
> > > @@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
> > >          # If library or Module is binary do not skip by hash
> > >          if self.IsBinaryModule:
> > >              return False
> > > -        # .inc is contains binary information so do not skip by hash as well
> > > +        # .inc or '.mcb' is contains binary information so do not skip by
> > > hash as well
> > >          for f_ext in self.SourceFileList:
> > > -            if '.inc' in str(f_ext):
> > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > >                  return False
> > >          if GlobalData.gUseHashCache:
> > >              # If there is a valid hash or function generated a valid hash;
> > > function will return False
> > > --
> > > 2.17.1.windows.2
> > >
> > >
> > > 

Re: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache

[Yao, Jiewen]

Thanks to explain that.
I think my general concern is: why we hardcode file suffix "INC" or "MCB" in the tool source code.

> > > > -            if '.inc' in str(f_ext):
> > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > >                  return False

What happen if we need skip another type of file later? update tool source code again ?

My thought is that: the tool should provide capability to skip *some* type of file.
What "some" type of file to be skipped should be configured outside of tool source code.
As such, when we need skip more type of file, we just update the configuration, instead of tool source code.

Just like we have tools_def and build_rule today, when we need update those configuration, we don't need update tool source code.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Shi, Steven
> Sent: Wednesday, May 8, 2019 10:54 PM
> To: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> <christian.rodriguez@intel.com>; Johnson, Michael
> <michael.johnson@intel.com>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> Cache
> 
> > > Can we have better way to describe the binary cache?
> > Yes. Steven provides some wiki pages for it.
> https://raw.githubusercontent.com/shijunjing/WikiDoc/master/BuildCache/
> MultipleBuildCache.jpg
> 
> I agree it sounds a bit confusing.  Binary Cache is the legacy name for the
> current model level build cache implementation. The current implementation
> try only save and recovery the module binary files (e.g. only cache .efi but
> no .obj, no .lib) which are necessary for the FV generation. But if we add .lib
> cache and .fv cache support in the future, we could rename it as Build Cache
> directly.
> 
> 
> Thanks
> Steven Shi
> 
> 
> > -----Original Message-----
> > From: Gao, Liming
> > Sent: Thursday, May 9, 2019 1:47 PM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Shi,
> Steven
> > <steven.shi@intel.com>
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> > <christian.rodriguez@intel.com>; Johnson, Michael
> > <michael.johnson@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> Binary
> > Cache
> >
> > Jiewen:
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen
> > > Sent: Thursday, May 9, 2019 1:13 PM
> > > To: devel@edk2.groups.io; Shi, Steven <steven.shi@intel.com>
> > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Rodriguez, Christian
> > <christian.rodriguez@intel.com>;
> > > Johnson, Michael <michael.johnson@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> Binary
> > Cache
> > >
> > > Hi
> > > Can we have better way to describe the binary cache?
> > Yes. Steven provides some wiki pages for it.
> > >
> > > I have 2 concern:
> > > 1) we have other type of binary besides MCB. How can we indicate that
> > without update base tool source code ?
> > MCB will be combined to single bin file. The problem is that the platform
> uses
> > the bin file from output IA32 or X64
> > directory instead of the module output directory. Binary cache is for the
> > module output directory.
> > I suggest to update the platform to consume the module output file. But, I
> > meet with one tool issue to support
> > RAW FILE type, because Microcode uses RAW file type. BZ
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1765 is submitted.
> > I suggest to fix this tool issue first.
> >
> > > 2) .inc might be text file, instead of binary. How can we support that?
> > .inc is similar to .c source file. Its output is .mcb, then be combined to .bin
> > file. .bin file will be cached.
> > .inc is not required to be cached. On next build, build tool will compare the
> > hash value of source file, if
> > hash value is not changed. Previous cached .bin file will be used.
> >
> > >
> > > Thank you
> > > Yao Jiewen
> > >
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> Of
> > > > Steven Shi
> > > > Sent: Wednesday, May 8, 2019 10:00 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > > > <liming.gao@intel.com>; Rodriguez, Christian
> > > > <christian.rodriguez@intel.com>; Johnson, Michael
> > > > <michael.johnson@intel.com>
> > > > Subject: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> Binary
> > > > Cache
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1723
> > > >
> > > > Current Kabylake open platform will build fail if enabled
> > > > to consume the binary cache, because the binary cache doesn't
> > > > support to recovery the .mcb microcode file,
> > > > e.g. m80406E8_00000026.mcb, in a platform level folder which
> > > > is outside of the module output folder. In normal build
> > > > without cache, the .mcb file is copied through OS copy/move
> > > > commands defined in build rules which are not supported by
> > > > Binary Cache.
> > > > Change the Binary Cache to skip the .mcb file type module and
> > > > always rebuild the module to apply the full build rules if
> > > > it contains .mcb file.
> > > > ---
> > > >  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > index 31721a6f9f..6b596c8a65 100644
> > > > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > @@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
> > > >          # If library or Module is binary do not skip by hash
> > > >          if self.IsBinaryModule:
> > > >              return False
> > > > -        # .inc is contains binary information so do not skip by hash as
> well
> > > > +        # .inc and .mcb is contains binary information so do not skip
> by
> > > > hash as well
> > > >          for f_ext in self.SourceFileList:
> > > > -            if '.inc' in str(f_ext):
> > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > >                  return False
> > > >          FileDir = path.join(GlobalData.gBinCacheSource,
> > > > self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain,
> self.Arch,
> > > > self.SourceDir, self.MetaFile.BaseName)
> > > >          HashFile = path.join(FileDir, self.Name + '.hash')
> > > > @@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
> > > >          # If library or Module is binary do not skip by hash
> > > >          if self.IsBinaryModule:
> > > >              return False
> > > > -        # .inc is contains binary information so do not skip by hash as
> well
> > > > +        # .inc or '.mcb' is contains binary information so do not skip
> by
> > > > hash as well
> > > >          for f_ext in self.SourceFileList:
> > > > -            if '.inc' in str(f_ext):
> > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > >                  return False
> > > >          if GlobalData.gUseHashCache:
> > > >              # If there is a valid hash or function generated a valid
> hash;
> > > > function will return False
> > > > --
> > > > 2.17.1.windows.2
> > > >
> > > >
> > > > 

Re: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache

[Liming Gao]

Jiewen:
  Yes. This is current tool design to leave the configuration instead of hard code. I would like to let 
tool owner to check this logic. 

Thanks
Liming
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, May 9, 2019 8:35 PM
> To: Shi, Steven <steven.shi@intel.com>; Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian <christian.rodriguez@intel.com>; Johnson, Michael
> <michael.johnson@intel.com>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache
> 
> Thanks to explain that.
> I think my general concern is: why we hardcode file suffix "INC" or "MCB" in the tool source code.
> 
> > > > > -            if '.inc' in str(f_ext):
> > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > >                  return False
> 
> What happen if we need skip another type of file later? update tool source code again ?
> 
> My thought is that: the tool should provide capability to skip *some* type of file.
> What "some" type of file to be skipped should be configured outside of tool source code.
> As such, when we need skip more type of file, we just update the configuration, instead of tool source code.
> 
> Just like we have tools_def and build_rule today, when we need update those configuration, we don't need update tool source code.
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: Shi, Steven
> > Sent: Wednesday, May 8, 2019 10:54 PM
> > To: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; devel@edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> > <christian.rodriguez@intel.com>; Johnson, Michael
> > <michael.johnson@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> > Cache
> >
> > > > Can we have better way to describe the binary cache?
> > > Yes. Steven provides some wiki pages for it.
> > https://raw.githubusercontent.com/shijunjing/WikiDoc/master/BuildCache/
> > MultipleBuildCache.jpg
> >
> > I agree it sounds a bit confusing.  Binary Cache is the legacy name for the
> > current model level build cache implementation. The current implementation
> > try only save and recovery the module binary files (e.g. only cache .efi but
> > no .obj, no .lib) which are necessary for the FV generation. But if we add .lib
> > cache and .fv cache support in the future, we could rename it as Build Cache
> > directly.
> >
> >
> > Thanks
> > Steven Shi
> >
> >
> > > -----Original Message-----
> > > From: Gao, Liming
> > > Sent: Thursday, May 9, 2019 1:47 PM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Shi,
> > Steven
> > > <steven.shi@intel.com>
> > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> > > <christian.rodriguez@intel.com>; Johnson, Michael
> > > <michael.johnson@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > Binary
> > > Cache
> > >
> > > Jiewen:
> > >
> > > > -----Original Message-----
> > > > From: Yao, Jiewen
> > > > Sent: Thursday, May 9, 2019 1:13 PM
> > > > To: devel@edk2.groups.io; Shi, Steven <steven.shi@intel.com>
> > > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Rodriguez, Christian
> > > <christian.rodriguez@intel.com>;
> > > > Johnson, Michael <michael.johnson@intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > Binary
> > > Cache
> > > >
> > > > Hi
> > > > Can we have better way to describe the binary cache?
> > > Yes. Steven provides some wiki pages for it.
> > > >
> > > > I have 2 concern:
> > > > 1) we have other type of binary besides MCB. How can we indicate that
> > > without update base tool source code ?
> > > MCB will be combined to single bin file. The problem is that the platform
> > uses
> > > the bin file from output IA32 or X64
> > > directory instead of the module output directory. Binary cache is for the
> > > module output directory.
> > > I suggest to update the platform to consume the module output file. But, I
> > > meet with one tool issue to support
> > > RAW FILE type, because Microcode uses RAW file type. BZ
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1765 is submitted.
> > > I suggest to fix this tool issue first.
> > >
> > > > 2) .inc might be text file, instead of binary. How can we support that?
> > > .inc is similar to .c source file. Its output is .mcb, then be combined to .bin
> > > file. .bin file will be cached.
> > > .inc is not required to be cached. On next build, build tool will compare the
> > > hash value of source file, if
> > > hash value is not changed. Previous cached .bin file will be used.
> > >
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> > Of
> > > > > Steven Shi
> > > > > Sent: Wednesday, May 8, 2019 10:00 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > > > > <liming.gao@intel.com>; Rodriguez, Christian
> > > > > <christian.rodriguez@intel.com>; Johnson, Michael
> > > > > <michael.johnson@intel.com>
> > > > > Subject: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > Binary
> > > > > Cache
> > > > >
> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1723
> > > > >
> > > > > Current Kabylake open platform will build fail if enabled
> > > > > to consume the binary cache, because the binary cache doesn't
> > > > > support to recovery the .mcb microcode file,
> > > > > e.g. m80406E8_00000026.mcb, in a platform level folder which
> > > > > is outside of the module output folder. In normal build
> > > > > without cache, the .mcb file is copied through OS copy/move
> > > > > commands defined in build rules which are not supported by
> > > > > Binary Cache.
> > > > > Change the Binary Cache to skip the .mcb file type module and
> > > > > always rebuild the module to apply the full build rules if
> > > > > it contains .mcb file.
> > > > > ---
> > > > >  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > index 31721a6f9f..6b596c8a65 100644
> > > > > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > @@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
> > > > >          # If library or Module is binary do not skip by hash
> > > > >          if self.IsBinaryModule:
> > > > >              return False
> > > > > -        # .inc is contains binary information so do not skip by hash as
> > well
> > > > > +        # .inc and .mcb is contains binary information so do not skip
> > by
> > > > > hash as well
> > > > >          for f_ext in self.SourceFileList:
> > > > > -            if '.inc' in str(f_ext):
> > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > >                  return False
> > > > >          FileDir = path.join(GlobalData.gBinCacheSource,
> > > > > self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain,
> > self.Arch,
> > > > > self.SourceDir, self.MetaFile.BaseName)
> > > > >          HashFile = path.join(FileDir, self.Name + '.hash')
> > > > > @@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
> > > > >          # If library or Module is binary do not skip by hash
> > > > >          if self.IsBinaryModule:
> > > > >              return False
> > > > > -        # .inc is contains binary information so do not skip by hash as
> > well
> > > > > +        # .inc or '.mcb' is contains binary information so do not skip
> > by
> > > > > hash as well
> > > > >          for f_ext in self.SourceFileList:
> > > > > -            if '.inc' in str(f_ext):
> > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > >                  return False
> > > > >          if GlobalData.gUseHashCache:
> > > > >              # If there is a valid hash or function generated a valid
> > hash;
> > > > > function will return False
> > > > > --
> > > > > 2.17.1.windows.2
> > > > >
> > > > >
> > > > > 

Re: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary Cache

[Yao, Jiewen]

Cool. Thanks!

Then I would like to see a patch to remove INC check. :)



> -----Original Message-----
> From: Gao, Liming
> Sent: Thursday, May 9, 2019 7:05 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Shi, Steven
> <steven.shi@intel.com>; devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> <christian.rodriguez@intel.com>; Johnson, Michael
> <michael.johnson@intel.com>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in Binary
> Cache
> 
> Jiewen:
>   Yes. This is current tool design to leave the configuration instead of hard
> code. I would like to let
> tool owner to check this logic.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Yao, Jiewen
> > Sent: Thursday, May 9, 2019 8:35 PM
> > To: Shi, Steven <steven.shi@intel.com>; Gao, Liming
> <liming.gao@intel.com>; devel@edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> <christian.rodriguez@intel.com>; Johnson, Michael
> > <michael.johnson@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> Binary Cache
> >
> > Thanks to explain that.
> > I think my general concern is: why we hardcode file suffix "INC" or "MCB"
> in the tool source code.
> >
> > > > > > -            if '.inc' in str(f_ext):
> > > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > > >                  return False
> >
> > What happen if we need skip another type of file later? update tool source
> code again ?
> >
> > My thought is that: the tool should provide capability to skip *some* type
> of file.
> > What "some" type of file to be skipped should be configured outside of
> tool source code.
> > As such, when we need skip more type of file, we just update the
> configuration, instead of tool source code.
> >
> > Just like we have tools_def and build_rule today, when we need update
> those configuration, we don't need update tool source code.
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: Shi, Steven
> > > Sent: Wednesday, May 8, 2019 10:54 PM
> > > To: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen
> > > <jiewen.yao@intel.com>; devel@edk2.groups.io
> > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> > > <christian.rodriguez@intel.com>; Johnson, Michael
> > > <michael.johnson@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> Binary
> > > Cache
> > >
> > > > > Can we have better way to describe the binary cache?
> > > > Yes. Steven provides some wiki pages for it.
> > >
> https://raw.githubusercontent.com/shijunjing/WikiDoc/master/BuildCache/
> > > MultipleBuildCache.jpg
> > >
> > > I agree it sounds a bit confusing.  Binary Cache is the legacy name for
> the
> > > current model level build cache implementation. The current
> implementation
> > > try only save and recovery the module binary files (e.g. only cache .efi
> but
> > > no .obj, no .lib) which are necessary for the FV generation. But if we
> add .lib
> > > cache and .fv cache support in the future, we could rename it as Build
> Cache
> > > directly.
> > >
> > >
> > > Thanks
> > > Steven Shi
> > >
> > >
> > > > -----Original Message-----
> > > > From: Gao, Liming
> > > > Sent: Thursday, May 9, 2019 1:47 PM
> > > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Shi,
> > > Steven
> > > > <steven.shi@intel.com>
> > > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Rodriguez, Christian
> > > > <christian.rodriguez@intel.com>; Johnson, Michael
> > > > <michael.johnson@intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > > Binary
> > > > Cache
> > > >
> > > > Jiewen:
> > > >
> > > > > -----Original Message-----
> > > > > From: Yao, Jiewen
> > > > > Sent: Thursday, May 9, 2019 1:13 PM
> > > > > To: devel@edk2.groups.io; Shi, Steven <steven.shi@intel.com>
> > > > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > > > <liming.gao@intel.com>; Rodriguez, Christian
> > > > <christian.rodriguez@intel.com>;
> > > > > Johnson, Michael <michael.johnson@intel.com>
> > > > > Subject: RE: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > > Binary
> > > > Cache
> > > > >
> > > > > Hi
> > > > > Can we have better way to describe the binary cache?
> > > > Yes. Steven provides some wiki pages for it.
> > > > >
> > > > > I have 2 concern:
> > > > > 1) we have other type of binary besides MCB. How can we indicate
> that
> > > > without update base tool source code ?
> > > > MCB will be combined to single bin file. The problem is that the
> platform
> > > uses
> > > > the bin file from output IA32 or X64
> > > > directory instead of the module output directory. Binary cache is for
> the
> > > > module output directory.
> > > > I suggest to update the platform to consume the module output file.
> But, I
> > > > meet with one tool issue to support
> > > > RAW FILE type, because Microcode uses RAW file type. BZ
> > > > https://bugzilla.tianocore.org/show_bug.cgi?id=1765 is submitted.
> > > > I suggest to fix this tool issue first.
> > > >
> > > > > 2) .inc might be text file, instead of binary. How can we support that?
> > > > .inc is similar to .c source file. Its output is .mcb, then be combined
> to .bin
> > > > file. .bin file will be cached.
> > > > .inc is not required to be cached. On next build, build tool will compare
> the
> > > > hash value of source file, if
> > > > hash value is not changed. Previous cached .bin file will be used.
> > > >
> > > > >
> > > > > Thank you
> > > > > Yao Jiewen
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On
> Behalf
> > > Of
> > > > > > Steven Shi
> > > > > > Sent: Wednesday, May 8, 2019 10:00 PM
> > > > > > To: devel@edk2.groups.io
> > > > > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > > > > > <liming.gao@intel.com>; Rodriguez, Christian
> > > > > > <christian.rodriguez@intel.com>; Johnson, Michael
> > > > > > <michael.johnson@intel.com>
> > > > > > Subject: [edk2-devel] [PATCH] BaseTools: Skip .mcb file module in
> > > Binary
> > > > > > Cache
> > > > > >
> > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1723
> > > > > >
> > > > > > Current Kabylake open platform will build fail if enabled
> > > > > > to consume the binary cache, because the binary cache doesn't
> > > > > > support to recovery the .mcb microcode file,
> > > > > > e.g. m80406E8_00000026.mcb, in a platform level folder which
> > > > > > is outside of the module output folder. In normal build
> > > > > > without cache, the .mcb file is copied through OS copy/move
> > > > > > commands defined in build rules which are not supported by
> > > > > > Binary Cache.
> > > > > > Change the Binary Cache to skip the .mcb file type module and
> > > > > > always rebuild the module to apply the full build rules if
> > > > > > it contains .mcb file.
> > > > > > ---
> > > > > >  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 ++++----
> > > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > > index 31721a6f9f..6b596c8a65 100644
> > > > > > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > > > > > @@ -3925,9 +3925,9 @@ class ModuleAutoGen(AutoGen):
> > > > > >          # If library or Module is binary do not skip by hash
> > > > > >          if self.IsBinaryModule:
> > > > > >              return False
> > > > > > -        # .inc is contains binary information so do not skip by
> hash as
> > > well
> > > > > > +        # .inc and .mcb is contains binary information so do not
> skip
> > > by
> > > > > > hash as well
> > > > > >          for f_ext in self.SourceFileList:
> > > > > > -            if '.inc' in str(f_ext):
> > > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > > >                  return False
> > > > > >          FileDir = path.join(GlobalData.gBinCacheSource,
> > > > > > self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain,
> > > self.Arch,
> > > > > > self.SourceDir, self.MetaFile.BaseName)
> > > > > >          HashFile = path.join(FileDir, self.Name + '.hash')
> > > > > > @@ -4138,9 +4138,9 @@ class ModuleAutoGen(AutoGen):
> > > > > >          # If library or Module is binary do not skip by hash
> > > > > >          if self.IsBinaryModule:
> > > > > >              return False
> > > > > > -        # .inc is contains binary information so do not skip by
> hash as
> > > well
> > > > > > +        # .inc or '.mcb' is contains binary information so do not
> skip
> > > by
> > > > > > hash as well
> > > > > >          for f_ext in self.SourceFileList:
> > > > > > -            if '.inc' in str(f_ext):
> > > > > > +            if '.inc' in str(f_ext) or '.mcb' in str(f_ext):
> > > > > >                  return False
> > > > > >          if GlobalData.gUseHashCache:
> > > > > >              # If there is a valid hash or function generated a
> valid
> > > hash;
> > > > > > function will return False
> > > > > > --
> > > > > > 2.17.1.windows.2
> > > > > >
> > > > > >
> > > > > >