On 12/14/18 12:22, Ard Biesheuvel wrote:
> Commit 9a67ba261fe9 ("ArmVirtPkg: Replace obsoleted network drivers
> from platform DSC/FDF") failed to take into account that the now
> unconditionally included IScsiDxe.inf from NetworkPkg requires a
> resolution for TcpIoLib.I don't understand why this happened. (a) I warned *precisely* about this issue, when I reviewed the v2 version of said commit. See bullet (5) in the following message: [email protected]">http://mid.mail-archive.com/[email protected] (b) What's more, my comments for the v3 version were summarily ignored as well. See bullet (2) in: [email protected]">http://mid.mail-archive.com/[email protected] And now, the BaseCryptLib, OpensslLib and IntrinsicLib resolutions for [LibraryClasses.common.UEFI_DRIVER] have been added to "ArmVirtPkg/ArmVirtQemuKernel.dsc", despite their being redundant and my having pointed out that fact. Worse, "ArmVirtQemuKernel.dsc" uses "OpensslLib.inf", while "OpensslLibCrypto.inf" from "ArmVirt.dsc.inc" is perfectly sufficient. Commit 9a67ba261fe9 does not carry my R-b, and that's not a random fact. The v3 patch was *not* ready for being pushed, to my eyes. And I was pretty explicit about that. > Since specifying such a resolution is harmless > for platforms that have no networking enabled, let's just fix things > by dropping the conditionals around it. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > ArmVirtPkg/ArmVirt.dsc.inc | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > index c3549c84d4c6..89c2db074711 100644 > --- a/ArmVirtPkg/ArmVirt.dsc.inc > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > @@ -80,9 +80,7 @@ > DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf > UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf > IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf > -!if $(NETWORK_IP6_ENABLE) == TRUE > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf > -!endif > !if $(HTTP_BOOT_ENABLE) == TRUE > HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf > !endif > I'm *very strongly* tempted to simply revert 9a67ba261fe9, for blatantly ignoring my explicit requests for updates. However, that would only result in my having to review more (possibly incomplete) iterations of the patch. At least, this incremental fix is in line with my request in (a) -- "we should make the current TcpIoLib class resolution unconditional". Please go ahead and push it. Reviewed-by: Laszlo Ersek <[email protected]> I should really file a TianoCore BZ about the wrong / redundant OpensslLib resolution in ArmVirtQemuKernel.dsc too. It's difficult for me to find te motivation for that right now, seeing the disregard for my earlier reviews. Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
