Re: [edk2-devel] [edk2/master PATCHv1 1/1] OvmfPkg: Make SOURCE_DEBUG_ENABLE actually need to be set to TRUE

["Laszlo Ersek" <lersek@redhat.com>]

Hi Peter,

thanks for the patch. Two comments:

On 09/20/19 20:45, Peter Jones wrote:
> Currently some tests check the value of SOURCE_DEBUG_ENABLE, and some
> tests check if it's defined or not.  Additionally, in UefiPayloadPkg as
> well as some other trees, we define it as FALSE in the .dsc file.
> 
> This patch changes all of the Ovmf platforms to explicitly define it as
> FALSE by default, and changes all of the checks to test if the value is
> TRUE.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1

(1) Please drop this line. (I cannot touch up this part for you.)

The Contributed-under line has not been necessary since
<https://bugzilla.tianocore.org/show_bug.cgi?id=1373> was fixed.

> Signed-off-by: Peter Jones <pjones@redhat.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 17 +++++++++--------
>  OvmfPkg/OvmfPkgIa32.fdf    |  2 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc | 19 ++++++++++---------
>  OvmfPkg/OvmfPkgIa32X64.fdf |  2 +-
>  OvmfPkg/OvmfPkgX64.dsc     | 19 ++++++++++---------
>  OvmfPkg/OvmfPkgX64.fdf     |  2 +-
>  OvmfPkg/OvmfXen.dsc        | 17 +++++++++--------
>  OvmfPkg/OvmfXen.fdf        |  2 +-
>  8 files changed, 42 insertions(+), 38 deletions(-)

[...]

> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 785affeb90c8..c7622fcfb7b0 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -260,7 +260,7 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>  
>  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
> -!ifndef $(SOURCE_DEBUG_ENABLE)
> +!if $(SOURCE_DEBUG_ENABLE) != TRUE

(2) Can you please write "== FALSE" here (in the other FDF files too)?
That has a tiny bit more precedent.

I'd be willing to fix up (2) for you just before pushing, but I can't
touch (1).

With (1) and (2) fixed, for v2:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo