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

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

[Philippe Mathieu-Daudé]

With Python3, the dict.value() method returns an iterator.
If a dictionary is updated while an iterator on its keys is used,
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 accessing the next target triggers a RuntimeError.

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

Note: The culprit commit (f8d11e5a4aaa) can not be found with bisection.
In 9c2d68c0a299 the build tools default to the python version provided
by the ${PYTHON} environment variable, however the Python3 transition is
not functional before d943b0c339fe. f8d11e5a4aaa falls between the
previous two.

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>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
v2:
 - fixed English errors (Laszlo)
 - the paragraph about bisection not working is not relevant to
   the fix, keep it as background info but move it after (Laszlo)
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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':
-- 
2.20.1

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

[Philippe Mathieu-Daudé]

On 2/6/19 1:03 PM, Philippe Mathieu-Daudé wrote:
[...]
> 
> 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>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> ---
> v2:
>  - fixed English errors (Laszlo)
>  - the paragraph about bisection not working is not relevant to
>    the fix, keep it as background info but move it after (Laszlo)
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

^ Oops I notice the tool I'm using (git publish) added this S-o-b line,
due to an incomplete config.

Anyway this is in part of the patch stripped out when applied, so no
need to resend a v3 for this detail.

> ---

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

[Laszlo Ersek]

On 02/06/19 13:03, Philippe Mathieu-Daudé wrote:
> With Python3, the dict.value() method returns an iterator.
> If a dictionary is updated while an iterator on its keys is used,
> 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 accessing the next target triggers a RuntimeError.
> 
> 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
> 
> Note: The culprit commit (f8d11e5a4aaa) can not be found with bisection.
> In 9c2d68c0a299 the build tools default to the python version provided
> by the ${PYTHON} environment variable, however the Python3 transition is
> not functional before d943b0c339fe. f8d11e5a4aaa falls between the
> previous two.
> 
> 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>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> ---
> v2:
>  - fixed English errors (Laszlo)
>  - the paragraph about bisection not working is not relevant to
>    the fix, keep it as background info but move it after (Laszlo)
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  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':
> 

Looks nice, thanks! My A-b stands.

Laszlo

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

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

And pushed.

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, February 06, 2019 8:26 AM
> To: Philippe Mathieu-Daudé <philmd@redhat.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Carsey, Jaben
> <jaben.carsey@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Subject: Re: [PATCH v2] BaseTools: Fix build failure when specifying multiple
> BUILDTARGET
> 
> On 02/06/19 13:03, Philippe Mathieu-Daudé wrote:
> > With Python3, the dict.value() method returns an iterator.
> > If a dictionary is updated while an iterator on its keys is used,
> > 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 accessing the next target triggers a RuntimeError.
> >
> > 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
> >
> > Note: The culprit commit (f8d11e5a4aaa) can not be found with bisection.
> > In 9c2d68c0a299 the build tools default to the python version provided
> > by the ${PYTHON} environment variable, however the Python3 transition is
> > not functional before d943b0c339fe. f8d11e5a4aaa falls between the
> > previous two.
> >
> > 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>
> > Acked-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> > v2:
> >  - fixed English errors (Laszlo)
> >  - the paragraph about bisection not working is not relevant to
> >    the fix, keep it as background info but move it after (Laszlo)
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >  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':
> >
> 
> Looks nice, thanks! My A-b stands.
> 
> Laszlo

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

[Philippe Mathieu-Daudé]

On 2/6/19 10:11 PM, Carsey, Jaben wrote:
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> 
> And pushed.

Thanks!

And I learned the hard way git config 'user.name' is obviously different
than 'sendemail.from'...
Liming asked me to not use non-ASCII character in commit message so I
fixed my user.name, but forgot the sendemail.from, and now I see my
lastname got mojibaked :S
No worry, my bad.

Regards,

Phil.

>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, February 06, 2019 8:26 AM
>> To: Philippe Mathieu-Daudé <philmd@redhat.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Carsey, Jaben
>> <jaben.carsey@intel.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Subject: Re: [PATCH v2] BaseTools: Fix build failure when specifying multiple
>> BUILDTARGET
>>
>> On 02/06/19 13:03, Philippe Mathieu-Daudé wrote:
>>> With Python3, the dict.value() method returns an iterator.
>>> If a dictionary is updated while an iterator on its keys is used,
>>> 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 accessing the next target triggers a RuntimeError.
>>>
>>> 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
>>>
>>> Note: The culprit commit (f8d11e5a4aaa) can not be found with bisection.
>>> In 9c2d68c0a299 the build tools default to the python version provided
>>> by the ${PYTHON} environment variable, however the Python3 transition is
>>> not functional before d943b0c339fe. f8d11e5a4aaa falls between the
>>> previous two.
>>>
>>> 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>
>>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>> v2:
>>>  - fixed English errors (Laszlo)
>>>  - the paragraph about bisection not working is not relevant to
>>>    the fix, keep it as background info but move it after (Laszlo)
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  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':
>>>
>>
>> Looks nice, thanks! My A-b stands.
>>
>> Laszlo