I didn't check with Bob about that patch. I guess that was to fix a bug I found in BaseTools which didn't take care of the library override properly. Or the rule of the library override was in a chaos that didn't seem to be an understood rule.
If some platforms happen to rely on this rule, I agree this is a risk. So, I think a better solution is to evaluate the library instance mapping before and after the patch. Thanks, Ray > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D > Kinney > Sent: Thursday, August 25, 2022 11:44 PM > To: devel@edk2.groups.io; quic_llind...@quicinc.com; Feng, Bob C > <bob.c.f...@intel.com>; Gao, Liming > <gaolim...@byosoft.com.cn>; 'Ard Biesheuvel' <a...@kernel.org>; > rebe...@bsdio.com; Chen, Christine > <yuwei.c...@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com> > Cc: 'Andrew Fish' <af...@apple.com>; Wang, Jian J <jian.j.w...@intel.com> > Subject: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04 > > Hi Bob, > > There was feedback in the Bugzilla on July 5 asking if this is > a DSC spec issue or a BaseTools implementation issue. There was > no response to that question. > > Why was a spec change not considered for this case to match the > BaseTools behavior? > > Why were no additional details added to the BZ? > > I agree with Leif. If this change introduced regressions, we should > consider a revert and evaluate again after the release. > > Thanks, > > Mike > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm > > Sent: Thursday, August 25, 2022 1:49 AM > > To: Feng, Bob C <bob.c.f...@intel.com>; devel@edk2.groups.io; Gao, Liming > > <gaolim...@byosoft.com.cn>; 'Ard > Biesheuvel' > > <a...@kernel.org>; rebe...@bsdio.com; Chen, Christine <yuwei.c...@intel.com> > > Cc: 'Andrew Fish' <af...@apple.com>; Kinney, Michael D > > <michael.d.kin...@intel.com>; Wang, Jian J > <jian.j.w...@intel.com> > > Subject: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu > > 22.04 > > > > 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.f...@intel.com>; devel@edk2.groups.io; 'Ard > > > Biesheuvel' <a...@kernel.org>; > rebe...@bsdio.com; Chen, > > Christine <yuwei.c...@intel.com> > > > Cc: 'Andrew Fish' <af...@apple.com>; 'Leif Lindholm' > > > <quic_llind...@quicinc.com>; Kinney, Michael D > > <michael.d.kin...@intel.com>; Wang, Jian J <jian.j.w...@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.f...@intel.com> > > >> 发送时间: 2022年8月23日 15:40 > > >> 收件人: devel@edk2.groups.io; Gao, Liming <gaolim...@byosoft.com.cn>; > > >> 'Ard Biesheuvel' <a...@kernel.org>; rebe...@bsdio.com; Chen, Christine > > >> <yuwei.c...@intel.com> > > >> 抄送: 'Andrew Fish' <af...@apple.com>; 'Leif Lindholm' > > >> <quic_llind...@quicinc.com>; Kinney, Michael D > > >> <michael.d.kin...@intel.com>; Wang, Jian J <jian.j.w...@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' <a...@kernel.org>; devel@edk2.groups.io; > > >> rebe...@bsdio.com; Feng, Bob C <bob.c.f...@intel.com>; Chen, Christine > > >> <yuwei.c...@intel.com> > > >> Cc: 'Andrew Fish' <af...@apple.com>; 'Leif Lindholm' > > >> <quic_llind...@quicinc.com>; Kinney, Michael D > > >> <michael.d.kin...@intel.com>; Wang, Jian J <jian.j.w...@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 <a...@kernel.org> > > >>> 发送时间: 2022年8月22日 17:34 > > >>> 收件人: devel@edk2.groups.io; rebe...@bsdio.com; Feng, Bob C > > >>> <bob.c.f...@intel.com>; Liming Gao <gaolim...@byosoft.com.cn> > > >>> 抄送: Andrew Fish <af...@apple.com>; Leif Lindholm > > >>> <quic_llind...@quicinc.com>; Michael D Kinney > > >>> <michael.d.kin...@intel.com>; Jian J Wang <jian.j.w...@intel.com> > > >>> 主题: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu > > >>> 22.04 > > >>> > > >>> On Mon, 22 Aug 2022 at 11:30, Ard Biesheuvel <a...@kernel.org> wrote: > > >>>> > > >>>> On Mon, 22 Aug 2022 at 11:11, Ard Biesheuvel <a...@kernel.org> wrote: > > >>>>> > > >>>>> (cc Bob) > > >>>>> > > >>>>> NOTE this affects the stable tag - please see below > > >>>>> > > >>>>> > > >>>>> On Sun, 21 Aug 2022 at 06:34, Rebecca Cran <rebe...@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.c...@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 > > >> > > >> > > >> > > >> > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92824): https://edk2.groups.io/g/devel/message/92824 Mute This Topic: https://groups.io/mt/93231667/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
