Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04

["Leif Lindholm" <quic_llindhol@quicinc.com>]

Hi Bob,

You are suggesting to knowingly break builds between one stable tag and 
another on a technicality, for one of the most common (and up to date) 
build operating systems.

The purpose of the stable tag is to avoid unplanned breakages. Catching 
this sort of situation is literally what it is for.

/
     Leif

On 2022-08-24 18:17, Feng, Bob C wrote:
> Hi Liming,
> 
> This commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 was to fix the bug that the basetools implementation dose not follow the dsc spec and pass the edk2 CI.
> The patch was send on June 30, reviewed on July 11 and pushed on July 17. I think there was enough time to give feedback to this change.
> 
> Why not consider to fix the DSC file?
> 
> Thanks,
> Bob
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming via groups.io
> Sent: Wednesday, August 24, 2022 3:16 PM
> To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io; 'Ard Biesheuvel' <ardb@kernel.org>; rebecca@bsdio.com; Chen, Christine <yuwei.chen@intel.com>
> Cc: 'Andrew Fish' <afish@apple.com>; 'Leif Lindholm' <quic_llindhol@quicinc.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: 回复: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
> 
> Bob:
>   The commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 was merged into Edk2 on Last month (2022 July 17th). This is very new change. It brings the behavior change. But, it doesn't highlight its impact to collect feedback.
> 
>   I suggest to revert it, because I think we need more time to discuss this behavior change. Even if we decide to make this change, we also need to give other people enough time to update DSC file.
> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: Feng, Bob C <bob.c.feng@intel.com>
>> 发送时间: 2022年8月23日 15:40
>> 收件人: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>;
>> 'Ard Biesheuvel' <ardb@kernel.org>; rebecca@bsdio.com; Chen, Christine
>> <yuwei.chen@intel.com>
>> 抄送: 'Andrew Fish' <afish@apple.com>; 'Leif Lindholm'
>> <quic_llindhol@quicinc.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
>> 主题: RE: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu
>> 22.04
>>
>> Hi Liming,
>>
>> Reverting patch may not a good idea, some platforms have done the
>> implementation based on the DSC spec, if revert, those platforms build
>> will break.
>> This commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 make the
>> Basetools behavior be consistent with DSC spec so I don't think it's a
>> regression bug.
>>
>> Thanks,
>> Bob
>>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>> gaoliming via groups.io
>> Sent: Tuesday, August 23, 2022 9:55 AM
>> To: 'Ard Biesheuvel' <ardb@kernel.org>; devel@edk2.groups.io;
>> rebecca@bsdio.com; Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
>> <yuwei.chen@intel.com>
>> Cc: 'Andrew Fish' <afish@apple.com>; 'Leif Lindholm'
>> <quic_llindhol@quicinc.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
>> Subject: 回复: [edk2-devel] MdeModulePkg build fails for AARCH64 on
>> Ubuntu 22.04
>> Importance: High
>>
>> Yuwei:
>>    The commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 (BaseTools:
>> Fix DSC LibraryClass precedence rule) introduces this issue. I agree
>> with Ard suggestion to revert this change first for this stable tag
>> 202208. Then, we can discuss this change in the detail.
>>
>> Thanks
>> Liming
>>> -----邮件原件-----
>>> 发件人: Ard Biesheuvel <ardb@kernel.org>
>>> 发送时间: 2022年8月22日 17:34
>>> 收件人: devel@edk2.groups.io; rebecca@bsdio.com; Feng, Bob C
>>> <bob.c.feng@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
>>> 抄送: Andrew Fish <afish@apple.com>; Leif Lindholm
>>> <quic_llindhol@quicinc.com>; Michael D Kinney
>>> <michael.d.kinney@intel.com>; Jian J Wang <jian.j.wang@intel.com>
>>> 主题: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu
>>> 22.04
>>>
>>> On Mon, 22 Aug 2022 at 11:30, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Mon, 22 Aug 2022 at 11:11, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>>
>>>>> (cc Bob)
>>>>>
>>>>> NOTE this affects the stable tag - please see below
>>>>>
>>>>>
>>>>> On Sun, 21 Aug 2022 at 06:34, Rebecca Cran <rebecca@bsdio.com>
>>> wrote:
>>>>>>
>>>>>> I noticed that MdeModulePkg fails to build on my Ubuntu 22.04
>>>>>> system (and previously on the Ubuntu 20.04 installation). Is
>>>>>> this a
>> known bug?
>>>>>>
>>>>>> It shouldn't be relevant, but I'm using gcc version 11.2.0
>>>>>> (Ubuntu 11.2.0-17ubuntu1). But perhaps more relevant, I'm
>>>>>> using
>> Python 3.10.4.
>>>>>>
>>>>>> I'm trying to build commit
>>> e2ac68a23b4954d5c0399913a1df3dd9fd90315d.
>>>>>>
>>>>>>
>>>>>> bcran@photon:~/src/tmp/edk2$ build -p
>>> MdeModulePkg/MdeModulePkg.dsc -a
>>>>>> AARCH64 -t GCC5 -b RELEASE
>>>>>> Build environment:
>>>>>> Linux-5.15.0-46-generic-x86_64-with-glibc2.35
>>>>>> Build start time: 22:29:18, Aug.20 2022
>>>>>>
>>>>>> WORKSPACE        = /home/bcran/src/tmp/edk2
>>>>>> EDK_TOOLS_PATH   = /home/bcran/src/tmp/edk2/BaseTools
>>>>>> CONF_PATH        = /home/bcran/src/tmp/edk2/Conf
>>>>>> PYTHON_COMMAND   = /usr/bin/python3
>>>>>>
>>>>>>
>>>>>> Processing meta-data
>>>>>> .Architecture(s)  = AARCH64
>>>>>> Build target     = RELEASE
>>>>>> Toolchain        = GCC5
>>>>>>
>>>>>> Active Platform          =
>>>>>> /home/bcran/src/tmp/edk2/MdeModulePkg/MdeModulePkg.dsc
>>>>>>
>>>>>>
>>>>>> build.py...
>>>>>>
>>>
>> /home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmL
>>> ockBoxPeiLib.inf(42):
>>>>>> error 3000: PCD
>>>>>> [gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode] in
>>>>>>
>>>
>> [/home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmL
>>> ockBoxPeiLib.inf]
>>>>>> is not found in dependent packages:
>>>>>>
>> /home/bcran/src/tmp/edk2/MdePkg/MdePkg.dec
>>>>>>
>>> /home/bcran/src/tmp/edk2/MdeModulePkg/MdeModulePkg.dec
>>>>>>
>>>>>>
>>>>>
>>>>> This looks like a BaseTools regression to me - afaik, these
>>>>> packages all used to build for AARCH64 in the past.
>>>>>
>>>>> The following LockBoxLib resolutions appear in MdeModulePkg.dsc
>>>>> (with line numbers)
>>>>>
>>>>> 115-[LibraryClasses.common.PEIM]
>>>>> 119:
>>>
>> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
>>>>>
>>>>> 178-[LibraryClasses.ARM, LibraryClasses.AARCH64]
>>>>> 181:
>>> LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>>>>>
>>>>> No other [LlibraryClasses] references to SmmLockBoxPeiLib.inf
>>>>> exist in that file, and the [Components] reference does not
>>>>> apply to
>> AARCH64.
>>>>>
>>>>> This means the DSC parser ends up using the earlier definition,
>>>>> which I think is a bug, or at least a change in behavior. Note
>>>>> that doing this silently could potentially break many platforms
>>>>> in subtle ways so I think this should be root caused and
>>>>> preferably fixed before creating the stable tag.
>>>>
>>>> Seems to be deliberate:
>>>>
>>>> commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860
>>>> Author: Chen, Christine <Yuwei.Chen@intel.com>
>>>> Date:   Thu Jun 30 17:04:05 2022 +0800
>>>>
>>>>      BaseTools: Fix DSC LibraryClass precedence rule
>>>>
>>>> but I don't think the impact on other platforms and drivers has
>>>> been taken into account here. Also, the document [0] seems ambiguous to me:
>>>>
>>>> """
>>>> The first globally defined library instance, defined in a DSC
>>>> file, that satisfies a module's requirement for a Library Class,
>>>> unless specifically overridden by the module in the [Components]
>>>> section, will be used.
>>>> """
>>>>
>>>> This says that the first occurrence of a library instance
>>>> definition that satisfies a INF dependency will be selected. Then,
>>>> there is
>>>>
>>>> """
>>>> The Library Instances will be selected using the following rules
>>>> to satisfy a library class for each module listed in the
>>>> [Components] section (in order of highest precedence):
>>>> """
>>>>
>>>> which says that not the *first* occurrence, but the occurrence
>>>> *with the highest precedence* will be selected.
>>>>
>>>> Then, the precedence list has these items:
>>>>
>>>> """
>>>> 2. [LibraryClasses.$(Arch).$(MODULE_TYPE),
>>>> LibraryClasses.$(Arch).$(MODULE_TYPE)]
>>>> 3. [LibraryClasses.$(Arch).$(MODULE_TYPE)]
>>>> """
>>>>
>>>> which seem to suggest that a definition that applies to multiple
>>>> arch/module_type combinations automatically takes precedence over
>>>> one that targets a single arch/module_type combination, which
>>>> makes no sense at all.
>>>>
>>>> So I strongly suggest we revert the patch, fix the document so it
>>>> is clear and ambiguous, and preferably, align it with how it used
>>>> to
>>>
>>> *un*ambiguous ....
>>>
>>> sigh
>>
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 
> 
>