Hi Phil, I double checked your previous mail. I missed your comments inside the patch content then. Sorry.
Platform.Pcds Yes, this statement looks odd, but it's not an incorrect copy/paste. I think the basetools code style is tend to lazy-evaluation, which means the Pcds will be evaluated when it's referenced, so Platform.Pcds is designed to be a property, it can't be called as Platform.Pcds(). I think if Platform.Pcds is written to be Platform.Pcds(), it will look normal. Platform.Pcds is there because the later code need to use the data which is evaluated in Platform.Pcds, but the direct result of Pcds is not needed at that time. Platform.Pcds is not introduced by Patch 2/10, it's been there for some time. Thanks, Bob -----Original Message----- From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Philippe Mathieu-Daudé Sent: Wednesday, July 31, 2019 7:34 PM To: devel@edk2.groups.io; Feng, Bob C <bob.c.f...@intel.com> Cc: Gao, Liming <liming....@intel.com> Subject: Re: [edk2-devel] [Patch 02/10 V5] BaseTools: Split WorkspaceAutoGen._InitWorker into multiple functions On 7/31/19 7:52 AM, Bob Feng wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875 > > The WorkspaceAutoGen.__InitWorker function is too long, it's hard to > read and understand. > This patch is to separate the __InitWorker into multiple small ones. > > Cc: Liming Gao <liming....@intel.com> > Signed-off-by: Bob Feng <bob.c.f...@intel.com> > --- > BaseTools/Source/Python/AutoGen/AutoGen.py | 247 > +++++++++++++-------- > 1 file changed, 152 insertions(+), 95 deletions(-) > > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py > b/BaseTools/Source/Python/AutoGen/AutoGen.py > index c5b3fbb0a87f..9e06bb942126 100644 > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py > @@ -333,13 +333,58 @@ class WorkspaceAutoGen(AutoGen): > self._GuidDict = {} > > # there's many relative directory operations, so ... > os.chdir(self.WorkspaceDir) > > + self.MergeArch() > + self.ValidateBuildTarget() > + > + EdkLogger.info("") > + if self.ArchList: > + EdkLogger.info('%-16s = %s' % ("Architecture(s)", ' > '.join(self.ArchList))) > + EdkLogger.info('%-16s = %s' % ("Build target", self.BuildTarget)) > + EdkLogger.info('%-16s = %s' % ("Toolchain", self.ToolChain)) > + > + EdkLogger.info('\n%-24s = %s' % ("Active Platform", self.Platform)) > + if BuildModule: > + EdkLogger.info('%-24s = %s' % ("Active Module", > + BuildModule)) > + > + if self.FdfFile: > + EdkLogger.info('%-24s = %s' % ("Flash Image Definition", > + self.FdfFile)) > + > + EdkLogger.verbose("\nFLASH_DEFINITION = %s" % self.FdfFile) > + > + if Progress: > + Progress.Start("\nProcessing meta-data") > # > - # Merge Arch > + # Mark now build in AutoGen Phase > # > + GlobalData.gAutoGenPhase = True > + self.ProcessModuleFromPdf() > + self.ProcessPcdType() > + self.ProcessMixedPcd() > + self.GetPcdsFromFDF() > + self.CollectAllPcds() > + self.GeneratePkgLevelHash() > + # > + # Check PCDs token value conflict in each DEC file. > + # > + self._CheckAllPcdsTokenValueConflict() > + # > + # Check PCD type and definition between DSC and DEC > + # > + self._CheckPcdDefineAndType() > + > + self.CreateBuildOptionsFile() > + self.CreatePcdTokenNumberFile() > + self.CreateModuleHashInfo() > + GlobalData.gAutoGenPhase = False > + > + # > + # Merge Arch > + # > + def MergeArch(self): > if not self.ArchList: > ArchList = set(self.Platform.SupArchList) > else: > ArchList = set(self.ArchList) & set(self.Platform.SupArchList) > if not ArchList: > @@ -349,57 +394,49 @@ class WorkspaceAutoGen(AutoGen): > SkippedArchList = > set(self.ArchList).symmetric_difference(set(self.Platform.SupArchList)) > EdkLogger.verbose("\nArch [%s] is ignored because the platform > supports [%s] only!" > % (" ".join(SkippedArchList), " > ".join(self.Platform.SupArchList))) > self.ArchList = tuple(ArchList) > > - # Validate build target > + # Validate build target > + def ValidateBuildTarget(self): > if self.BuildTarget not in self.Platform.BuildTargets: > EdkLogger.error("build", PARAMETER_INVALID, > ExtraData="Build target [%s] is not supported by > the platform. [Valid target: %s]" > % (self.BuildTarget, " > ".join(self.Platform.BuildTargets))) > - > - > - # parse FDF file to get PCDs in it, if any > + @cached_property > + def FdfProfile(self): > if not self.FdfFile: > self.FdfFile = self.Platform.FlashDefinition > > - EdkLogger.info("") > - if self.ArchList: > - EdkLogger.info('%-16s = %s' % ("Architecture(s)", ' > '.join(self.ArchList))) > - EdkLogger.info('%-16s = %s' % ("Build target", self.BuildTarget)) > - EdkLogger.info('%-16s = %s' % ("Toolchain", self.ToolChain)) > - > - EdkLogger.info('\n%-24s = %s' % ("Active Platform", self.Platform)) > - if BuildModule: > - EdkLogger.info('%-24s = %s' % ("Active Module", BuildModule)) > - > + FdfProfile = None > if self.FdfFile: > - EdkLogger.info('%-24s = %s' % ("Flash Image Definition", > self.FdfFile)) > - > - EdkLogger.verbose("\nFLASH_DEFINITION = %s" % self.FdfFile) > - > - if Progress: > - Progress.Start("\nProcessing meta-data") > - > - if self.FdfFile: > - # > - # Mark now build in AutoGen Phase > - # > - GlobalData.gAutoGenPhase = True > Fdf = FdfParser(self.FdfFile.Path) > Fdf.ParseFile() > GlobalData.gFdfParser = Fdf > - GlobalData.gAutoGenPhase = False > - PcdSet = Fdf.Profile.PcdDict > if Fdf.CurrentFdName and Fdf.CurrentFdName in Fdf.Profile.FdDict: > FdDict = Fdf.Profile.FdDict[Fdf.CurrentFdName] > for FdRegion in FdDict.RegionList: > if str(FdRegion.RegionType) is 'FILE' and > self.Platform.VpdToolGuid in str(FdRegion.RegionDataList): > if int(FdRegion.Offset) % 8 != 0: > EdkLogger.error("build", FORMAT_INVALID, 'The > VPD Base Address %s must be 8-byte aligned.' % (FdRegion.Offset)) > - ModuleList = Fdf.Profile.InfList > - self.FdfProfile = Fdf.Profile > + FdfProfile = Fdf.Profile > + else: > + if self.FdTargetList: > + EdkLogger.info("No flash definition file found. FD [%s] will > be ignored." % " ".join(self.FdTargetList)) > + self.FdTargetList = [] > + if self.FvTargetList: > + EdkLogger.info("No flash definition file found. FV [%s] will > be ignored." % " ".join(self.FvTargetList)) > + self.FvTargetList = [] > + if self.CapTargetList: > + EdkLogger.info("No flash definition file found. Capsule [%s] > will be ignored." % " ".join(self.CapTargetList)) > + self.CapTargetList = [] > + > + return FdfProfile > + > + def ProcessModuleFromPdf(self): > + > + if self.FdfProfile: > for fvname in self.FvTargetList: > if fvname.upper() not in self.FdfProfile.FvDict: > EdkLogger.error("build", OPTION_VALUE_INVALID, > "No such an FV in FDF file: %s" % > fvname) > > @@ -407,64 +444,60 @@ class WorkspaceAutoGen(AutoGen): > # but the path (self.MetaFile.Path) is the real path > for key in self.FdfProfile.InfDict: > if key == 'ArchTBD': > MetaFile_cache = defaultdict(set) > for Arch in self.ArchList: > - Current_Platform_cache = > self.BuildDatabase[self.MetaFile, Arch, Target, Toolchain] > + Current_Platform_cache = > + self.BuildDatabase[self.MetaFile, Arch, self.BuildTarget, > + self.ToolChain] > for Pkey in Current_Platform_cache.Modules: > > MetaFile_cache[Arch].add(Current_Platform_cache.Modules[Pkey].MetaFile) > for Inf in self.FdfProfile.InfDict[key]: > ModuleFile = PathClass(NormPath(Inf), > GlobalData.gWorkspace, Arch) > for Arch in self.ArchList: > if ModuleFile in MetaFile_cache[Arch]: > break > else: > - ModuleData = self.BuildDatabase[ModuleFile, > Arch, Target, Toolchain] > + ModuleData = > + self.BuildDatabase[ModuleFile, Arch, self.BuildTarget, > + self.ToolChain] > if not ModuleData.IsBinaryModule: > EdkLogger.error('build', > PARSER_ERROR, "Module %s NOT found in DSC file; Is it really a binary > module?" % ModuleFile) > > else: > for Arch in self.ArchList: > if Arch == key: > - Platform = self.BuildDatabase[self.MetaFile, > Arch, Target, Toolchain] > + Platform = > + self.BuildDatabase[self.MetaFile, Arch, self.BuildTarget, > + self.ToolChain] > MetaFileList = set() > for Pkey in Platform.Modules: > > MetaFileList.add(Platform.Modules[Pkey].MetaFile) > for Inf in self.FdfProfile.InfDict[key]: > ModuleFile = PathClass(NormPath(Inf), > GlobalData.gWorkspace, Arch) > if ModuleFile in MetaFileList: > continue > - ModuleData = self.BuildDatabase[ModuleFile, > Arch, Target, Toolchain] > + ModuleData = > + self.BuildDatabase[ModuleFile, Arch, self.BuildTarget, > + self.ToolChain] > if not ModuleData.IsBinaryModule: > EdkLogger.error('build', > PARSER_ERROR, "Module %s NOT found in DSC file; Is it really a binary > module?" % ModuleFile) > > - else: > - PcdSet = {} > - ModuleList = [] > - self.FdfProfile = None > - if self.FdTargetList: > - EdkLogger.info("No flash definition file found. FD [%s] will > be ignored." % " ".join(self.FdTargetList)) > - self.FdTargetList = [] > - if self.FvTargetList: > - EdkLogger.info("No flash definition file found. FV [%s] will > be ignored." % " ".join(self.FvTargetList)) > - self.FvTargetList = [] > - if self.CapTargetList: > - EdkLogger.info("No flash definition file found. Capsule [%s] > will be ignored." % " ".join(self.CapTargetList)) > - self.CapTargetList = [] > - > - # apply SKU and inject PCDs from Flash Definition file > + > + > + # parse FDF file to get PCDs in it, if any > + def GetPcdsFromFDF(self): > + > + if self.FdfProfile: > + PcdSet = self.FdfProfile.PcdDict > + # handle the mixed pcd in FDF file > + for key in PcdSet: > + if key in GlobalData.MixedPcd: > + Value = PcdSet[key] > + del PcdSet[key] > + for item in GlobalData.MixedPcd[key]: > + PcdSet[item] = Value > + self.VerifyPcdDeclearation(PcdSet) > + > + def ProcessPcdType(self): > for Arch in self.ArchList: > - Platform = self.BuildDatabase[self.MetaFile, Arch, Target, > Toolchain] > - PlatformPcds = Platform.Pcds > - self._GuidDict = Platform._GuidDict > - SourcePcdDict = {TAB_PCDS_DYNAMIC_EX:set(), > TAB_PCDS_PATCHABLE_IN_MODULE:set(),TAB_PCDS_DYNAMIC:set(),TAB_PCDS_FIXED_AT_BUILD:set()} > - BinaryPcdDict = {TAB_PCDS_DYNAMIC_EX:set(), > TAB_PCDS_PATCHABLE_IN_MODULE:set()} > - SourcePcdDict_Keys = SourcePcdDict.keys() > - BinaryPcdDict_Keys = BinaryPcdDict.keys() > - > + Platform = self.BuildDatabase[self.MetaFile, Arch, > self.BuildTarget, self.ToolChain] > + Platform.Pcds v4 comment not answered/addressed. > # generate the SourcePcdDict and BinaryPcdDict > - PGen = PlatformAutoGen(self, self.MetaFile, Target, Toolchain, > Arch) > + PGen = PlatformAutoGen(self, self.MetaFile, > + self.BuildTarget, self.ToolChain, Arch) > for BuildData in list(PGen.BuildDatabase._CACHE_.values()): > if BuildData.Arch != Arch: > continue > if BuildData.MetaFile.Ext == '.inf': > for key in BuildData.Pcds: > @@ -483,11 +516,11 @@ class WorkspaceAutoGen(AutoGen): > BuildData.Pcds[key].Type = > PcdInPlatform.Type > BuildData.Pcds[key].Pending = False > else: > #Pcd used in Library, Pcd Type from > reference module if Pcd Type is Pending > if BuildData.Pcds[key].Pending: > - MGen = ModuleAutoGen(self, > BuildData.MetaFile, Target, Toolchain, Arch, self.MetaFile) > + MGen = ModuleAutoGen(self, > + BuildData.MetaFile, self.BuildTarget, self.ToolChain, Arch, > + self.MetaFile) > if MGen and MGen.IsLibrary: > if MGen in PGen.LibraryAutoGenList: > ReferenceModules = > MGen.ReferenceModules > for ReferenceModule in > ReferenceModules: > if ReferenceModule.MetaFile > in Platform.Modules: > @@ -497,10 +530,24 @@ class WorkspaceAutoGen(AutoGen): > if > PcdInReferenceModule.Type: > > BuildData.Pcds[key].Type = PcdInReferenceModule.Type > > BuildData.Pcds[key].Pending = False > break > > + def ProcessMixedPcd(self): > + for Arch in self.ArchList: > + SourcePcdDict = {TAB_PCDS_DYNAMIC_EX:set(), > TAB_PCDS_PATCHABLE_IN_MODULE:set(),TAB_PCDS_DYNAMIC:set(),TAB_PCDS_FIXED_AT_BUILD:set()} > + BinaryPcdDict = {TAB_PCDS_DYNAMIC_EX:set(), > TAB_PCDS_PATCHABLE_IN_MODULE:set()} > + SourcePcdDict_Keys = SourcePcdDict.keys() > + BinaryPcdDict_Keys = BinaryPcdDict.keys() > + > + # generate the SourcePcdDict and BinaryPcdDict > + PGen = PlatformAutoGen(self, self.MetaFile, self.BuildTarget, > self.ToolChain, Arch) > + for BuildData in list(PGen.BuildDatabase._CACHE_.values()): > + if BuildData.Arch != Arch: > + continue > + if BuildData.MetaFile.Ext == '.inf': > + for key in BuildData.Pcds: > if TAB_PCDS_DYNAMIC_EX in BuildData.Pcds[key].Type: > if BuildData.IsBinaryModule: > > BinaryPcdDict[TAB_PCDS_DYNAMIC_EX].add((BuildData.Pcds[key].TokenCName, > BuildData.Pcds[key].TokenSpaceGuidCName)) > else: > > SourcePcdDict[TAB_PCDS_DYNAMIC_EX].add((BuildData.Pcds[key].TokenCName > , BuildData.Pcds[key].TokenSpaceGuidCName)) > @@ -514,12 +561,11 @@ class WorkspaceAutoGen(AutoGen): > > elif TAB_PCDS_DYNAMIC in BuildData.Pcds[key].Type: > > SourcePcdDict[TAB_PCDS_DYNAMIC].add((BuildData.Pcds[key].TokenCName, > BuildData.Pcds[key].TokenSpaceGuidCName)) > elif TAB_PCDS_FIXED_AT_BUILD in > BuildData.Pcds[key].Type: > > SourcePcdDict[TAB_PCDS_FIXED_AT_BUILD].add((BuildData.Pcds[key].TokenCName, > BuildData.Pcds[key].TokenSpaceGuidCName)) > - else: > - pass > + > # > # A PCD can only use one type for all source modules > # > for i in SourcePcdDict_Keys: > for j in SourcePcdDict_Keys: > @@ -588,27 +634,38 @@ class WorkspaceAutoGen(AutoGen): > del BuildData.Pcds[key] > BuildData.Pcds[newkey] = Value > break > break > > - # handle the mixed pcd in FDF file > - for key in PcdSet: > - if key in GlobalData.MixedPcd: > - Value = PcdSet[key] > - del PcdSet[key] > - for item in GlobalData.MixedPcd[key]: > - PcdSet[item] = Value > + #Collect package set information from INF of FDF > + @cached_property > + def PkgSet(self): > + if not self.FdfFile: > + self.FdfFile = self.Platform.FlashDefinition > > - #Collect package set information from INF of FDF > + if self.FdfFile: > + ModuleList = self.FdfProfile.InfList > + else: > + ModuleList = [] > + Pkgs = {} > + for Arch in self.ArchList: > + Platform = self.BuildDatabase[self.MetaFile, Arch, > self.BuildTarget, self.ToolChain] > + PGen = PlatformAutoGen(self, self.MetaFile, > + self.BuildTarget, self.ToolChain, Arch) > PkgSet = set() > for Inf in ModuleList: > ModuleFile = PathClass(NormPath(Inf), GlobalData.gWorkspace, > Arch) > if ModuleFile in Platform.Modules: > continue > - ModuleData = self.BuildDatabase[ModuleFile, Arch, Target, > Toolchain] > + ModuleData = self.BuildDatabase[ModuleFile, Arch, > + self.BuildTarget, self.ToolChain] > PkgSet.update(ModuleData.Packages) > - Pkgs = list(PkgSet) + list(PGen.PackageList) > + Pkgs[Arch] = list(PkgSet) + list(PGen.PackageList) > + return Pkgs > + > + def VerifyPcdDeclearation(self,PcdSet): > + for Arch in self.ArchList: > + Platform = self.BuildDatabase[self.MetaFile, Arch, > self.BuildTarget, self.ToolChain] > + Pkgs = self.PkgSet[Arch] > DecPcds = set() > DecPcdsKey = set() > for Pkg in Pkgs: > for Pcd in Pkg.Pcds: > DecPcds.add((Pcd[0], Pcd[1])) @@ -636,37 +693,33 > @@ class WorkspaceAutoGen(AutoGen): > PARSER_ERROR, > "Using Dynamic or DynamicEx type of PCD > [%s.%s] in FDF file is not allowed." % (Guid, Name), > File = self.FdfProfile.PcdFileLineDict[Name, > Guid, Fileds][0], > Line = self.FdfProfile.PcdFileLineDict[Name, > Guid, Fileds][1] > ) > + def CollectAllPcds(self): > > - Pa = PlatformAutoGen(self, self.MetaFile, Target, Toolchain, > Arch) > + for Arch in self.ArchList: > + Pa = PlatformAutoGen(self, self.MetaFile, > + self.BuildTarget, self.ToolChain, Arch) > # > # Explicitly collect platform's dynamic PCDs > # > Pa.CollectPlatformDynamicPcds() > Pa.CollectFixedAtBuildPcds() > self.AutoGenObjectList.append(Pa) > > - # > - # Generate Package level hash value > - # > + # > + # Generate Package level hash value > + # > + def GeneratePkgLevelHash(self): > + for Arch in self.ArchList: > GlobalData.gPackageHash = {} > if GlobalData.gUseHashCache: > - for Pkg in Pkgs: > + for Pkg in self.PkgSet[Arch]: > self._GenPkgLevelHash(Pkg) > > - # > - # Check PCDs token value conflict in each DEC file. > - # > - self._CheckAllPcdsTokenValueConflict() > - > - # > - # Check PCD type and definition between DSC and DEC > - # > - self._CheckPcdDefineAndType() > > + def CreateBuildOptionsFile(self): > # > # Create BuildOptions Macro & PCD metafile, also add the Active > Platform and FDF file. > # > content = 'gCommandLineDefines: ' > content += str(GlobalData.gCommandLineDefines) > @@ -681,27 +734,31 @@ class WorkspaceAutoGen(AutoGen): > content += 'Flash Image Definition: ' > content += str(self.FdfFile) > content += TAB_LINE_BREAK > SaveFileOnChange(os.path.join(self.BuildDir, 'BuildOptions'), > content, False) > > + def CreatePcdTokenNumberFile(self): > # > # Create PcdToken Number file for Dynamic/DynamicEx Pcd. > # > PcdTokenNumber = 'PcdTokenNumber: ' > - if Pa.PcdTokenNumber: > - if Pa.DynamicPcdList: > - for Pcd in Pa.DynamicPcdList: > - PcdTokenNumber += TAB_LINE_BREAK > - PcdTokenNumber += str((Pcd.TokenCName, > Pcd.TokenSpaceGuidCName)) > - PcdTokenNumber += ' : ' > - PcdTokenNumber += str(Pa.PcdTokenNumber[Pcd.TokenCName, > Pcd.TokenSpaceGuidCName]) > + for Arch in self.ArchList: > + Pa = PlatformAutoGen(self, self.MetaFile, self.BuildTarget, > self.ToolChain, Arch) > + if Pa.PcdTokenNumber: > + if Pa.DynamicPcdList: > + for Pcd in Pa.DynamicPcdList: > + PcdTokenNumber += TAB_LINE_BREAK > + PcdTokenNumber += str((Pcd.TokenCName, > Pcd.TokenSpaceGuidCName)) > + PcdTokenNumber += ' : ' > + PcdTokenNumber += > + str(Pa.PcdTokenNumber[Pcd.TokenCName, Pcd.TokenSpaceGuidCName]) > SaveFileOnChange(os.path.join(self.BuildDir, > 'PcdTokenNumber'), PcdTokenNumber, False) > > + def CreateModuleHashInfo(self): > # > # Get set of workspace metafiles > # > - AllWorkSpaceMetaFiles = self._GetMetaFiles(Target, Toolchain, Arch) > + AllWorkSpaceMetaFiles = self._GetMetaFiles(self.BuildTarget, > + self.ToolChain) > > # > # Retrieve latest modified time of all metafiles > # > SrcTimeStamp = 0 > @@ -759,11 +816,11 @@ class WorkspaceAutoGen(AutoGen): > f.close() > m.update(Content) > SaveFileOnChange(HashFile, m.hexdigest(), False) > GlobalData.gPackageHash[Pkg.PackageName] = m.hexdigest() > > - def _GetMetaFiles(self, Target, Toolchain, Arch): > + def _GetMetaFiles(self, Target, Toolchain): > AllWorkSpaceMetaFiles = set() > # > # add fdf > # > if self.FdfFile: > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44679): https://edk2.groups.io/g/devel/message/44679 Mute This Topic: https://groups.io/mt/32662290/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
