Re: [edk2-devel] [PATCH] BaseTools: Limit command line length.

["Laszlo Ersek" <lersek@redhat.com>]

(+Ard)

On 10/23/20 05:00, mliang2x wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2528
> 
> Currently, the command line is too long because the CL
> command is followed by multiple C files.
> 
> Therefore, the number of C files
> can be used to determine whether the command line
> needs to be written to the file. If the number of
> C files is greater than one, the command line is
> directly written to the file. On the contrary,
> whether to write to the file is determined by whether
> the length of the command line exceeds the limited
> length Documents.
> 
> Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> ---
>  BaseTools/Source/Python/AutoGen/GenMake.py    | 45 +++++++++++++++----
>  .../Source/Python/AutoGen/IncludesAutoGen.py  | 13 +++++-
>  2 files changed, 49 insertions(+), 9 deletions(-)

(1) I've read both BZ#2528 (up to comment 13), and the above commit
message too.

I still don't have the slightest idea what the problem is. Please clarify.

(2) How do this patch (and this issue) relate to the "--cmd-len" option?


The bugzilla ticket says the issue is related to MSVC, and that it was
exposed by fixing BZ#1672 ("Enable multiple thread for MSVC compiler").
But, at least superficially, the diffstat and the patch body seem to be
more generic. What I really care about is that the gcc command lines
should not change. It's annoying to look at a build log and see
references to "cc_resp" text files, rather than the actual command
lines. My build scripts use "--cmd-len=65536" for that reason -- is this
patch going to keep that functional?

Thanks,
Laszlo

> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
> index 0314d0ea34..0cb97dc18d 100755
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -576,7 +576,8 @@ cleanlib:
>              EdkLogger.error("build", AUTOGEN_ERROR, "Nothing to build",
>                              ExtraData="[%s]" % str(MyAgo))
>  
> -        self.ProcessBuildTargetList()
> +        self.ProcessBuildTargetList(MyAgo.OutputDir,ToolsDef)
> +
>          self.ParserGenerateFfsCmd()
>  
>          # Generate macros used to represent input files
> @@ -866,7 +867,6 @@ cleanlib:
>                                          else:
>                                              break
>                                      SingleCommandLength += len(Str)
> -
>                              if SingleCommandLength > GlobalData.gCommandMaxLength:
>                                  FlagDict[Tool]['Value'] = True
>  
> @@ -890,18 +890,18 @@ cleanlib:
>                                      break
>                              else:
>                                  break
> -
>                          if self._AutoGenObject.ToolChainFamily == 'GCC':
>                              RespDict[Key] = Value.replace('\\', '/')
>                          else:
>                              RespDict[Key] = Value
> +
>                          for Target in BuildTargets:
>                              for i, SingleCommand in enumerate(BuildTargets[Target].Commands):
>                                  if FlagDict[Flag]['Macro'] in SingleCommand:
>                                      BuildTargets[Target].Commands[i] = SingleCommand.replace('$(INC)', '').replace(FlagDict[Flag]['Macro'], RespMacro)
>          return RespDict
>  
> -    def ProcessBuildTargetList(self):
> +    def ProcessBuildTargetList(self, RespFile, ToolsDef):
>          #
>          # Search dependency file list for each source file
>          #
> @@ -1002,6 +1002,7 @@ cleanlib:
>                      self.ObjTargetDict[T.Target.SubDir] = set()
>                  self.ObjTargetDict[T.Target.SubDir].add(NewFile)
>          for Type in self._AutoGenObject.Targets:
> +            resp_file_number = 0
>              for T in self._AutoGenObject.Targets[Type]:
>                  # Generate related macros if needed
>                  if T.GenFileListMacro and T.FileListMacro not in self.FileListMacros:
> @@ -1043,7 +1044,8 @@ cleanlib:
>                          Deps.append("$(%s)" % T.ListFileMacro)
>  
>                  if self._AutoGenObject.BuildRuleFamily == TAB_COMPILER_MSFT and Type == TAB_C_CODE_FILE:
> -                    T, CmdTarget, CmdTargetDict, CmdCppDict = self.ParserCCodeFile(T, Type, CmdSumDict, CmdTargetDict, CmdCppDict, DependencyDict)
> +                    T, CmdTarget, CmdTargetDict, CmdCppDict = self.ParserCCodeFile(T, Type, CmdSumDict, CmdTargetDict, CmdCppDict, DependencyDict, RespFile, ToolsDef, resp_file_number)
> +                    resp_file_number += 1
>                      TargetDict = {"target": self.PlaceMacro(T.Target.Path, self.Macros), "cmd": "\n\t".join(T.Commands),"deps": CCodeDeps}
>                      CmdLine = self._BUILD_TARGET_TEMPLATE.Replace(TargetDict).rstrip().replace('\t$(OBJLIST', '$(OBJLIST')
>                      if T.Commands:
> @@ -1060,7 +1062,7 @@ cleanlib:
>                          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(AnnexeTargetDict))
>  
> -    def ParserCCodeFile(self, T, Type, CmdSumDict, CmdTargetDict, CmdCppDict, DependencyDict):
> +    def ParserCCodeFile(self, T, Type, CmdSumDict, CmdTargetDict, CmdCppDict, DependencyDict, RespFile, ToolsDef, resp_file_number):
>          if not CmdSumDict:
>              for item in self._AutoGenObject.Targets[Type]:
>                  CmdSumDict[item.Target.SubDir] = item.Target.BaseName
> @@ -1080,6 +1082,7 @@ cleanlib:
>                                  CmdCppDict[item.Target.SubDir].append(Path)
>          if T.Commands:
>              CommandList = T.Commands[:]
> +            SaveFilePath = os.path.join(RespFile, "cc_resp_%s.txt" % resp_file_number)
>              for Item in CommandList[:]:
>                  SingleCommandList = Item.split()
>                  if len(SingleCommandList) > 0 and self.CheckCCCmd(SingleCommandList):
> @@ -1087,19 +1090,45 @@ cleanlib:
>                          if Temp.startswith('/Fo'):
>                              CmdSign = '%s%s' % (Temp.rsplit(TAB_SLASH, 1)[0], TAB_SLASH)
>                              break
> -                    else: continue
> +                    else:
> +                        continue
>                      if CmdSign not in list(CmdTargetDict.keys()):
>                          CmdTargetDict[CmdSign] = Item.replace(Temp, CmdSign)
>                      else:
>                          CmdTargetDict[CmdSign] = "%s %s" % (CmdTargetDict[CmdSign], SingleCommandList[-1])
> +
>                      Index = CommandList.index(Item)
>                      CommandList.pop(Index)
>                      if SingleCommandList[-1].endswith("%s%s.c" % (TAB_SLASH, CmdSumDict[CmdSign[3:].rsplit(TAB_SLASH, 1)[0]])):
>                          Cpplist = CmdCppDict[T.Target.SubDir]
>                          Cpplist.insert(0, '$(OBJLIST_%d): ' % list(self.ObjTargetDict.keys()).index(T.Target.SubDir))
> -                        T.Commands[Index] = '%s\n\t%s' % (' \\\n\t'.join(Cpplist), CmdTargetDict[CmdSign])
> +                        cmdtargetlist = CmdTargetDict[CmdSign].split(" ")
> +                        # get Source files and Save resp file.
> +                        c_files = []
> +                        cmds = []
> +                        if cmdtargetlist:
> +                            for item in cmdtargetlist:
> +                                if item.startswith('$(') or item.startswith('/Fo') or item.startswith('"$('):
> +                                    cmds.append(item)
> +                                if item.endswith('.c'):
> +                                    c_files.append(item)
> +                            c_files.insert(0, " ")
> +                            if len(c_files) > 2:
> +                                SaveFileOnChange(SaveFilePath," ".join(c_files), False)
> +                                T.Commands[Index] = '%s\n\t%s $(cc_resp_%s)' % (' \\\n\t'.join(Cpplist), " ".join(cmds), resp_file_number)
> +                                ToolsDef.append("cc_resp_%s = @%s" % (resp_file_number, SaveFilePath))
> +
> +                            elif len(CmdTargetDict[CmdSign]) > GlobalData.gCommandMaxLength and len(c_files) <=2:
> +                                SaveFileOnChange(SaveFilePath, " ".join(c_files), False)
> +                                T.Commands[Index] = '%s\n\t%s $(cc_resp_%s)' % (' \\\n\t'.join(Cpplist), " ".join(cmds), resp_file_number)
> +                                ToolsDef.append("cc_resp_%s = @%s" % (resp_file_number, SaveFilePath))
> +
> +                            else:
> +                                T.Commands[Index] = '%s\n\t%s' % (' \\\n\t'.join(Cpplist), CmdTargetDict[CmdSign])
> +
>                      else:
>                          T.Commands.pop(Index)
> +
>          return T, CmdSumDict, CmdTargetDict, CmdCppDict
>  
>      def CheckCCCmd(self, CommandList):
> diff --git a/BaseTools/Source/Python/AutoGen/IncludesAutoGen.py b/BaseTools/Source/Python/AutoGen/IncludesAutoGen.py
> index 720d93395a..9f61d49b3a 100644
> --- a/BaseTools/Source/Python/AutoGen/IncludesAutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/IncludesAutoGen.py
> @@ -203,7 +203,18 @@ ${END}
>                              cc_options = line[len(cc_cmd)+2:].split()
>                          else:
>                              cc_options = line[len(cc_cmd):].split()
> -                        SourceFileAbsPathMap = {os.path.basename(item):item for item in cc_options if not item.startswith("/") and os.path.exists(item)}
> +
> +                        for item in cc_options:
> +                            if not item.startswith("/"):
> +                                # if item.startswith("@"):
> +                                if item.endswith(".txt") and item.startswith("@"):
> +                                    with open(item[1:], "r") as file:
> +                                        source_files = file.readlines()[0].split()
> +                                        SourceFileAbsPathMap = {os.path.basename(file):file for file in source_files if os.path.exists(file)}
> +                                else:
> +                                    if os.path.exists(item):
> +                                        SourceFileAbsPathMap.update({os.path.basename(item): item.strip()})
> +
>              if line in SourceFileAbsPathMap:
>                  current_source = line
>                  if current_source not in ModuleDepDict:
>