Re: [edk2-devel] [PATCH v2 1/4] BaseTools: Generate multiple rules when multiple output files

["PierreGondois" <pierre.gondois@arm.com>]

Hello Yuwei and Bob,
Thank you for the review Yuwei.
Bob, are the patches acceptable?

Regards,
Pierre

-----Original Message-----
From: Chen, Yuwei <yuwei.chen@intel.com> 
Sent: Friday, June 19, 2020 7:35 AM
To: devel@edk2.groups.io; Pierre Gondois <Pierre.Gondois@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Tomas Pilar <Tomas.Pilar@arm.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; nd <nd@arm.com>
Subject: RE: [edk2-devel] [PATCH v2 1/4] BaseTools: Generate multiple rules when multiple output files

Hi, Pierre

The patch looks good.

Thanks,
Yuwei
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> PierreGondois
> Sent: Thursday, June 18, 2020 10:27 PM
> To: devel@edk2.groups.io
> Cc: Pierre Gondois <Pierre.Gondois@arm.com>; sami.mujawar@arm.com; 
> tomas.pilar@arm.com; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming 
> <liming.gao@intel.com>; nd@arm.com
> Subject: [edk2-devel] [PATCH v2 1/4] BaseTools: Generate multiple 
> rules when multiple output files
> 
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> This patch modifies the Makefile generation not to stop adding Makfile 
> rules when the first final target is found.
> E.g.:
> If the following rules are described in build_rule.txt:
>  -[Rule1]: .X files generate .Y and .Z files;
>  -[Rule2]: .Z files generate .Z1 files.
> Currently, if a File1.X file was part of the sources of a module, only 
> [Rule1] would be generated in the Makefile.
> Indeed, there are no rules to apply to .Y files: .Y files are a final target.
> However, there is still [Rule2] to apply to .Z files.
> 
> This patch also adds a dependency between the first ouput file of a 
> rule and the other output files.
> For instance, with the same example as above, File1.Y and File1.Z are 
> generated by the following rule:
> File1.Y: File1.X
>     <Generate File1.Y>
>     <Generate File1.Z>
> 
> and the new dependency is:
> File1.Z: File1.Y
> 
> This is necessary to keep a dependency order during the execution of 
> the Makefile. Indeed, .Y and .Z files are generated by the execution 
> of a common set of commands, and without this rule, there is no 
> explicit dependency relation between them.
> 
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> 
> The changes can be seen at
> https://github.com/PierreARM/edk2/commits/pg/803_Compile_AML_bytec
> ode_array_into_OBJ_file_v2
> 
> Notes:
>     Notes:
>       v1:
>        - Generate multiple rules when multiple output files
>          are specified in the build_rule.txt file. [Pierre]
>       v2:
>        - Use the "FileType" variable in the _ApplyBuildRule
>          function as it is in the current state. [Pierre]
> 
>  BaseTools/Source/Python/AutoGen/GenMake.py       |  6 ++++
>  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 38
> +++++++++++---------
>  2 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
> b/BaseTools/Source/Python/AutoGen/GenMake.py
> index
> bbb3c29446f53fa7f2cb61a216a5b119f72c3fbc..0314d0ea34d99a014379e8d30c
> 46ac0f0a7068ce 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -1054,6 +1054,12 @@ cleanlib:
>                      TargetDict = {"target": 
> self.PlaceMacro(T.Target.Path, self.Macros),
> "cmd": "\n\t".join(T.Commands),"deps": Deps}
> 
> self.BuildTargetList.append(self._BUILD_TARGET_TEMPLATE.Replace(Target
> Dict))
> 
> +                    # Add a Makefile rule for targets generating multiple files.
> +                    # The main output is a prerequisite for the other output files.
> +                    for i in T.Outputs[1:]:
> +                        AnnexeTargetDict = {"target": 
> + self.PlaceMacro(i.Path,
> self.Macros), "cmd": "", "deps": self.PlaceMacro(T.Target.Path, 
> self.Macros)}
> +
> +
> self.BuildTargetList.append(self._BUILD_TARGET_TEMPLATE.Replace(Annex
> e
> + TargetDict))
> +
>      def ParserCCodeFile(self, T, Type, CmdSumDict, CmdTargetDict, 
> CmdCppDict, DependencyDict):
>          if not CmdSumDict:
>              for item in self._AutoGenObject.Targets[Type]:
> diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> index
> aad591de65f086043d55aeea5661f59c53792e7c..dc8b1fe3d160cac2da22227fc2
> 33e3aa0d92cb1e 100755
> --- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> @@ -860,7 +860,8 @@ class ModuleAutoGen(AutoGen):
>          SubDirectory = os.path.join(self.OutputDir, File.SubDir)
>          if not os.path.exists(SubDirectory):
>              CreateDirectory(SubDirectory)
> -        LastTarget = None
> +        TargetList = set()
> +        FinalTargetName = set()
>          RuleChain = set()
>          SourceList = [File]
>          Index = 0
> @@ -870,6 +871,9 @@ class ModuleAutoGen(AutoGen):
>          self.BuildOption
> 
>          while Index < len(SourceList):
> +            # Reset the FileType if not the first iteration.
> +            if Index > 0:
> +                FileType = TAB_UNKNOWN_FILE
>              Source = SourceList[Index]
>              Index = Index + 1
> 
> @@ -886,29 +890,25 @@ class ModuleAutoGen(AutoGen):
>              elif Source.Ext in self.BuildRules:
>                  RuleObject = self.BuildRules[Source.Ext]
>              else:
> -                # stop at no more rules
> -                if LastTarget:
> -                    self._FinalBuildTargetList.add(LastTarget)
> -                break
> +                # No more rule to apply: Source is a final target.
> +                FinalTargetName.add(Source)
> +                continue
> 
>              FileType = RuleObject.SourceFileType
>              self._FileTypes[FileType].add(Source)
> 
>              # stop at STATIC_LIBRARY for library
>              if self.IsLibrary and FileType == TAB_STATIC_LIBRARY:
> -                if LastTarget:
> -                    self._FinalBuildTargetList.add(LastTarget)
> -                break
> +                FinalTargetName.add(Source)
> +                continue
> 
>              Target = RuleObject.Apply(Source, self.BuildRuleOrder)
>              if not Target:
> -                if LastTarget:
> -                    self._FinalBuildTargetList.add(LastTarget)
> -                break
> -            elif not Target.Outputs:
> -                # Only do build for target with outputs
> -                self._FinalBuildTargetList.add(Target)
> +                # No Target: Source is a final target.
> +                FinalTargetName.add(Source)
> +                continue
> 
> +            TargetList.add(Target)
>              self._BuildTargets[FileType].add(Target)
> 
>              if not Source.IsBinary and Source == File:
> @@ -916,12 +916,16 @@ class ModuleAutoGen(AutoGen):
> 
>              # to avoid cyclic rule
>              if FileType in RuleChain:
> -                break
> +                EdkLogger.error("build", ERROR_STATEMENT, "Cyclic 
> + dependency detected while generating rule for %s" % str(Source))
> 
>              RuleChain.add(FileType)
>              SourceList.extend(Target.Outputs)
> -            LastTarget = Target
> -            FileType = TAB_UNKNOWN_FILE
> +
> +        # For each final target name, retrieve the corresponding
> TargetDescBlock instance.
> +        for FTargetName in FinalTargetName:
> +            for Target in TargetList:
> +                if FTargetName == Target.Target:
> +                    self._FinalBuildTargetList.add(Target)
> 
>      @cached_property
>      def Targets(self):
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
>