Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
Yao, How many closed source packages are going to be working against the trunk? Don't most of those platforms already have a copy of ShellPkg and therefore they wont care? -Jaben > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Saturday, October 15, 2016 6:30 AM > To: Tim Lewis ; Ni, Ruiyu ; > edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > Importance: High > > Of course, it is fine to give a good name in MdePkg. :) > > My concern is only about "Compatibility". > This series patch only updated ArmPkg and EmbeddedPkg. > But there might be more close source IP protected packages using the > original header file. > > That is why I propose a compatible way: We give a good file name in MdePkg. > At same time, we can keep the old file name in ShellPkg to include the good > name in MdePkg. > > I recommend we give a solution to make close source package unchanged > after this patch is applied. > > Thank you > Yao Jiewen > > > From: Tim Lewis [mailto:tim.le...@insyde.com] > Sent: Saturday, October 15, 2016 12:21 AM > To: Yao, Jiewen ; Ni, Ruiyu ; > edk2-devel@lists.01.org > Subject: RE: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > I prefer the renamed .h file, even though I have substantial investment in > the current infrastructure. > > Why? Because engineers don't have time to remember "how does protocol > X translate to header file Y" It should be a consistent rule. How many times > have I needed to grep the header file name just because I got a build error > because the rule wasn't predictable. > > 1) remove the EFI_ prefix and the _PROTOCOL suffix > (EFI_BLOCK_IO_PROTOCOL -> BLOCK_IO > 2) Convert to upper-and-lower case, BLOCK_IO -> Block_Io > 3) Remove the _ (Block_Io -> BlockIo) > 4) add a .h BlockIo -> BlockIo.h > > Most protocol and PPI header files already follow this rule. The current > header files for shell are among the few exceptions, because they don't > follow this. > > Tim > > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Friday, October 14, 2016 6:14 AM > To: Yao, Jiewen mailto:jiewen@intel.com>>; > Ni, Ruiyu mailto:ruiyu...@intel.com>>; edk2- > de...@lists.01.org<mailto:edk2-devel@lists.01.org> > Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > Or if you really think we should give a better name. > > My recommendation is: > > 1) We add content in MdePkg. > > 2) We can keep the old protocol file in ShellPkg, but let .h in shellPkg > include the .h in MdePkg. > > Then we can avoid duplicated code and make it a compatible solution to > avoid other module update. > > Thank you > Yao Jiewen > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Friday, October 14, 2016 9:09 PM > To: Ni, Ruiyu mailto:ruiyu...@intel.com>>; edk2- > de...@lists.01.org<mailto:edk2-devel@lists.01.org> > Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > Hi > I think the requests is just to *move*. > > There is no request to *rename*. > > Can we just move to avoid other update? > > > > -Original Message- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > > Ruiyu Ni > > Sent: Friday, October 14, 2016 5:44 PM > > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2- > de...@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > > Subject: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > > > The patches moves Shell spec defined protocol definitions to MdePkg > > and updates all references. > > Content of ShellBase.h is moved to Protocol/Shell.h and ShellBase.h is > > removed. > > > > Ruiyu Ni (5): > > ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h > > MdePkg: Include Shell/ShellDynamicCommand/ShellParameters > > definitions > > ArmPkg/LinuxLoader: Reference Shell protocols in MdePkg > > EmbeddedPkg/FdtPlatformDxe: Reference Shell protocols in MdePkg > > ShellPkg: Remove Shell/ShellDynamicCommand/ShellParameter > > definitions > > > > ArmPkg/Application/LinuxLoader/LinuxLoader.h | 4 +- > > EmbeddedPkg/Drivers/FdtPlatformDxe/FdtPlatform.h | 4 +- > > .../EfiShell.h => MdePkg/Include/Protocol/Shell.h | 134 > > +- > > ..
Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
Of course, it is fine to give a good name in MdePkg. :) My concern is only about "Compatibility". This series patch only updated ArmPkg and EmbeddedPkg. But there might be more close source IP protected packages using the original header file. That is why I propose a compatible way: We give a good file name in MdePkg. At same time, we can keep the old file name in ShellPkg to include the good name in MdePkg. I recommend we give a solution to make close source package unchanged after this patch is applied. Thank you Yao Jiewen From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: Saturday, October 15, 2016 12:21 AM To: Yao, Jiewen ; Ni, Ruiyu ; edk2-devel@lists.01.org Subject: RE: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg I prefer the renamed .h file, even though I have substantial investment in the current infrastructure. Why? Because engineers don't have time to remember "how does protocol X translate to header file Y" It should be a consistent rule. How many times have I needed to grep the header file name just because I got a build error because the rule wasn't predictable. 1) remove the EFI_ prefix and the _PROTOCOL suffix (EFI_BLOCK_IO_PROTOCOL -> BLOCK_IO 2) Convert to upper-and-lower case, BLOCK_IO -> Block_Io 3) Remove the _ (Block_Io -> BlockIo) 4) add a .h BlockIo -> BlockIo.h Most protocol and PPI header files already follow this rule. The current header files for shell are among the few exceptions, because they don't follow this. Tim -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, Jiewen Sent: Friday, October 14, 2016 6:14 AM To: Yao, Jiewen mailto:jiewen@intel.com>>; Ni, Ruiyu mailto:ruiyu...@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg Or if you really think we should give a better name. My recommendation is: 1) We add content in MdePkg. 2) We can keep the old protocol file in ShellPkg, but let .h in shellPkg include the .h in MdePkg. Then we can avoid duplicated code and make it a compatible solution to avoid other module update. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, Jiewen Sent: Friday, October 14, 2016 9:09 PM To: Ni, Ruiyu mailto:ruiyu...@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg Hi I think the requests is just to *move*. There is no request to *rename*. Can we just move to avoid other update? > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ruiyu Ni > Sent: Friday, October 14, 2016 5:44 PM > To: > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > Subject: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > The patches moves Shell spec defined protocol definitions to MdePkg > and updates all references. > Content of ShellBase.h is moved to Protocol/Shell.h and ShellBase.h is > removed. > > Ruiyu Ni (5): > ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h > MdePkg: Include Shell/ShellDynamicCommand/ShellParameters > definitions > ArmPkg/LinuxLoader: Reference Shell protocols in MdePkg > EmbeddedPkg/FdtPlatformDxe: Reference Shell protocols in MdePkg > ShellPkg: Remove Shell/ShellDynamicCommand/ShellParameter > definitions > > ArmPkg/Application/LinuxLoader/LinuxLoader.h | 4 +- > EmbeddedPkg/Drivers/FdtPlatformDxe/FdtPlatform.h | 4 +- > .../EfiShell.h => MdePkg/Include/Protocol/Shell.h | 134 > +- > .../Include/Protocol/ShellDynamicCommand.h | 7 +- > .../Include/Protocol/ShellParameters.h | 4 +- > MdePkg/MdePkg.dec | 15 ++ > ShellPkg/Application/Shell/Shell.h | 5 +- > ShellPkg/Include/Library/ShellCommandLib.h | 5 +- > ShellPkg/Include/Library/ShellLib.h| 14 +- > ShellPkg/Include/ShellBase.h | 157 > - > ShellPkg/Library/UefiDpLib/Dp.h| 3 +- > ShellPkg/Library/UefiDpLib/UefiDpLib.h | 7 +- > .../UefiHandleParsingLib/UefiHandleParsingLib.h| 8 +- > .../UefiShellBcfgCommandLib.c | 5 +- > .../UefiShellCEntryLib/UefiShellCEntryLib.c| 6 +- > .../UefiShellCommandLib/UefiShellCommandLib.h | 7 +- > .../Library/UefiShellDebug1CommandsLib/Compress.c | 7 +- > .../UefiShellDebug1CommandsLib.h | 7 +- > .../UefiShellDriver1Comm
Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
I prefer the renamed .h file, even though I have substantial investment in the current infrastructure. Why? Because engineers don't have time to remember "how does protocol X translate to header file Y" It should be a consistent rule. How many times have I needed to grep the header file name just because I got a build error because the rule wasn't predictable. 1) remove the EFI_ prefix and the _PROTOCOL suffix (EFI_BLOCK_IO_PROTOCOL -> BLOCK_IO 2) Convert to upper-and-lower case, BLOCK_IO -> Block_Io 3) Remove the _ (Block_Io -> BlockIo) 4) add a .h BlockIo -> BlockIo.h Most protocol and PPI header files already follow this rule. The current header files for shell are among the few exceptions, because they don't follow this. Tim -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, Jiewen Sent: Friday, October 14, 2016 6:14 AM To: Yao, Jiewen ; Ni, Ruiyu ; edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg Or if you really think we should give a better name. My recommendation is: 1) We add content in MdePkg. 2) We can keep the old protocol file in ShellPkg, but let .h in shellPkg include the .h in MdePkg. Then we can avoid duplicated code and make it a compatible solution to avoid other module update. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, Jiewen Sent: Friday, October 14, 2016 9:09 PM To: Ni, Ruiyu ; edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg Hi I think the requests is just to *move*. There is no request to *rename*. Can we just move to avoid other update? > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ruiyu Ni > Sent: Friday, October 14, 2016 5:44 PM > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Subject: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > The patches moves Shell spec defined protocol definitions to MdePkg > and updates all references. > Content of ShellBase.h is moved to Protocol/Shell.h and ShellBase.h is > removed. > > Ruiyu Ni (5): > ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h > MdePkg: Include Shell/ShellDynamicCommand/ShellParameters > definitions > ArmPkg/LinuxLoader: Reference Shell protocols in MdePkg > EmbeddedPkg/FdtPlatformDxe: Reference Shell protocols in MdePkg > ShellPkg: Remove Shell/ShellDynamicCommand/ShellParameter > definitions > > ArmPkg/Application/LinuxLoader/LinuxLoader.h | 4 +- > EmbeddedPkg/Drivers/FdtPlatformDxe/FdtPlatform.h | 4 +- > .../EfiShell.h => MdePkg/Include/Protocol/Shell.h | 134 > +- > .../Include/Protocol/ShellDynamicCommand.h | 7 +- > .../Include/Protocol/ShellParameters.h | 4 +- > MdePkg/MdePkg.dec | 15 ++ > ShellPkg/Application/Shell/Shell.h | 5 +- > ShellPkg/Include/Library/ShellCommandLib.h | 5 +- > ShellPkg/Include/Library/ShellLib.h| 14 +- > ShellPkg/Include/ShellBase.h | 157 > - > ShellPkg/Library/UefiDpLib/Dp.h| 3 +- > ShellPkg/Library/UefiDpLib/UefiDpLib.h | 7 +- > .../UefiHandleParsingLib/UefiHandleParsingLib.h| 8 +- > .../UefiShellBcfgCommandLib.c | 5 +- > .../UefiShellCEntryLib/UefiShellCEntryLib.c| 6 +- > .../UefiShellCommandLib/UefiShellCommandLib.h | 7 +- > .../Library/UefiShellDebug1CommandsLib/Compress.c | 7 +- > .../UefiShellDebug1CommandsLib.h | 7 +- > .../UefiShellDriver1CommandsLib.h | 7 +- > .../UefiShellLevel1CommandsLib.h | 7 +- > .../UefiShellLevel2CommandsLib.h | 7 +- > ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c | 2 +- > .../UefiShellLevel3CommandsLib.h | 7 +- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 1 - > ShellPkg/Library/UefiShellLib/UefiShellLib.h | 6 +- > .../UefiShellNetwork1CommandsLib.h | 3 +- > .../UefiShellTftpCommandLib.h | 3 +- > ShellPkg/ShellPkg.dec | 3 - > ShellPkg/ShellPkg.dsc | 2 + > 29 files changed, 213 insertions(+), 234 deletions(-) rename > ShellPkg/Include/Protocol/EfiShell.h => > MdePkg/Include/Protocol/Shell.h (92%) rename > ShellPkg/Include/Protocol/EfiShellDynamicCommand.h => > MdePkg/Include/Protocol/ShellDynamicCommand.h (92%) rename > ShellPkg/Include/Protocol/EfiShellParameters.h =>
Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
Or if you really think we should give a better name. My recommendation is: 1) We add content in MdePkg. 2) We can keep the old protocol file in ShellPkg, but let .h in shellPkg include the .h in MdePkg. Then we can avoid duplicated code and make it a compatible solution to avoid other module update. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, Jiewen Sent: Friday, October 14, 2016 9:09 PM To: Ni, Ruiyu ; edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg Hi I think the requests is just to *move*. There is no request to *rename*. Can we just move to avoid other update? > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ruiyu Ni > Sent: Friday, October 14, 2016 5:44 PM > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Subject: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > The patches moves Shell spec defined protocol definitions to > MdePkg and updates all references. > Content of ShellBase.h is moved to Protocol/Shell.h and > ShellBase.h is removed. > > Ruiyu Ni (5): > ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h > MdePkg: Include Shell/ShellDynamicCommand/ShellParameters > definitions > ArmPkg/LinuxLoader: Reference Shell protocols in MdePkg > EmbeddedPkg/FdtPlatformDxe: Reference Shell protocols in MdePkg > ShellPkg: Remove Shell/ShellDynamicCommand/ShellParameter > definitions > > ArmPkg/Application/LinuxLoader/LinuxLoader.h | 4 +- > EmbeddedPkg/Drivers/FdtPlatformDxe/FdtPlatform.h | 4 +- > .../EfiShell.h => MdePkg/Include/Protocol/Shell.h | 134 > +- > .../Include/Protocol/ShellDynamicCommand.h | 7 +- > .../Include/Protocol/ShellParameters.h | 4 +- > MdePkg/MdePkg.dec | 15 ++ > ShellPkg/Application/Shell/Shell.h | 5 +- > ShellPkg/Include/Library/ShellCommandLib.h | 5 +- > ShellPkg/Include/Library/ShellLib.h| 14 +- > ShellPkg/Include/ShellBase.h | 157 > - > ShellPkg/Library/UefiDpLib/Dp.h| 3 +- > ShellPkg/Library/UefiDpLib/UefiDpLib.h | 7 +- > .../UefiHandleParsingLib/UefiHandleParsingLib.h| 8 +- > .../UefiShellBcfgCommandLib.c | 5 +- > .../UefiShellCEntryLib/UefiShellCEntryLib.c| 6 +- > .../UefiShellCommandLib/UefiShellCommandLib.h | 7 +- > .../Library/UefiShellDebug1CommandsLib/Compress.c | 7 +- > .../UefiShellDebug1CommandsLib.h | 7 +- > .../UefiShellDriver1CommandsLib.h | 7 +- > .../UefiShellLevel1CommandsLib.h | 7 +- > .../UefiShellLevel2CommandsLib.h | 7 +- > ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c | 2 +- > .../UefiShellLevel3CommandsLib.h | 7 +- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 1 - > ShellPkg/Library/UefiShellLib/UefiShellLib.h | 6 +- > .../UefiShellNetwork1CommandsLib.h | 3 +- > .../UefiShellTftpCommandLib.h | 3 +- > ShellPkg/ShellPkg.dec | 3 - > ShellPkg/ShellPkg.dsc | 2 + > 29 files changed, 213 insertions(+), 234 deletions(-) > rename ShellPkg/Include/Protocol/EfiShell.h => > MdePkg/Include/Protocol/Shell.h (92%) > rename ShellPkg/Include/Protocol/EfiShellDynamicCommand.h => > MdePkg/Include/Protocol/ShellDynamicCommand.h (92%) > rename ShellPkg/Include/Protocol/EfiShellParameters.h => > MdePkg/Include/Protocol/ShellParameters.h (92%) > delete mode 100644 ShellPkg/Include/ShellBase.h > > -- > 2.9.0.windows.1 > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
Hi I think the requests is just to *move*. There is no request to *rename*. Can we just move to avoid other update? > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ruiyu Ni > Sent: Friday, October 14, 2016 5:44 PM > To: edk2-devel@lists.01.org > Subject: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg > > The patches moves Shell spec defined protocol definitions to > MdePkg and updates all references. > Content of ShellBase.h is moved to Protocol/Shell.h and > ShellBase.h is removed. > > Ruiyu Ni (5): > ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h > MdePkg: Include Shell/ShellDynamicCommand/ShellParameters > definitions > ArmPkg/LinuxLoader: Reference Shell protocols in MdePkg > EmbeddedPkg/FdtPlatformDxe: Reference Shell protocols in MdePkg > ShellPkg: Remove Shell/ShellDynamicCommand/ShellParameter > definitions > > ArmPkg/Application/LinuxLoader/LinuxLoader.h | 4 +- > EmbeddedPkg/Drivers/FdtPlatformDxe/FdtPlatform.h | 4 +- > .../EfiShell.h => MdePkg/Include/Protocol/Shell.h | 134 > +- > .../Include/Protocol/ShellDynamicCommand.h | 7 +- > .../Include/Protocol/ShellParameters.h | 4 +- > MdePkg/MdePkg.dec | 15 ++ > ShellPkg/Application/Shell/Shell.h | 5 +- > ShellPkg/Include/Library/ShellCommandLib.h | 5 +- > ShellPkg/Include/Library/ShellLib.h| 14 +- > ShellPkg/Include/ShellBase.h | 157 > - > ShellPkg/Library/UefiDpLib/Dp.h| 3 +- > ShellPkg/Library/UefiDpLib/UefiDpLib.h | 7 +- > .../UefiHandleParsingLib/UefiHandleParsingLib.h| 8 +- > .../UefiShellBcfgCommandLib.c | 5 +- > .../UefiShellCEntryLib/UefiShellCEntryLib.c| 6 +- > .../UefiShellCommandLib/UefiShellCommandLib.h | 7 +- > .../Library/UefiShellDebug1CommandsLib/Compress.c | 7 +- > .../UefiShellDebug1CommandsLib.h | 7 +- > .../UefiShellDriver1CommandsLib.h | 7 +- > .../UefiShellLevel1CommandsLib.h | 7 +- > .../UefiShellLevel2CommandsLib.h | 7 +- > ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c | 2 +- > .../UefiShellLevel3CommandsLib.h | 7 +- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 1 - > ShellPkg/Library/UefiShellLib/UefiShellLib.h | 6 +- > .../UefiShellNetwork1CommandsLib.h | 3 +- > .../UefiShellTftpCommandLib.h | 3 +- > ShellPkg/ShellPkg.dec | 3 - > ShellPkg/ShellPkg.dsc | 2 + > 29 files changed, 213 insertions(+), 234 deletions(-) > rename ShellPkg/Include/Protocol/EfiShell.h => > MdePkg/Include/Protocol/Shell.h (92%) > rename ShellPkg/Include/Protocol/EfiShellDynamicCommand.h => > MdePkg/Include/Protocol/ShellDynamicCommand.h (92%) > rename ShellPkg/Include/Protocol/EfiShellParameters.h => > MdePkg/Include/Protocol/ShellParameters.h (92%) > delete mode 100644 ShellPkg/Include/ShellBase.h > > -- > 2.9.0.windows.1 > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
