Re: [edk2-devel] [PATCH 1/2] Features/Intel/AdvancedFeaturePkg: Remove temporary build workaround

["Michael Kubacki" <michael.kubacki@outlook.com>]

I think the series cover letter should have been filled out but this 
seems reasonable to me for removal of the temporary workaround.

Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 4/23/2020 1:42 AM, Heng Luo wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2688
> 
> Remove Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround,
> Add Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> to support Feature PCDs.
> 
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Heng Luo <heng.luo@intel.com>
> ---
>   Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc                                |  2 +-
>   Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc                       | 41 +++++++++++++++++++++++++++++++++++++++++
>   Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.c   | 31 -------------------------------
>   Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.dsc | 76 ----------------------------------------------------------------------------
>   Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.inf | 60 ------------------------------------------------------------
>   5 files changed, 42 insertions(+), 168 deletions(-)
> 
> diff --git a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> index ea879680ba..e509ef3e1b 100644
> --- a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> +++ b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> @@ -30,7 +30,7 @@
>     PEI_ARCH                            = IA32
> 
>     DXE_ARCH                            = X64
> 
>   
> 
> -!include AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.dsc
> 
> +!include AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> 
>   
> 
>   ################################################################################
> 
>   #
> 
> diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> new file mode 100644
> index 0000000000..2eacec18a0
> --- /dev/null
> +++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> @@ -0,0 +1,41 @@
> +## @file
> 
> +#  DSC file for defining Pcd of advanced features.
> 
> +#
> 
> +#  This file is intended to be included into another package so advanced features
> 
> +#  can be conditionally built by enabling the respective feature via its FeaturePCD.
> 
> +#
> 
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> 
> +#
> 
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +#
> 
> +##
> 
> +
> 
> +#
> 
> +# The section references the package DEC files,
> 
> +# it allow a FeaturePCD to be used in a conditional statement
> 
> +#
> 
> +[Packages]
> 
> +  MdePkg/MdePkg.dec
> 
> +  AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
> 
> +  Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dec
> 
> +  NetworkFeaturePkg/NetworkFeaturePkg.dec
> 
> +  IpmiFeaturePkg/IpmiFeaturePkg.dec
> 
> +  S3FeaturePkg/S3FeaturePkg.dec
> 
> +  SmbiosFeaturePkg/SmbiosFeaturePkg.dec
> 
> +  UserAuthFeaturePkg/UserAuthFeaturePkg.dec
> 
> +  LogoFeaturePkg/LogoFeaturePkg.dec
> 
> +
> 
> +#
> 
> +# The section below sets all PCDs to FALSE in this DSC file so the feature is not enabled by default.
> 
> +# Board can set PCDs to TRUE in its DSC file to enable a subset of advanced features
> 
> +#
> 
> +[PcdsFeatureFlag]
> 
> +  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable            |FALSE
> 
> +  gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable                      |FALSE
> 
> +  gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable                |FALSE
> 
> +  gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable                          |FALSE
> 
> +  gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable                  |FALSE
> 
> +  gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable            |FALSE
> 
> +  gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable    |FALSE
> 
> +  gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable                      |FALSE
> 
> +  gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable                              |FALSE
> 
> diff --git a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.c b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.c
> deleted file mode 100644
> index 2cd91b06f0..0000000000
> --- a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.c
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/** @file
> 
> -  Source code file for a temporary build workaround.
> 
> -
> 
> -  The purpose of this workaround is described in the module INF file.
> 
> -
> 
> -Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> 
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -
> 
> -**/
> 
> -
> 
> -#include <Base.h>
> 
> -#include <Library/BaseLib.h>
> 
> -
> 
> -/**
> 
> -  An empty entry point function.
> 
> -
> 
> -  @param  FileHandle  Handle of the file being invoked.
> 
> -  @param  PeiServices Describes the list of possible PEI Services.
> 
> -
> 
> -  @retval  EFI_SUCCESS  This function always returns EFI_SUCCESS.
> 
> -
> 
> -**/
> 
> -EFI_STATUS
> 
> -EFIAPI
> 
> -TemporaryBuildWorkaroundEntry (
> 
> -  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> 
> -  IN CONST EFI_PEI_SERVICES     **PeiServices
> 
> -  )
> 
> -{
> 
> -  return EFI_SUCCESS;
> 
> -}
> 
> diff --git a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.dsc b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.dsc
> deleted file mode 100644
> index c62f9ecc6e..0000000000
> --- a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.dsc
> +++ /dev/null
> @@ -1,76 +0,0 @@
> -## @file
> 
> -# Build description file for a temporary build workaround.
> 
> -#
> 
> -# The feature enable PCD for advanced features must be referenced in an INF
> 
> -# to be referenced in DSC/FDF files. This DSC only exists in the build to
> 
> -# allow the PCDs to be referenced. This workaround does not affect the final
> 
> -# flash image or boot in any way.
> 
> -#
> 
> -# The request to update BaseTools to allow a PCD to be referenced in DSC/FDF
> 
> -# files without requiring the PCD to be referenced in an INF file is tracked
> 
> -# here: https://bugzilla.tianocore.org/show_bug.cgi?id=2270
> 
> -#
> 
> -# When the BaseTools update is complete, this file can entirely be removed
> 
> -# from this package.
> 
> -#
> 
> -# Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
> 
> -#
> 
> -# SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -#
> 
> -##
> 
> -
> 
> -#
> 
> -# BEGIN:Temporary Build Workaround (resolution: https://bugzilla.tianocore.org/show_bug.cgi?id=2270)
> 
> -#
> 
> -
> 
> -# THIS FILE IS TEMPORARY. PLEASE TRY TO LOOK PAST THE "HACKS" ASSOCIATED WITH IT.
> 
> -#
> 
> -# With the BaseTools change requested, the changes needed to move to the end state are simply:
> 
> -#  1. Remove the !include for this file in AdvancedFeatures.dsc
> 
> -#  2. Remove this directory
> 
> -
> 
> -!if $(PLATFORM_NAME) != AdvancedFeaturePkg
> 
> -#
> 
> -# AdvancedFeaturePkg initializes all FeaturePCDs to TRUE so they can conveniently be built in one package.
> 
> -# Board packages will normally only enable (and therefore reference) a small subset of advanced features
> 
> -# relative to the board. If an INF does not reference a package DEC file (which will be the case if the
> 
> -# feature is not enabled) then the DSC must set ("define") the PCD for the conditional statements based
> 
> -# on the PCD to work.
> 
> -#
> 
> -# AdvancedFeaturePkg has no problem as it naturally has a need to set al PCDs to TRUE for build.
> 
> -# The section below sets all PCDs to FALSE in the DSC file so if the feature is not enabled by a board,
> 
> -# the build will still be successful.
> 
> -#
> 
> -[PcdsFeatureFlag]
> 
> -  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable            |FALSE
> 
> -  gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable                      |FALSE
> 
> -  gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable                |FALSE
> 
> -  gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable                          |FALSE
> 
> -  gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable                  |FALSE
> 
> -  gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable            |FALSE
> 
> -  gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable    |FALSE
> 
> -  gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable                      |FALSE
> 
> -  gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable                              |FALSE
> 
> -!endif
> 
> -
> 
> -#
> 
> -# The LibraryClasses required to build TemporaryBuildWorkaround.inf
> 
> -# (mostly libraries requiring other libraries)
> 
> -#
> 
> -[LibraryClasses]
> 
> -  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> 
> -  BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> 
> -  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> 
> -  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> 
> -  PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> 
> -  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> 
> -  PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
> 
> -
> 
> -#
> 
> -# The driver that references all feature PCDs to satsify current build limitations
> 
> -#
> 
> -[Components]
> 
> -  AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.inf
> 
> -#
> 
> -# END:Temporary Build Workaround (resolution: https://bugzilla.tianocore.org/show_bug.cgi?id=2270)
> 
> -#
> 
> diff --git a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.inf b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.inf
> deleted file mode 100644
> index 00818fbe0a..0000000000
> --- a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.inf
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -### @file
> 
> -# Component information file for a temporary build workaround.
> 
> -#
> 
> -# The feature enable PCD for this package must be referenced in an INF to be
> 
> -# referenced in DSC/FDF files. This driver is only included in the build to
> 
> -# allow the PCD to be referenced. This driver is not included in the flash
> 
> -# image and does not affect the boot in any way.
> 
> -#
> 
> -# The request to update BaseTools to allow a PCD to be referenced in DSC/FDF
> 
> -# files without requiring the PCD to be referenced in an INF file is tracked
> 
> -# here: https://bugzilla.tianocore.org/show_bug.cgi?id=2270
> 
> -#
> 
> -# When the BaseTools update is complete, this file can entirely be removed
> 
> -# from this package.
> 
> -#
> 
> -# Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
> 
> -#
> 
> -# SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -#
> 
> -###
> 
> -
> 
> -[Defines]
> 
> -  INF_VERSION       = 0x00010017
> 
> -  BASE_NAME         = TemporaryBuildWorkaround
> 
> -  FILE_GUID         = 8846A81E-F552-4917-81F5-80B62E4EFBAC
> 
> -  VERSION_STRING    = 1.0
> 
> -  MODULE_TYPE       = PEIM
> 
> -  ENTRY_POINT       = TemporaryBuildWorkaroundEntry
> 
> -
> 
> -[LibraryClasses]
> 
> -  BaseLib
> 
> -  PeimEntryPoint
> 
> -
> 
> -[Packages]
> 
> -  MdePkg/MdePkg.dec
> 
> -  Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
> 
> -  Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dec
> 
> -  Network/NetworkFeaturePkg/NetworkFeaturePkg.dec
> 
> -  OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
> 
> -  PowerManagement/S3FeaturePkg/S3FeaturePkg.dec
> 
> -  SystemInformation/SmbiosFeaturePkg/SmbiosFeaturePkg.dec
> 
> -  UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec
> 
> -  UserInterface/LogoFeaturePkg/LogoFeaturePkg.dec
> 
> -
> 
> -[FeaturePcd]
> 
> -  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable
> 
> -  gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable
> 
> -  gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable
> 
> -  gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable
> 
> -  gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable
> 
> -  gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable
> 
> -  gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable
> 
> -  gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable
> 
> -  gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable
> 
> -
> 
> -[Sources]
> 
> -  TemporaryBuildWorkaround.c
> 
> -
> 
> -[Depex]
> 
> - TRUE
>