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

Yao, Jiewen jiewen.yao at intel.com
Thu May 9 14:22:56 UTC 2019 

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 at intel.com>; Shi, Steven
> <steven.shi at intel.com>; devel at edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng at intel.com>; Rodriguez, Christian
> <christian.rodriguez at intel.com>; Johnson, Michael
> <michael.johnson at 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 at intel.com>; Gao, Liming
> <liming.gao at intel.com>; devel at edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng at intel.com>; Rodriguez, Christian
> <christian.rodriguez at intel.com>; Johnson, Michael
> > <michael.johnson at 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 at intel.com>; Yao, Jiewen
> > > <jiewen.yao at intel.com>; devel at edk2.groups.io
> > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Rodriguez, Christian
> > > <christian.rodriguez at intel.com>; Johnson, Michael
> > > <michael.johnson at 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 at intel.com>; devel at edk2.groups.io; Shi,
> > > Steven
> > > > <steven.shi at intel.com>
> > > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Rodriguez, Christian
> > > > <christian.rodriguez at intel.com>; Johnson, Michael
> > > > <michael.johnson at 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 at edk2.groups.io; Shi, Steven <steven.shi at intel.com>
> > > > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming
> > > > <liming.gao at intel.com>; Rodriguez, Christian
> > > > <christian.rodriguez at intel.com>;
> > > > > Johnson, Michael <michael.johnson at 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 at edk2.groups.io [mailto:devel at edk2.groups.io] On
> Behalf
> > > Of
> > > > > > Steven Shi
> > > > > > Sent: Wednesday, May 8, 2019 10:00 PM
> > > > > > To: devel at edk2.groups.io
> > > > > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming
> > > > > > <liming.gao at intel.com>; Rodriguez, Christian
> > > > > > <christian.rodriguez at intel.com>; Johnson, Michael
> > > > > > <michael.johnson at 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
> > > > > >
> > > > > >
> > > > > > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#40365): https://edk2.groups.io/g/devel/message/40365
Mute This Topic: https://groups.io/mt/31552068/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-

More information about the edk2-devel-archive mailing list