Re: [Patch V2 0/7] Reproduce builds across source format changes

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Liming,

RELEASE in tools_def.txt only means disable symbols.
It does not mean disable DEBUG() and ASSERT().
Some platforms use DEBUG/RELEASE build type to control
DEBUG() and ASSERT(), but that is not required.

In order to do binary build comparison, I am actually having to
use NOOPT builds because optimizing builds add extra information
to the objs and libs that seems to include line number related
information.  I do want to make sure all the logic associated
with use of DEBUG and ASSERT macros are not changed from a 
source format change.  So we actually want NOOPT builds with all
DEBUG and ASSERT macros enabled.

I have a python helper script to do a build comparison.  I will
be sending that for review tomorrow.

Mike


> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Monday, November 1, 2021 7:31 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: 'Ard Biesheuvel' <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; 'Gerd Hoffmann' <kraxel@redhat.com>; 'Michael Kubacki' <michael.kubacki@microsoft.com>; Wang,
> Jian J <jian.j.wang@intel.com>; 'Maciej Rabeda' <maciej.rabeda@linux.intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>; 'Leif Lindholm' <leif@nuviainc.com>; Liu, Zhiguang <zhiguang.liu@intel.com>
> Subject: 回复: [Patch V2 0/7] Reproduce builds across source format changes
> 
> Mike：
>   Seemly, this change is for debug image build reproduce. But, if the fixed
> value is used, they are not correct for the debug image. If so, can RELEASE
> image be used for the binary comparison?
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Michael D Kinney <michael.d.kinney@intel.com>
> > 发送时间: 2021年11月2日 5:38
> > 收件人: devel@edk2.groups.io
> > 抄送: Ard Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao
> > <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd
> > Hoffmann <kraxel@redhat.com>; Michael Kubacki
> > <michael.kubacki@microsoft.com>; Jian J Wang <jian.j.wang@intel.com>;
> > Maciej Rabeda <maciej.rabeda@linux.intel.com>; Jiaxin Wu
> > <jiaxin.wu@intel.com>; Siyuan Fu <siyuan.fu@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Leif Lindholm <leif@nuviainc.com>; Zhiguang
> > Liu <zhiguang.liu@intel.com>
> > 主题: [Patch V2 0/7] Reproduce builds across source format changes
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688
> >
> > New in V2
> > ------------
> > * Split MdePkg DebugLib class enhancements out into its own commit
> > * Address ECC failures
> > * Align macro continuation marks in same column
> >
> > This change is required to help verify that source code formatting
> > changes such as the use of uncrustify and line ending corrections
> > do not have any functional differences.  Source format changes may
> > add or remove line endings that change the source file line numbers
> > of C statements or may change the use of spaces in C expressions
> > used in an ASSERT() statements.  These types of changes can impact
> > the generated binaries when DEBUG() and ASSERT() macros are
> > enabled.  The following set of changes adds 2 defines that can be used
> > to override the use of `__LINE__` in DEBUG() macros and the use of
> > #Expression in ASSERT() macros.
> >
> > * Add DEBUG_LINE_NUMBER define to DebugLib.h that is
> >   by default mapped to `__LINE__`.  A build can pre-define
> >   DEBUG_LINE_NUMBER to use a fixed value.
> > * Add DEBUG_EXPRESSION_STRING(Expression) macros to
> >   DebugLib.h that is by default mapped to #Expression.
> >   A build can define DEBUG_EXPRESSION_STRING_VALUE to
> >   set all expression strings to a fixed string value.
> > * Use DEBUG_LINE_NUMBER instead of `__LINE__`.
> > * Use DEBUG_EXPRESSION_STRING instead of #Expression.
> >
> > Submodules that use `__LINE__` are not updated.  These do not
> > currently impact build reproducibility unless the debug features
> > of those submodules are enabled.
> >
> > The one exception is the UnitTestFrameworkPkg cmocka submodule
> > that uses `__LINE__`.  This means that the binaries generated by host
> > based unit tests that use cmocka features may not be identical across
> > a source format change.
> >
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > Michael D Kinney (7):
> >   MdePkg/Include: Enhance DebugLib to support reproduce builds
> >   MdePkg: Reproduce builds across source format changes
> >   ArmPkg: Reproduce builds across source format changes
> >   MdeModulePkg: Reproduce builds across source format changes
> >   NetworkPkg: Reproduce builds across source format changes
> >   SecurityPkg: Reproduce builds across source format changes
> >   OvmfPkg: Reproduce builds across source format changes
> >
> >  .../ProcessorSubClassDxe/ProcessorSubClass.c  |  2 +-
> >  .../Type00/MiscBiosVendorFunction.c           |  2 +-
> >  .../Type01/MiscSystemManufacturerFunction.c   |  2 +-
> >  .../MiscBaseBoardManufacturerFunction.c       |  2 +-
> >  .../Type03/MiscChassisManufacturerFunction.c  |  2 +-
> >  ...MiscNumberOfInstallableLanguagesFunction.c |  2 +-
> >  .../Type32/MiscBootInformationFunction.c      |  2 +-
> >  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c  |  2 +-
> >  .../UefiBootManagerLib/BmDriverHealth.c       |  2 +-
> >  MdePkg/Include/Library/DebugLib.h             | 43
> > +++++++++++++++--
> >  MdePkg/Include/Library/UnitTestLib.h          | 46 +++++++++----------
> >  MdePkg/Library/BaseLib/SafeString.c           |  2 +-
> >  NetworkPkg/Include/Library/NetLib.h           |  8 ++--
> >  NetworkPkg/Library/DxeNetLib/DxeNetLib.c      |  2 +-
> >  OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c         |  6 +--
> >  .../PlatformBootManagerLib/BdsPlatform.c      |  4 +-
> >  .../PlatformBootManagerLibBhyve/BdsPlatform.c |  2 +-
> >  .../PlatformBootManagerLibGrub/BdsPlatform.c  |  4 +-
> >  .../Include/Library/TcgStorageCoreLib.h       | 26 +++++------
> >  19 files changed, 98 insertions(+), 63 deletions(-)
> >
> > --
> > 2.32.0.windows.1
> 
>