diff mbox series

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

Message ID 1490191448-22398-2-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show
Series Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID | expand

Commit Message

Ard Biesheuvel March 22, 2017, 2:04 p.m. UTC 
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(+)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Carsey, Jaben March 22, 2017, 3:13 p.m. UTC | #1 
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
Ard Biesheuvel March 22, 2017, 3:19 p.m. UTC | #2 
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?


(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--- 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
Carsey, Jaben March 22, 2017, 3:21 p.m. UTC | #3 
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
Kinney, Michael D March 22, 2017, 3:39 p.m. UTC | #4 
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
Carsey, Jaben March 22, 2017, 3:41 p.m. UTC | #5 
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
Andrew Fish March 22, 2017, 3:54 p.m. UTC | #6 
> 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
diff mbox series

Patch

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}}