Re: [PATCH] BaseTools: Fix build failure when specifying multiple BUILDTARGET

[Laszlo Ersek <lersek@redhat.com>]

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 <leif.lindholm@linaro.org>
> Fixes: f8d11e5a4aaa90bf63b4789f3993dd6d16c60787
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
> Tested-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  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 <lersek@redhat.com>

Thanks
Laszlo