Re: [PATCH] GPT Shell Application/Library

["Shah, Tapan" <tapandshah@hpe.com>]

1. Create this new shared library in MdeModulePkg.
2. For parameters: Other than standard parameters defined in Shell specification I would suggest to follow single character flag where it makes sense (example: -d to delete, -l to list, -c to create etc.) and if more than one operation can be tied with one flag, then add individual value as a flag value. There are plenty of ShellPkg commands you can use as a reference.

-----Original Message-----
From: Carsey, Jaben [mailto:jaben.carsey@intel.com] 
Sent: Tuesday, October 18, 2016 12:04 PM
To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; Shah, Tapan <tapandshah@hpe.com>; Michael Zimmermann <sigmaepsilon92@gmail.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Arshi, Shala <shala.arshi@intel.com>; edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Laszlo Ersek <lersek@redhat.com>; Carsey, Jaben <jaben.carsey@intel.com>
Subject: RE: [edk2] [PATCH] GPT Shell Application/Library

For the standards, I would try to use the UEFI Shell Spec for some parameters.  For Example:
-v for verbose
-h for help
-sfo for standard format output

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Vladimir Olovyannikov
> Sent: Tuesday, October 18, 2016 9:58 AM
> To: Shah, Tapan <tapandshah@hpe.com>; Carsey, Jaben 
> <jaben.carsey@intel.com>; Michael Zimmermann 
> <sigmaepsilon92@gmail.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Arshi, Shala 
> <shala.arshi@intel.com>; edk2-devel@lists.01.org 
> <edk2-devel@ml01.01.org>; Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> Importance: High
> 
> Thank you all for comments,
> 
> So to summarize the discussion:
> 
> 1. I will create a Shell library which would perform all GPT operations.
>      Part of PartitionDxe will also be in that library so PartitionDxe 
> will be using it.
>      The gpt Shell tool will also be using it.
> 2.  Refactor the parameters of the gpt utility to make it similar to 
> other existing Shell commands.
>       BTW Is there any document describing Shell utility parameters'
> standards?
> 
> Please let me know if you have other suggestions.
> 
> Thank you,
> Vladimir
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Shah, Tapan
> Sent: October-18-16 6:59 AM
> To: Carsey, Jaben; Vladimir Olovyannikov; Michael Zimmermann
> Cc: Ni, Ruiyu; Arshi, Shala; edk2-devel@lists.01.org; Laszlo Ersek
> Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> 
> Thanks for the contribution Vladimir!
> 
> Few comments:
> 1. It's better to refactor the code now before commit and move GPT 
> related code outside ShellPkg and create a shared library.
> 2. CLI parameters of this utility are too complex and need to be 
> refactored to make it similar to other existing Shell commands.
> 
> Thanks,
> Tapan
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Carsey, Jaben
> Sent: Monday, October 17, 2016 12:56 PM
> To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; 
> Michael Zimmermann <sigmaepsilon92@gmail.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Arshi, Shala 
> <shala.arshi@intel.com>; edk2-devel@lists.01.org 
> <edk2-devel@ml01.01.org>; Carsey, Jaben <jaben.carsey@intel.com>; 
> Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> 
> To the old question about license: I asked our people to check and was 
> told that the license is compatible with our BSD and ok by Intel.
> 
> To the technical content – I really like this idea of a shared library.
> That would be a great way to share code and not have as much duplicate.
> 
> -Jaben
> 
> From: Vladimir Olovyannikov 
> [mailto:vladimir.olovyannikov@broadcom.com]
> Sent: Monday, October 17, 2016 10:52 AM
> To: Michael Zimmermann <sigmaepsilon92@gmail.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; Carsey, Jaben 
> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; 
> edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
> Subject: RE: [edk2] [PATCH] GPT Shell Application/Library
> Importance: High
> 
> Hi Michael,
> I am absolutely agree with your proposal.
> 
> In the gpt Shell library/application I had to “borrow” some stuff from 
> PartitionDxe.c to not reinvent a  wheel.
> If the PartitionDxe maintainer agrees to have a separate library 
> available for everybody, I would move all the GPT-related stuff from 
> the GptWorker (and partially from the PartitionDxe itself) to that independent library.
> This could be a longer-term task.
> Right now I just wanted to share the tool which could be useful for 
> anybody who would wish to manage GPT partitions (and/or do a FAT32 
> format of either a disk or a GPT partition) from within the Shell. 
> What do you think?
> 
> Thank you,
> Vladimir
> From: Michael Zimmermann
> [mailto:sigmaepsilon92@gmail.com<mailto:sigmaepsilon92@gmail.com>]
> Sent: October-17-16 12:25 AM
> To: Vladimir Olovyannikov
> Cc: Laszlo Ersek; Jaben Carsey; Ni, Ruiyu; 
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> 
> Hi,
> 
> wouldn't it be better to make a generic gpt parsing library which is 
> independent of the shell so both the shell and PartitionDxe can use it?
> It may also be useful for other applications which need additional 
> information like the gpt partition names.
> 
> Thanks
> Michael
> 
> On Mon, Oct 17, 2016 at 8:49 AM, Vladimir Olovyannikov 
> <vladimir.olovyannikov@broadcom.com<mailto:vladimir.olovyannikov@broa
> dcom.com>>
> wrote:
> Thank you Laszlo,
> 
> Sorry, I missed the fields; it is my first contribution, I will add 
> the required lines, review the source according to your comments and 
> will resubmit the patch.
> So do you think the command should be _gpt instead of gpt? I was 
> following TFTP and SF commands as a template.
> 
> Thank you,
> Vladimir
> 
> On Oct 16, 2016 1:05 PM, "Laszlo Ersek"
> <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:
> >
> > On 10/16/16 07:23, Vladimir Olovyannikov wrote:
> > > This allows managing (create, delete, modify, fat format) of GPT 
> > > partitions from within UEFI Shell.
> > > Syntax:
> > > gpt <command> [device_mapped_name] [parameters...] See usage 
> > > examples in the .uni file
> > > ---
> > >  .../Library/UefiShellGptCommandLib/FatFormat.c     |  611 +++++++
> > >  .../Library/UefiShellGptCommandLib/FatFormat.h     |  111 ++
> > >  .../Library/UefiShellGptCommandLib/GptWorker.c     | 1902
> ++++++++++++++++++++
> > >  .../Library/UefiShellGptCommandLib/GptWorker.h     |  186 ++
> > >  .../UefiShellGptCommandLib.c                       | 1135 ++++++++++++
> > >  .../UefiShellGptCommandLib.inf                     |   79 +
> > >  .../UefiShellGptCommandLib.uni                     |  117 ++
> > >  ShellPkg/ShellPkg.dec                              |    1 +
> > >  ShellPkg/ShellPkg.dsc                              |    4 +
> > >  9 files changed, 4146 insertions(+)  create mode 100644 
> > > ShellPkg/Library/UefiShellGptCommandLib/FatFormat.c
> > >  create mode 100644
> > > ShellPkg/Library/UefiShellGptCommandLib/FatFormat.h
> > >  create mode 100644
> > > ShellPkg/Library/UefiShellGptCommandLib/GptWorker.c
> > >  create mode 100644
> > > ShellPkg/Library/UefiShellGptCommandLib/GptWorker.h
> > >  create mode 100644
> ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.c
> > >  create mode 100644
> ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.inf
> > >  create mode 100644
> ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.uni
> >
> > This looks like a supremely welcome, long-awaited addition (latest
> > request:
> > <https://lists.01.org/pipermail/edk2-devel/2016-October/002667.html>
> > ), but it really needs your Signed-off-by, and the Contributed-under 
> > line above it:
> >
> > ShellPkg/Contributions.txt
> >
> > I would also suggest (simply based on what I've seen elsewhere in
> > edk2) to keep the copyright notices tightly collected in the file 
> > headings.
> >
> > Someone will have to go over all the licenses too -- does the 
> > "Marvell BSD License Option" for example correspond to the 3-clause BSDL?
> >
> > On the technical side, I believe that as long as a shell command (or 
> > a command option) is not standardized (in the shell spec), it 
> > usually starts with an underscore (_), so as to prevent future name collisions.
> > (I could be wrong about this -- I now recall the TFTP command, which 
> > is also not in the 2.2 spec.)
> >
> > Just my two cents.
> >
> > Thanks
> > Laszlo
> _______________________________________________
> 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
> _______________________________________________
> 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