Re: [PATCH 0/2] MdePkg: UiLib library

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Felix,

The CONSTRUCTOR for the UiLib could do the registration.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix
> Poludov
> Sent: Thursday, November 10, 2016 7:44 AM
> To: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-
> devel@lists.01.org
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
> 
> Liming,
> 
> With your proposal if a projects uses RegisterCreatePopUpHandler(), DSC file has
> to be updated to link popup handler NULL library
> with all the modules that consume CreatePopup().
> Since projects tend to evolve (new modules added, existing code changes), this
> sounds like a constant maintenance effort.
> Project integrator would have to scan source tree after each commit to check if a
> new code that uses CretePopup has been added and if new DSC file modifications
> are required.
> 
> Thanks
> Felix
> 
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Thursday, November 10, 2016 10:10 AM
> To: Felix Poludov; Yao, Jiewen; edk2-devel@lists.01.org
> Subject: RE: [PATCH 0/2] MdePkg: UiLib library
> 
> Felix:
>   I think two major concerns are here.
> 
> 1)       UefiLib depends on UiLib
> 
> 2)       UiLib class is defined in MdePkg.
> 
>   To resolve it, I have another idea.
> 
> 1)       Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input
> parameter is the handler function that is same to UiCreatePopUpVaList().
> 
> 2)       Update CreatePopup() to use the registered handler if new handler is
> registered. If no handler, CreatePopup() still uses current logic.
> 
> 3)       If platform wants to customize CreatePopup(), it can provide its library
> instance with library constructor only. Its library class is NULL, its library
> constructor() will consume RegisterCreatePopUpHandler() to register its handler.
>   This solution is compatible. It just updates UefiLib, doesn't need to update
> platform DSC files if the platforms don't enable the customized popup. It doesn't
> conflict with new UiLib design. New UiLib class introduce new APIs and update
> edk2 codes to use new one. For the driver uses old API, this way can provide the
> customization. For the driver uses new API, the different UiLib instances can
> provide the customization.
> 
> Thanks
> Liming
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix
> Poludov
> Sent: Thursday, November 10, 2016 9:38 PM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-
> devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
> 
> 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.yao@intel.com]
> Sent: Wednesday, November 09, 2016 8:54 PM
> To: Felix Poludov; edk2-devel@lists.01.org<mailto: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-bounces@lists.01.org] On Behalf Of
> > Felix Poludov
> > Sent: Wednesday, November 9, 2016 11:44 PM
> > To: edk2-devel@lists.01.org<mailto: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
> >
> > 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<mailto: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<mailto: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