1. Patchew
  2. EDK2
  3. [edk2] [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID
  4. View patch

[edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package

Ard Biesheuvel posted 4 patches 7 years, 7 months ago
[edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
Posted by Ard Biesheuvel 7 years, 7 months ago 
In QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c,
there is a definition of mUefiShellFileGuid which is a constant reference
to the FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf.

To prevent the need for duplicating it to other modules, promote it to
a proper global GUID, and add it to the ShellPkg.dec package declaration.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ShellPkg/ShellPkg.dec | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
index bb31c2df8cb3..3ad17a44b447 100644
--- a/ShellPkg/ShellPkg.dec
+++ b/ShellPkg/ShellPkg.dec
@@ -57,6 +57,9 @@ [Guids]
   gShellTftpHiiGuid               = {0x738a9314, 0x82c1, 0x4592, {0x8f, 0xf7, 0xc1, 0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
   gShellBcfgHiiGuid               = {0x5f5f605d, 0x1583, 0x4a2d, {0xa6, 0xb2, 0xeb, 0x12, 0xda, 0xb4, 0xa2, 0xb6}}
 
+  # FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf
+  gUefiShellFileGuid              = {0x7c04a583, 0x9e3e, 0x4f1c, {0xad, 0x65, 0xe0, 0x52, 0x68, 0xd0, 0xb4, 0xd1}}
+
 [Protocols]
   gEfiShellEnvironment2Guid           = {0x47c7b221, 0xc42a, 0x11d2, {0x8e, 0x57, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
   gEfiShellInterfaceGuid              = {0x47c7b223, 0xc42a, 0x11d2, {0x8e, 0x57, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
Posted by Carsey, Jaben 7 years, 7 months ago 
Ard,

I am good with this change.  

What do you think about a comment to the INF file so that if someone makes a change to the GUI there, they are informed that they need to change this location?  I worry as usually updating an INF GUID is permitted without any need to change another file...

-Jaben

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, March 22, 2017 7:04 AM
> To: edk2-devel@lists.01.org; leif.lindholm@linaro.org; lersek@redhat.com;
> Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Steele, Kelly
> <kelly.steele@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of
> UEFI Shell app to package
> Importance: High
> 
> In
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.
> c,
> there is a definition of mUefiShellFileGuid which is a constant reference
> to the FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf.
> 
> To prevent the need for duplicating it to other modules, promote it to
> a proper global GUID, and add it to the ShellPkg.dec package declaration.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ShellPkg/ShellPkg.dec | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
> index bb31c2df8cb3..3ad17a44b447 100644
> --- a/ShellPkg/ShellPkg.dec
> +++ b/ShellPkg/ShellPkg.dec
> @@ -57,6 +57,9 @@ [Guids]
>    gShellTftpHiiGuid               = {0x738a9314, 0x82c1, 0x4592, {0x8f, 0xf7, 0xc1,
> 0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
>    gShellBcfgHiiGuid               = {0x5f5f605d, 0x1583, 0x4a2d, {0xa6, 0xb2, 0xeb,
> 0x12, 0xda, 0xb4, 0xa2, 0xb6}}
> 
> +  # FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf
> +  gUefiShellFileGuid              = {0x7c04a583, 0x9e3e, 0x4f1c, {0xad, 0x65, 0xe0,
> 0x52, 0x68, 0xd0, 0xb4, 0xd1}}
> +
>  [Protocols]
>    gEfiShellEnvironment2Guid           = {0x47c7b221, 0xc42a, 0x11d2, {0x8e,
> 0x57, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
>    gEfiShellInterfaceGuid              = {0x47c7b223, 0xc42a, 0x11d2, {0x8e, 0x57,
> 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
> --
> 2.7.4
> 
> _______________________________________________
> 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
Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
Posted by Ard Biesheuvel 7 years, 7 months ago 
On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> Ard,
>
> I am good with this change.
>
> What do you think about a comment to the INF file so that if someone makes a change to the GUI there, they are informed that they need to change this location?  I worry as usually updating an INF GUID is permitted without any need to change another file...
>

Something like this perhaps?

--- a/ShellPkg/Application/Shell/Shell.inf
+++ b/ShellPkg/Application/Shell/Shell.inf
@@ -17,7 +17,7 @@
 [Defines]
   INF_VERSION    = 0x00010006
   BASE_NAME      = Shell
-  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
+  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 # gUefiShellFileGuid
   MODULE_TYPE    = UEFI_APPLICATION
   VERSION_STRING = 1.0
   ENTRY_POINT    = UefiMain

(Note that the same FILE_GUID occurs in ShellBinPkg as well, but
people are unlikely that randomly change that one without regard to
the source build)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
Posted by Carsey, Jaben 7 years, 7 months ago 
Yes. that looks great.

For the changes to ShellPkg.
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, March 22, 2017 8:20 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>;
> lersek@redhat.com
> Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> FILE_GUID of UEFI Shell app to package
> Importance: High
> 
> On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> > Ard,
> >
> > I am good with this change.
> >
> > What do you think about a comment to the INF file so that if someone
> makes a change to the GUI there, they are informed that they need to
> change this location?  I worry as usually updating an INF GUID is permitted
> without any need to change another file...
> >
> 
> Something like this perhaps?
> 
> --- a/ShellPkg/Application/Shell/Shell.inf
> +++ b/ShellPkg/Application/Shell/Shell.inf
> @@ -17,7 +17,7 @@
>  [Defines]
>    INF_VERSION    = 0x00010006
>    BASE_NAME      = Shell
> -  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
> +  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
> gUefiShellFileGuid
>    MODULE_TYPE    = UEFI_APPLICATION
>    VERSION_STRING = 1.0
>    ENTRY_POINT    = UefiMain
> 
> (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
> people are unlikely that randomly change that one without regard to
> the source build)
> _______________________________________________
> 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
Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
Posted by Kinney, Michael D 7 years, 7 months ago 
Jaben,

I like the added comment.

Maybe we should consider an INF spec enhancement to support a GUID C Name 
for the FILE_GUID define, and the GUID C Names can be any GUID declared
in a dependent packages from the [Packages] section of the INF.  This
would eliminate the GUID value duplication for this use case.

Mike

> -----Original Message-----
> From: Carsey, Jaben
> Sent: Wednesday, March 22, 2017 8:21 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; leif.lindholm@linaro.org;
> Kinney, Michael D <michael.d.kinney@intel.com>; lersek@redhat.com; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI
> Shell app to package
> 
> Yes. that looks great.
> 
> For the changes to ShellPkg.
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Wednesday, March 22, 2017 8:20 AM
> > To: Carsey, Jaben <jaben.carsey@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> > leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>;
> > lersek@redhat.com
> > Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> > FILE_GUID of UEFI Shell app to package
> > Importance: High
> >
> > On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> > > Ard,
> > >
> > > I am good with this change.
> > >
> > > What do you think about a comment to the INF file so that if someone
> > makes a change to the GUI there, they are informed that they need to
> > change this location?  I worry as usually updating an INF GUID is permitted
> > without any need to change another file...
> > >
> >
> > Something like this perhaps?
> >
> > --- a/ShellPkg/Application/Shell/Shell.inf
> > +++ b/ShellPkg/Application/Shell/Shell.inf
> > @@ -17,7 +17,7 @@
> >  [Defines]
> >    INF_VERSION    = 0x00010006
> >    BASE_NAME      = Shell
> > -  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
> > +  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
> > gUefiShellFileGuid
> >    MODULE_TYPE    = UEFI_APPLICATION
> >    VERSION_STRING = 1.0
> >    ENTRY_POINT    = UefiMain
> >
> > (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
> > people are unlikely that randomly change that one without regard to
> > the source build)
> > _______________________________________________
> > 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
Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
Posted by Carsey, Jaben 7 years, 7 months ago 
That would be quite nice.  I know that spinning GUIDs due to a non-backwards compatible change can be a scary thing.

-Jaben

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, March 22, 2017 8:40 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> leif.lindholm@linaro.org; lersek@redhat.com
> Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> FILE_GUID of UEFI Shell app to package
> Importance: High
> 
> Jaben,
> 
> I like the added comment.
> 
> Maybe we should consider an INF spec enhancement to support a GUID C
> Name
> for the FILE_GUID define, and the GUID C Names can be any GUID declared
> in a dependent packages from the [Packages] section of the INF.  This
> would eliminate the GUID value duplication for this use case.
> 
> Mike
> 
> > -----Original Message-----
> > From: Carsey, Jaben
> > Sent: Wednesday, March 22, 2017 8:21 AM
> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> leif.lindholm@linaro.org;
> > Kinney, Michael D <michael.d.kinney@intel.com>; lersek@redhat.com;
> Carsey, Jaben
> > <jaben.carsey@intel.com>
> > Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> FILE_GUID of UEFI
> > Shell app to package
> >
> > Yes. that looks great.
> >
> > For the changes to ShellPkg.
> > Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > > Ard Biesheuvel
> > > Sent: Wednesday, March 22, 2017 8:20 AM
> > > To: Carsey, Jaben <jaben.carsey@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> > > leif.lindholm@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > lersek@redhat.com
> > > Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> > > FILE_GUID of UEFI Shell app to package
> > > Importance: High
> > >
> > > On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com>
> wrote:
> > > > Ard,
> > > >
> > > > I am good with this change.
> > > >
> > > > What do you think about a comment to the INF file so that if someone
> > > makes a change to the GUI there, they are informed that they need to
> > > change this location?  I worry as usually updating an INF GUID is
> permitted
> > > without any need to change another file...
> > > >
> > >
> > > Something like this perhaps?
> > >
> > > --- a/ShellPkg/Application/Shell/Shell.inf
> > > +++ b/ShellPkg/Application/Shell/Shell.inf
> > > @@ -17,7 +17,7 @@
> > >  [Defines]
> > >    INF_VERSION    = 0x00010006
> > >    BASE_NAME      = Shell
> > > -  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
> > > +  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
> > > gUefiShellFileGuid
> > >    MODULE_TYPE    = UEFI_APPLICATION
> > >    VERSION_STRING = 1.0
> > >    ENTRY_POINT    = UefiMain
> > >
> > > (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
> > > people are unlikely that randomly change that one without regard to
> > > the source build)
> > > _______________________________________________
> > > 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
Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
Posted by Andrew Fish 7 years, 7 months ago 
> On Mar 22, 2017, at 8:39 AM, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
> 
> Jaben,
> 
> I like the added comment.
> 
> Maybe we should consider an INF spec enhancement to support a GUID C Name 
> for the FILE_GUID define, and the GUID C Names can be any GUID declared
> in a dependent packages from the [Packages] section of the INF.  This
> would eliminate the GUID value duplication for this use case.
> 

+1

Thanks,

Andrew Fish

> Mike
> 
>> -----Original Message-----
>> From: Carsey, Jaben
>> Sent: Wednesday, March 22, 2017 8:21 AM
>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; leif.lindholm@linaro.org;
>> Kinney, Michael D <michael.d.kinney@intel.com>; lersek@redhat.com; Carsey, Jaben
>> <jaben.carsey@intel.com>
>> Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI
>> Shell app to package
>> 
>> Yes. that looks great.
>> 
>> For the changes to ShellPkg.
>> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
>> 
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>>> Ard Biesheuvel
>>> Sent: Wednesday, March 22, 2017 8:20 AM
>>> To: Carsey, Jaben <jaben.carsey@intel.com>
>>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
>>> leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>;
>>> lersek@redhat.com
>>> Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
>>> FILE_GUID of UEFI Shell app to package
>>> Importance: High
>>> 
>>> On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
>>>> Ard,
>>>> 
>>>> I am good with this change.
>>>> 
>>>> What do you think about a comment to the INF file so that if someone
>>> makes a change to the GUI there, they are informed that they need to
>>> change this location?  I worry as usually updating an INF GUID is permitted
>>> without any need to change another file...
>>>> 
>>> 
>>> Something like this perhaps?
>>> 
>>> --- a/ShellPkg/Application/Shell/Shell.inf
>>> +++ b/ShellPkg/Application/Shell/Shell.inf
>>> @@ -17,7 +17,7 @@
>>> [Defines]
>>>   INF_VERSION    = 0x00010006
>>>   BASE_NAME      = Shell
>>> -  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
>>> +  FILE_GUID      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
>>> gUefiShellFileGuid
>>>   MODULE_TYPE    = UEFI_APPLICATION
>>>   VERSION_STRING = 1.0
>>>   ENTRY_POINT    = UefiMain
>>> 
>>> (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
>>> people are unlikely that randomly change that one without regard to
>>> the source build)
>>> _______________________________________________
>>> 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.