From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: christian.rodriguez@intel.com) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by groups.io with SMTP; Fri, 10 May 2019 08:28:07 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 May 2019 08:28:06 -0700 X-ExtLoop1: 1 Received: from orsmsx102.amr.corp.intel.com ([10.22.225.129]) by orsmga005.jf.intel.com with ESMTP; 10 May 2019 08:28:06 -0700 Received: from orsmsx112.amr.corp.intel.com ([169.254.3.109]) by ORSMSX102.amr.corp.intel.com ([169.254.3.72]) with mapi id 14.03.0415.000; Fri, 10 May 2019 08:28:06 -0700 From: "Christian Rodriguez" To: "Carsey, Jaben" , "devel@edk2.groups.io" CC: "Feng, Bob C" , "Gao, Liming" , "Zhu, Yonghong" Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed Thread-Topic: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed Thread-Index: AQHVBq4PRL7Y+aN3pkyXMseGtKl8F6ZjcrKQgAD9E/A= Date: Fri, 10 May 2019 15:28:06 +0000 Message-ID: <3A7DCC9A944C6149BF832E1C9B718ABC01EDA27B@ORSMSX112.amr.corp.intel.com> References: <20190509212719.6156-1-christian.rodriguez@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTQ4NWIxNTUtYzcyYi00OGZmLWFjOTItYmM3YmQwMWRiMjE5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUjlzUVYrdXVGRDhralJ4WWthNFRxYW9ZZzJHXC9KQ1hIYTZ6NEd3U3l5Nkg3MVJKWUoxaVBhcUFjRFRDUW83N2oifQ== dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [10.22.254.139] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Replies inline. >-----Original Message----- >From: Carsey, Jaben >Sent: Thursday, May 9, 2019 4:39 PM >To: devel@edk2.groups.io; Rodriguez, Christian > >Cc: Feng, Bob C ; Gao, Liming >; Zhu, Yonghong >Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not mentione= d >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 ; Gao, Liming >> ; Zhu, Yonghong >> Subject: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >> in inf are not hashed >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1787 >> >> 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 >> Cc: Bob Feng >> Cc: Liming Gao >> Cc: Yonghong Zhu >> --- >> 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 =3D 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=3Dlambda = x: >> x.PackageName): >> @@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen): >> Content =3D f.read() >> f.close() >> m.update(Content) >> + >> # Add Module's source files >> + localSourceFileList =3D set() >> if self.SourceFileList: >> for File in sorted(self.SourceFileList, key=3Dlambda x: st= r(x)): >> + localSourceFileList.add(str(File)) >> f =3D open(str(File), 'rb') >> Content =3D f.read() >> f.close() >> m.update(Content) >> >> + # Get a list of Module's local header files not included in me= taFile >> + localHeaderList =3D set() >> + if self.SourceDir: >> + for root, dirs, files in os.walk(self.SourceDir): >> + for aFile in files: >> + filePath =3D 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 differen= t 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:=20 1. self.SourceFileList contains source file paths of class PathClass and n= ot type str 2. If we want to use self.SourceFileList you must convert PathClass to a s= tr 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 =3D list(localHeaderList) >> + for File in sorted(localHeaderList): >> + f =3D open(str(File), 'rb') >> + Content =3D f.read() > >Can you use 'with open(...) as...:' syntax to make sure the file is alway= s closed? I just used the same implementation as the above existing code. I can defi= nitely change it to use 'with open(...)'. Though explicitly calling f.close() as below should be making sure the fil= e is always closed. > >> + f.close() >> + m.update(Content) >> + >> ModuleHashFile =3D path.join(self.BuildDir, self.Name + ".hash= ") >> if self.Name not in GlobalData.gModuleHash[self.Arch]: >> GlobalData.gModuleHash[self.Arch][self.Name] =3D >> m.hexdigest() >> -- >> 2.19.1.windows.1 >> >> >>=20