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

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

[Philippe Mathieu-Daudé]

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.

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

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':
-- 
2.20.1

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

[Laszlo Ersek]

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

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

[Leif Lindholm]

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 <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>

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.

/
    Leif

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

[Philippe Mathieu-Daudé]

On 2/5/19 10:03 AM, 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"?

Probably :)

> (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.

Yes, I'll reword to explain the tree isn't bisectable there, or drop
that background info.

>> 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/
> 
> ?

Sure.

>>
>> 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!

Regards,

Phil.

> 
> Thanks
> Laszlo
> 

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

[Laszlo Ersek]

(+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 <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>
> 
> 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

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

[Laszlo Ersek]

On 02/05/19 13:02, Philippe Mathieu-Daudé wrote:
> On 2/5/19 10:03 AM, 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"?
> 
> Probably :)
> 
>> (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.
> 
> Yes, I'll reword to explain the tree isn't bisectable there, or drop
> that background info.

Ah, great point; so please do keep the paragraph, just point out that
the culprit commit (f8d11e5a4aaa) couldn't be found with bisection,
because between A and B the history isn't bisectable.

Thanks!
Laszlo

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

[Carsey, Jaben]

Laszlo,

not sure which Andrew you wanted, but he didn’t get added so far as I can tell.

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Tuesday, February 05, 2019 4:05 AM
> To: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2] [PATCH] BaseTools: Fix build failure when specifying
> multiple BUILDTARGET
> 
> (+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 <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>
> >
> > 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.

Agreed.  I can push if desired, once final version is complete.

> 
> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Laszlo Ersek]

On 02/05/19 22:20, Carsey, Jaben wrote:
> Laszlo,
> 
> not sure which Andrew you wanted, but he didn’t get added so far as I can tell.

He did, it's just the mailman2 list software pulling tricks on us again.

The default setting for list subscribers is to eliminate duplicates.
That is, if you are subscribed to the list, and someone send an email to
both the list and you personally, then mailman2 will see that you are
already CC'd on the original, so it will not deliver a 2nd copy to you.
The trick is that mailman2 will also strip your address from the copy
that it delivers to *other* list subscribers. Those other subscribers
won't be able to tell whether the original sender CC'd you or not.

If you manually invert this setting at
<https://lists.01.org/mailman/options/edk2-devel> (it's called "enable
duplicates" or something similar), then you will get two copies of the
original email, one directly, and another from the list. (This is
generally the more useful setting, because you can file the reflected
copy in your list folder, and keep the direct email in your inbox.) The
trick is that in this case mailman2 will *not* strip your address from
the copy that it reflects to the other subscribers.

So now you can tell that Mike has duplicates enabled, and Andrew has
them disabled. :)

>> I suggest we push the (upcoming v2) patch tomorrow.
> 
> Agreed.  I can push if desired, once final version is complete.

Thanks for that!
Laszlo