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 (#92802): https://edk2.groups.io/g/devel/message/92802
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]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to