On 6 March 2015 at 13:35, Laszlo Ersek <ler...@redhat.com> wrote:
> On 03/06/15 13:14, Ard Biesheuvel wrote:
>> On 5 March 2015 at 11:19, Laszlo Ersek <ler...@redhat.com> wrote:
>>> The stanza for the [Components.common] section of
>>> "ArmVirtualization.dsc.inc" originates from under OvmfPkg.
>>>
>>
>> Could we have a rationale in the commit log? I can't remember what it was :-)
>
> I don't understand.
>
> What rationale do you need to be *reminded of* for not including a
> prebuilt executable in your hand-built firmware binary? :)
>
> Including a prebuilt executable is inexcusable practice, especially
> given that the source code of the UEFI shell resides in the same tree.
> It's always been wrong (*), both technically (= if you patch the shell,
> you won't see the results) and "ideologically" (= because your
> built-from-source firmware is not in fact built from source). We might
> want to mention "security" too.
>
> (*) I realize the relevant code in the DSC & FDF files originates from
> other ArmPlatformPkg DSC & FDF files; they are wrong too.
>

OK, this was my original motivation for doing it as well: not
deviating from the ARM platform DSCs. But actually, I think it makes
much more sense to align with Ovmf in this respect.

So

Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org>

I still think the commit log is un-Laszlo like in the sense that it
contains only a single sentence.
I would have expected at least one diagram there :-)

But seriously, I do think it makes sense putting some rationale in the
commit log, considering that this is a policy decision and the code
changes don't speak for themselves.

Regards,
Ard.


It would still mak
> So, would you like me to add the above to the commit message, or
> something else? I thought the reasons were obvious.
>
> (And, indeed, I'm not forgetting about the miserable FAT driver either.
> I have absolutely no idea why that driver has to live in a separate
> repo, presenting upstream developers with technical hurdles when they
> want to build their entire tree from source. Our downstreams embed
> FatPkg in the tree and build it from source, together with the rest.
> (Yes, we pay attention to the license.))
>
> Cheers
> Laszlo
>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
>>> ---
>>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc | 26 
>>> ++++++++++++++++++++
>>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.fdf |  2 +-
>>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.fdf  |  2 +-
>>>  3 files changed, 28 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc 
>>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>> index 4e30e9f..51c1632 100644
>>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>> @@ -339,3 +339,29 @@
>>>    MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
>>>    MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>    MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>>> +
>>> +  #
>>> +  # UEFI application (Shell Embedded Boot Loader)
>>> +  #
>>> +  ShellPkg/Application/Shell/Shell.inf {
>>> +    <LibraryClasses>
>>> +      
>>> ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
>>> +      
>>> NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>>> +      
>>> NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>>> +      
>>> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
>>> +      
>>> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>>> +      
>>> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>>> +      
>>> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>>> +      
>>> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>>> +      
>>> HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
>>> +      ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
>>> +      FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
>>> +      SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
>>> +      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>>> +      
>>> BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
>>> +
>>> +    <PcdsFixedAtBuild>
>>> +      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
>>> +      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>>> +      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
>>> +  }
>>> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.fdf 
>>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.fdf
>>> index 235d812..1f5d530 100644
>>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.fdf
>>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.fdf
>>> @@ -155,7 +155,7 @@ READ_LOCK_STATUS   = TRUE
>>>    #
>>>    # UEFI application (Shell Embedded Boot Loader)
>>>    #
>>> -  INF ShellBinPkg/UefiShell/UefiShell.inf
>>> +  INF ShellPkg/Application/Shell/Shell.inf
>>>
>>>    #
>>>    # Bds
>>> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.fdf 
>>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.fdf
>>> index 07ae02a..270a14c 100644
>>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.fdf
>>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.fdf
>>> @@ -151,7 +151,7 @@ READ_LOCK_STATUS   = TRUE
>>>    #
>>>    # UEFI application (Shell Embedded Boot Loader)
>>>    #
>>> -  INF ShellBinPkg/UefiShell/UefiShell.inf
>>> +  INF ShellPkg/Application/Shell/Shell.inf
>>>
>>>    #
>>>    # Bds
>>> --
>>> 1.8.3.1
>>>
>>>
>

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to