[edk2-devel] [Patch 02/11] BaseTools: Split WorkspaceAutoGen._InitWorker into multiple functions
Bob Feng
bob.c.feng at intel.com
Tue Jul 30 02:10:41 UTC 2019
Hi Phil,
Thanks for your comments. I agree it will be easier to review if this patch is split into multiple smaller ones.
Since the later patches in Multiple-process autogen patch series are based on this patch. It will be bigger effort to recreate this whole patch series. So I'd like to prefer not to change...
I'm willing to answer the questions for this patch.
Thanks,
Bob
-----Original Message-----
From: Philippe Mathieu-Daudé [mailto:philmd at redhat.com]
Sent: Monday, July 29, 2019 11:03 PM
To: devel at edk2.groups.io; Feng, Bob C <bob.c.feng at intel.com>
Cc: Gao, Liming <liming.gao at intel.com>
Subject: Re: [edk2-devel] [Patch 02/11] BaseTools: Split WorkspaceAutoGen._InitWorker into multiple functions
Hi Bob,
On 7/29/19 10:44 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.
Patch looks good, however not trivial to review, you are refactoring 1 big function into 13 (or 14) ones, it would be much simpler to review if you use 1 patch per function extracted. If you mind and that is not too much effort, that would be appreciated. If you prefer not, I'll review your patch more carefully.
Thanks,
Phil.
>
> Cc: Liming Gao <liming.gao at intel.com>
> Signed-off-by: Bob Feng <bob.c.feng at 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
> # 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 (#44579): https://edk2.groups.io/g/devel/message/44579
Mute This Topic: https://groups.io/mt/32640228/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