public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Zhichao" <zhichao.gao@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	Vitaly Cheptsov <vit9696@protonmail.com>
Subject: Re: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances
Date: Mon, 23 Dec 2019 00:43:39 +0000	[thread overview]
Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B8A4DE2@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C3A41D7@SHSMSX104.ccr.corp.intel.com>

Ray,

Your suggestion is good for open source, but unfriendly to the close source platforms which consume this lib.

Hi Mike/Liming/Ray/Others,

Do we have a progress to retire lib/API/others in the open source, like below?
1. Announce that there is something going to retire.
2. Suggestion to replace the retired function in open source. Or the justification of the retirement.
3. Collect the feedback especially the disagreement.
4. Discuss and make the decision.
5. Reject the retirement. Or announce the retire date to let consumers to change their platform codes.
6. Retire the function.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, December 20, 2019 3:16 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate
> the lib instances
> 
> Zhichao,
> I prefer to have one patch serial which include:
> 1. Adds new mandatory instance
> 2. Update consumers to use the new instance 2. Removes the old mandatory
> instance
> 
> Otherwise, adding a new mandatory instance introduces more code duplication
> IMO.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao <zhichao.gao@intel.com>
> > Sent: Friday, December 20, 2019 2:41 PM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > Separate the lib instances
> >
> > For open source, it would impact fsp2 package, ovmf package and some
> > open platform packages. Not sure for others.
> > I didn't plan the removal of UefiDevicePathLibDevicePathProtocol yet.
> >
> > Thanks,
> > Zhichao
> > > -----Original Message-----
> > > From: Ni, Ray
> > > Sent: Friday, December 20, 2019 2:20 PM
> > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > Separate
> > > the lib instances
> > >
> > > Removing code duplication is great.
> > >
> > > But your patch introduces more code duplication: the mandatory one
> > > in UefiDevicePathLib directory and the other one in
> > > UefiDevicePathLibDevicePathProtocol directory.
> > >
> > > Do you have a plan to remove the one in
> > UefiDevicePathLibDevicePathProtocol
> > > directory?
> > > Have you evaluated the impact to consumers of removing that one?
> > >
> > > Thanks,
> > > Ray
> > >
> > > > -----Original Message-----
> > > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > > Sent: Friday, December 20, 2019 2:03 PM
> > > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > Separate the lib instances
> > > >
> > > > Ray,
> > > >
> > > > I knew there is one in MdePkg. But it has duplicate code with
> > > > UefiDevicePathLib. That is why I add the Mandatory one.
> > > > And it is recommended to use the one in UefiDevicePathLib path.
> > > >
> > > > Thanks,
> > > > Zhichao
> > > >
> > > > > -----Original Message-----
> > > > > From: Ni, Ray
> > > > > Sent: Friday, December 20, 2019 1:50 PM
> > > > > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > > <liming.gao@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> > > > > Subject: RE: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > Separate
> > > > > the lib instances
> > > > >
> > > > > Zhichao,
> > > > > \MdePkg\Library\UefiDevicePathLibDevicePathProtocol\ contains
> > > > > the
> > > > version
> > > > > that hard-depends on the protocol.
> > > > > I don't think you need to add another version.
> > > > >
> > > > > Thanks,
> > > > > Ray
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > > Gao, Zhichao
> > > > > > Sent: Wednesday, December 18, 2019 10:11 AM
> > > > > > To: devel@edk2.groups.io
> > > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> > > > > > Liming <liming.gao@intel.com>; Vitaly Cheptsov
> > > > > > <vit9696@protonmail.com>
> > > > > > Subject: [edk2-devel] [PATCH V3 0/2] *MdePkg/UefiDevicePathLib:
> > > > > > Separate the lib instances
> > > > > >
> > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2298
> > > > > >
> > > > > > The UefiDevicePathLibOptionalDevicePathProtocolConstructor's
> > > > > > implementation
> > > > > > isn't match with its instance name.
> > > > > > Remove the ASSERT and depex of the
> > > > gEfiDevicePathUtilitiesProtocolGuid
> > > > > > because of "Optional".
> > > > > >
> > > > > > Add a mandatory instance to force using the
> > > > > > DevicePathUtilities, DevicePathToText and DevicePathFromText
> > > > > > protocol with the ASSERT
> > > > and
> > > > > > depex.
> > > > > >
> > > > > > V2:
> > > > > > The optional lib instance's construction should return success
> > > > > > all the time.
> > > > > > Change the desciption of the optional lib uni file.
> > > > > > Change the copyright date of the mandatory one's uni file.
> > > > > >
> > > > > > V3:
> > > > > > Remove the Status variable in
> > > > > > UefiDevicePathLibOptionalDevicePathProtocolConstructor.
> > > > > > The Status would cause GCC build fail because the variable is
> > > > > > initialized but not used.
> > > > > > Since it is useless for the constructor, directly remove it.
> > > > > >
> > > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > > Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> > > > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > > >
> > > > > > Zhichao Gao (2):
> > > > > >   MdePkg/UefiDevicePathLib: Separate the device path lib
> > > > > >   MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol
> > for
> > > > > > build
> > > > > >
> > > > > >  ...DevicePathLibMandatoryDevicePathProtocol.c | 469
> > > > > > ++++++++++++++++++
> > > > > >  ...vicePathLibMandatoryDevicePathProtocol.inf |  86 ++++
> > > > > > ...vicePathLibMandatoryDevicePathProtocol.uni |  18 +
> > > > > > ...iDevicePathLibOptionalDevicePathProtocol.c |  21 +-
> > > > > >  ...evicePathLibOptionalDevicePathProtocol.inf |   5 +-
> > > > > >  ...evicePathLibOptionalDevicePathProtocol.uni |   6 +-
> > > > > >  MdePkg/MdePkg.dsc                             |   3 +-
> > > > > >  7 files changed, 587 insertions(+), 21 deletions(-)  create
> > > > > > mode
> > > > > > 100644
> > > > > >
> > > >
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > > hProtocol.c
> > > > > >  create mode 100644
> > > > > >
> > > >
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > > hProtocol.inf
> > > > > >  create mode 100644
> > > > > >
> > > >
> > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibMandatoryDevicePat
> > > > > > hProtocol.uni
> > > > > >
> > > > > > --
> > > > > > 2.21.0.windows.1
> > > > > >
> > > > > >
> > > > > > 


  reply	other threads:[~2019-12-23  0:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-18  2:10 [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Gao, Zhichao
2019-12-18  2:10 ` [PATCH V3 1/2] MdePkg/UefiDevicePathLib: Separate the device path lib Gao, Zhichao
2019-12-18  8:27   ` Vitaly Cheptsov
2019-12-18  2:10 ` [PATCH V3 2/2] MdePkg/dsc: Add UefiDevicePathLibMandatoryDevicePathProtocol for build Gao, Zhichao
2019-12-18  8:28   ` Vitaly Cheptsov
2019-12-18  8:26 ` [PATCH V3 0/2] *MdePkg/UefiDevicePathLib: Separate the lib instances Vitaly Cheptsov
2019-12-19  1:51   ` [edk2-devel] " Gao, Zhichao
2019-12-20  5:49 ` Ni, Ray
2019-12-20  6:03   ` Gao, Zhichao
2019-12-20  6:19     ` Ni, Ray
2019-12-20  6:40       ` Gao, Zhichao
2019-12-20  7:16         ` Ni, Ray
2019-12-23  0:43           ` Gao, Zhichao [this message]
2019-12-23  8:01             ` Liming Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3CE959C139B4C44DBEA1810E3AA6F9000B8A4DE2@SHSMSX101.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox