From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web12.12797.1603799877306439242 for ; Tue, 27 Oct 2020 04:57:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=aiNpTikZ; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603799876; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wlReOPxIULsgsuvWaKAbjzno3zcyA/uCI5f9dbmwOqM=; b=aiNpTikZ8tExjJim3JaJjBu37qJryJfUNNY6VUMXtX4WwTiC2GXDE/0keoxJSn9x9ips6r Hx0srOWAIn3iST3FtrNhkpsD+qm9WHG8r0AyLqvNuQMvZeIqwLLqdSOnpXpp1iOuMJeM9e 0LQhnIu0LR0PLqKRkr28rkQPi89g/jQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-364-0c2cydFkOlulmDNQYYoJGQ-1; Tue, 27 Oct 2020 07:57:52 -0400 X-MC-Unique: 0c2cydFkOlulmDNQYYoJGQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A10431084CA7; Tue, 27 Oct 2020 11:57:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-132.ams2.redhat.com [10.36.114.132]) by smtp.corp.redhat.com (Postfix) with ESMTP id EEC8055774; Tue, 27 Oct 2020 11:57:48 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] BaseTools: Limit command line length. To: devel@edk2.groups.io, mingyuex.liang@intel.com Cc: Bob Feng , Liming Gao , Yuwei Chen , "Ard Biesheuvel (ARM address)" References: <20201023030043.1047-1-mingyuex.liang@intel.com> From: "Laszlo Ersek" Message-ID: <7466e3f1-6190-cf5c-3cf6-cec369dbb3a9@redhat.com> Date: Tue, 27 Oct 2020 12:57:47 +0100 MIME-Version: 1.0 In-Reply-To: <20201023030043.1047-1-mingyuex.liang@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit (+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 > Cc: Bob Feng > Cc: Liming Gao > Cc: Yuwei Chen > --- > 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: >