Hi Jiewen,

Updating "all the code that uses the old API to use the new API" is not always 
possible.
We can certainly update all the EDKII packages; however, it is trickier with 
the non-open source packages.
For example, the Reference Code we are getting from a chipset vendor is using 
CreatePopUp.
Certainly, if we mark CreatePopUp deprecated (as you have suggested), the 
chances are, reference code will get eventually updated not to use the function.
However, it might take a significant amount of time.
Moreover, a project integrators that create board firmware by combining 
open-source and close-source packages coming from multiple sources(from 
multiple vendors),
would have to solve this issue again and again; whereas with my proposal, the 
issue is solved by upgrading EDKII code base (no need to touch third party 
packages).

Having said that, I do understand your concerns with keeping the library in 
MdePkg.
How about finding a middle ground between your proposal and my proposal?
We can keep UiLib instance in MdePkg, but keep only a minimalistic barebones 
implementation there.
We can also have another, more advanced, UiLib instance in the MdeModulePkg.
This technique is already used in EDKII.
For example, MdePkg defines DebugLib library class and provides 
BaseDebugLibSerialPort instance.
MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the same 
library class.

Since "advanced" version of Popup is not currently available, there is no need 
to create new instance in MdeModulePkg.
However, once Eric is ready with his Popup-on-top-of-HII implementation, it can 
be added to a new UiLib instance in MdeModulePkg.
Also, if new functions are added to UiLib in the future, MdePkg UiLib instance 
can have a NULL implementation like many other libraries in the MdePkg.

What do you think?

Thanks
Felix

From: Yao, Jiewen [mailto:jiewen....@intel.com]
Sent: Wednesday, November 09, 2016 8:54 PM
To: Felix Poludov; edk2-devel@lists.01.org
Subject: RE: [PATCH 0/2] MdePkg: UiLib library

HI Poludov
It is a good idea to have a new UiLib.

Both Mike and I give the suggestion to "You could  define new lib and update 
all the code that uses the old API to use the new API."

IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
We have plan to resolve it, but since you already have proposal to add UiLib. I 
hope it can be resolved here.

There is no real need to put UiLib into MdePkg. MdePkg is designed to hold 
EFI/PI speciation or industry standard.
In this case, I believe MdeModulePkg is a better place.

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Wednesday, November 9, 2016 11:44 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> This series of two patches introduces new UiLib library.
> 1. New UiLib library class is added to MdePkg.
> The library is intended for functions that produce UI elements.
> The patch introduces two functions that produce a popup window:
> UiCreatePopUp and UiCreatePopUpVaList.
> 2. An instance of UiLib library class is added to MdePkg.
> Popup implementation is copied from UefiLib
> (CreatePopUp function in UefiLib/Console.c).
> 3. MdePkg/UefiLib library is updated to implement CreatePopUp
> by calling UiCreatePopUpVaList.
> 4. Platform DSC files are updated to add UiLib instance.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Felix Polyudov <fel...@ami.com>
>
> AppPkg/AppPkg.dsc                                  |   1 +
> ArmPkg/ArmPkg.dsc                                  |   1 +
> BeagleBoardPkg/BeagleBoardPkg.dsc                  |   1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc      |   1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc   |   1 +
> CryptoPkg/CryptoPkg.dsc                            |   1 +
> DuetPkg/DuetPkgIa32.dsc                            |   1 +
> DuetPkg/DuetPkgX64.dsc                             |   1 +
> EdkCompatibilityPkg/EdkCompatibilityPkg.dsc        |   1 +
> EmbeddedPkg/EmbeddedPkg.dsc                        |   1 +
> EmulatorPkg/EmulatorPkg.dsc                        |   1 +
> FatPkg/FatPkg.dsc                                  |   1 +
> .../IntelFrameworkModulePkg.dsc                    |   1 +
> IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc        |   1 +
> IntelFspWrapperPkg/IntelFspWrapperPkg.dsc          |   1 +
> MdeModulePkg/MdeModulePkg.dsc                      |   1 +
> MdePkg/Include/Library/UiLib.h                     |  71 +++++
> MdePkg/Library/UefiLib/Console.c                   | 252
> +---------------
> MdePkg/Library/UefiLib/UefiLib.inf                 |   1 +
> MdePkg/Library/UefiLib/UefiLibInternal.h           |   1 +
> MdePkg/Library/UiLib/UiLib.c                       | 334
> +++++++++++++++++++++
> MdePkg/Library/UiLib/UiLib.inf                     |  38 +++
> MdePkg/Library/UiLib/UiLib.uni                     |  21 ++
> MdePkg/MdePkg.dec                                  |   3 +
> NetworkPkg/NetworkPkg.dsc                          |   1 +
> Nt32Pkg/Nt32Pkg.dsc                                |   1 +
> Omap35xxPkg/Omap35xxPkg.dsc                        |   1 +
> OptionRomPkg/OptionRomPkg.dsc                      |   1 +
> OvmfPkg/OvmfPkgIa32.dsc                            |   1 +
> OvmfPkg/OvmfPkgIa32X64.dsc                         |   1 +
> OvmfPkg/OvmfPkgX64.dsc                             |   1 +
> PcAtChipsetPkg/PcAtChipsetPkg.dsc                  |   1 +
> PerformancePkg/PerformancePkg.dsc                  |   1 +
> QuarkPlatformPkg/Quark.dsc                         |   1 +
> QuarkPlatformPkg/QuarkMin.dsc                      |   1 +
> QuarkSocPkg/QuarkSocPkg.dsc                        |   1 +
> SecurityPkg/SecurityPkg.dsc                        |   1 +
> ShellPkg/ShellPkg.dsc                              |   1 +
> SourceLevelDebugPkg/SourceLevelDebugPkg.dsc        |   1 +
> StdLib/StdLib.dsc                                  |   1 +
> UefiCpuPkg/UefiCpuPkg.dsc                          |   1 +
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc            |   1 +
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc              |   1 +
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc               |   1 +
> 44 files changed, 506 insertions(+), 251 deletions(-)
> create mode 100644 MdePkg/Include/Library/UiLib.h
> create mode 100644 MdePkg/Library/UiLib/UiLib.c
> create mode 100644 MdePkg/Library/UiLib/UiLib.inf
> create mode 100644 MdePkg/Library/UiLib/UiLib.uni
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc.  This communication is intended
> to be read only by the individual or entity to whom it is addressed or by 
> their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited.  Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary 
to American Megatrends, Inc.  This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited.  Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to