Replies inline. >-----Original Message----- >From: Carsey, Jaben >Sent: Thursday, May 9, 2019 4:39 PM >To: devel@edk2.groups.io; Rodriguez, Christian ><christian.rodrig...@intel.com> >Cc: Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming ><liming....@intel.com>; Zhu, Yonghong <yonghong....@intel.com> >Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >Some questions inline. > >> -----Original Message----- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> Christian Rodriguez >> Sent: Thursday, May 09, 2019 2:27 PM >> To: devel@edk2.groups.io >> Cc: Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming >> <liming....@intel.com>; Zhu, Yonghong <yonghong....@intel.com> >> Subject: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >> in inf are not hashed >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1787 >> >> Get a list of local header files that are not present in the MetaFile >> for this module. Add those local header files into the hashing >> algorithm for a module. If a local header file is not present in the >> MetaFile, the module will still build correctly though the hashing >> system didn't know about it before. >> >> Signed-off-by: Christian Rodriguez <christian.rodrig...@intel.com> >> Cc: Bob Feng <bob.c.f...@intel.com> >> Cc: Liming Gao <liming....@intel.com> >> Cc: Yonghong Zhu <yonghong....@intel.com> >> --- >> BaseTools/Source/Python/AutoGen/AutoGen.py | 24 >> ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py >> b/BaseTools/Source/Python/AutoGen/AutoGen.py >> index 31721a6f9f..bd282d3ec1 100644 >> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py >> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py >> @@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen): >> if self.Name in GlobalData.gModuleHash[self.Arch] and >> GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy(): >> return False >> m = hashlib.md5() >> + >> # Add Platform level hash >> m.update(GlobalData.gPlatformHash.encode('utf-8')) >> + >> # Add Package level hash >> if self.DependentPackageList: >> for Pkg in sorted(self.DependentPackageList, key=lambda x: >> x.PackageName): >> @@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen): >> Content = f.read() >> f.close() >> m.update(Content) >> + >> # Add Module's source files >> + localSourceFileList = set() >> if self.SourceFileList: >> for File in sorted(self.SourceFileList, key=lambda x: str(x)): >> + localSourceFileList.add(str(File)) >> f = open(str(File), 'rb') >> Content = f.read() >> f.close() >> m.update(Content) >> >> + # Get a list of Module's local header files not included in metaFile >> + localHeaderList = set() >> + if self.SourceDir: >> + for root, dirs, files in os.walk(self.SourceDir): >> + for aFile in files: >> + filePath = os.path.join(self.WorkspaceDir, >> + os.path.join(root, >> aFile)) >> + if not filePath.endswith(('.h', '.H')): >> + continue >> + if filePath not in localSourceFileList: > >Confused about localSourceFileList. >Why is it a set and named list? >Why not just use self.SourceFileList? > The naming convention could be better and I can address that in a different patch, if we decide to go forward with this idea overall. It should probably be named a set. The reason to using this new set is for a few reasons: 1. self.SourceFileList contains source file paths of class PathClass and not type str 2. If we want to use self.SourceFileList you must convert PathClass to a str for string comparison The set just allows for a unique list of objects and theoretically faster to check existence.
>> + localHeaderList.add(filePath) >> + >> + # Add Module's local header files >> + localHeaderList = list(localHeaderList) >> + for File in sorted(localHeaderList): >> + f = open(str(File), 'rb') >> + Content = f.read() > >Can you use 'with open(...) as...:' syntax to make sure the file is always >closed? I just used the same implementation as the above existing code. I can definitely change it to use 'with open(...)'. Though explicitly calling f.close() as below should be making sure the file is always closed. > >> + f.close() >> + m.update(Content) >> + >> ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash") >> if self.Name not in GlobalData.gModuleHash[self.Arch]: >> GlobalData.gModuleHash[self.Arch][self.Name] = >> m.hexdigest() >> -- >> 2.19.1.windows.1 >> >> >> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40443): https://edk2.groups.io/g/devel/message/40443 Mute This Topic: https://groups.io/mt/31570019/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
