From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8C7A821962301 for ; Tue, 5 Feb 2019 04:05:03 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EA7007F3F4; Tue, 5 Feb 2019 12:05:02 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-79.rdu2.redhat.com [10.10.120.79]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6856217AB7; Tue, 5 Feb 2019 12:04:56 +0000 (UTC) To: Leif Lindholm Cc: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , edk2-devel@lists.01.org, Liming Gao , Bob Feng , "Zhiju . Fan" , Yonghong Zhu , Ard Biesheuvel , Michael Kinney , Andrew Fish References: <20190205012356.31376-1-philmd@redhat.com> <20190205114701.rzhrwb75uecwswjl@bivouac.eciton.net> From: Laszlo Ersek Message-ID: <1f75d314-4853-e405-dd59-f12901803f66@redhat.com> Date: Tue, 5 Feb 2019 13:04:56 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190205114701.rzhrwb75uecwswjl@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 05 Feb 2019 12:05:03 +0000 (UTC) Subject: Re: [PATCH] BaseTools: Fix build failure when specifying multiple BUILDTARGET X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Feb 2019 12:05:03 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit (+Mike, +Andrew) On 02/05/19 12:47, Leif Lindholm wrote: > On Tue, Feb 05, 2019 at 10:03:21AM +0100, Laszlo Ersek wrote: >> On 02/05/19 02:23, Philippe Mathieu-Daudé wrote: >>> Since 9c2d68c0a299 the build tools default to the python version >>> provided by the ${PYTHON} environment variable. >>> However the Python3 transition is not effective before d943b0c339fe. >> >> (1) Do you mean "functional" rather than "effective"? >> >> (2) Why is this information relevant for this commit? I see that commit >> f8d11e5a4aaa, referenced below, falls between the above two, but I'm >> unsure if that has any special relevance. >> >> If the above paragraph is just background info, that's OK with me, of >> course. >> >>> With Python3, the dict.value() method returns an iterator. >>> If a dictionary is updated while an iterator on his keys is used, >> >> (3) s/his/its/ >> >>> a RuntimeError is generated. >>> Converting the iterator to a list() forces a copy of the mutable >>> keys in an immutable list which can be safely iterated. >>> >>> Commit f8d11e5a4aaa converted various uses but missed one: >>> When specifying multiple BUILDTARGET, the first target builds >>> successfully, but then the PGen.BuildDatabase._CACHE_ dictionary is >>> updated, and the next target accessing it triggers a RuntimeError. >> >> (4) Can we clarify this please; I think it's not the "next target" that >> accesses the dictionary, instead the code accesses the next target in >> the dictionary. How about >> >> s/the next target accessing it/accessing the next target/ >> >> ? >> >>> >>> Convert this iterator to an immutable list, to solve this build error: >>> >>> $ build -a IA32 -t GCC5 -b RELEASE -b NOOPT -p OvmfPkg/OvmfPkgIa32.dsc >>> [...] >>> Processing meta-data ... >>> build.py... >>> : error C0DE: Unknown fatal error when processing [OvmfPkg/OvmfPkgIa32.dsc] >>> >>> (Please send email to edk2-devel@lists.01.org for help, attaching following call stack trace!) >>> >>> (Python 3.5.3 on linux) Traceback (most recent call last): >>> File "BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py", line 2387, in Main >>> MyBuild.Launch() >>> File "BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py", line 2141, in Launch >>> self._MultiThreadBuildPlatform() >>> File "BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py", line 1921, in _MultiThreadBuildPlatform >>> self.Progress >>> File "BaseTools/Source/Python/AutoGen/AutoGen.py", line 304, in __init__ >>> self._InitWorker(Workspace, MetaFile, Target, Toolchain, Arch, *args, **kwargs) >>> File "BaseTools/Source/Python/AutoGen/AutoGen.py", line 477, in _InitWorker >>> for BuildData in PGen.BuildDatabase._CACHE_.values(): >>> RuntimeError: dictionary changed size during iteration >>> >>> Reported-by: Leif Lindholm >>> Fixes: f8d11e5a4aaa90bf63b4789f3993dd6d16c60787 >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Philippe Mathieu-Daude >>> Tested-by: Leif Lindholm >>> --- >>> BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py >>> index a95d2c710e..12592a2a46 100644 >>> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py >>> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py >>> @@ -474,7 +474,7 @@ class WorkspaceAutoGen(AutoGen): >>> >>> # generate the SourcePcdDict and BinaryPcdDict >>> PGen = PlatformAutoGen(self, self.MetaFile, Target, Toolchain, Arch) >>> - for BuildData in PGen.BuildDatabase._CACHE_.values(): >>> + for BuildData in list(PGen.BuildDatabase._CACHE_.values()): >>> if BuildData.Arch != Arch: >>> continue >>> if BuildData.MetaFile.Ext == '.inf': >>> >> >> LGTM :) >> >> With the commit message updated (as you prefer): >> >> Acked-by: Laszlo Ersek > > I would be surprised if we hear back from the BaseTools maintainers > this week (Chinese New Year). For me, I have both a simple workaround > and this patch, so I'm OK with waiting. > > But if anyone reports issues with CI environments or similar, I would > say either you or I could push this patch in their absence. Right; see also the patch I just posted: [edk2] [PATCH] BaseTools/BuildReport: fix report for platforms/arches without struct PCDs It's another build breakage, although less intrusive (it only occurs if you ask for a report file). I think the last agreement on the list was that fixes for build breakages could be pushed after 24 hours of posting, regardless of Maintainer review, if the patches were "sufficiently reviewed otherwise". I suggest we push the (upcoming v2) patch tomorrow. Thanks Laszlo