Re: [edk2-devel] [PATCH V4 04/13] NetworkPkg: Enable MD5 while enable iSCSI

["Gao, Zhichao" <zhichao.gao@intel.com>]

New change has been pushed to my branch: https://github.com/ZhichaoGao/edk2/commits/MD5_disable
Only #4 is changed to adjust the [BuildOptions] location. As there is not other change, so I keep Laszlo's R-B.

Hi Rebeda/Jiaxin/Siyuan,

Can you help to review the #4 and #12 patch?

Thanks,
Zhichao

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Saturday, November 14, 2020 7:47 AM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2-devel] [PATCH V4 04/13] NetworkPkg: Enable MD5 while
> enable iSCSI
> 
> On 11/12/20 06:55, Gao, Zhichao wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3003
> >
> > There is a plan to make MD5 disable as default.
> > The new MACRO ENABLE_MD5_DEPRECATED_INTERFACES would be
> introduced to
> > enable MD5. Make the definition ahead of the change to avoid build
> > error after the MACRO changed.
> > 1. Add the NetworkBuildOptions.dsc.inc to define the MACRO for build
> > (support: GCC, INTEL, MSFT and
> > RVCT)
> > 2. Add the BuildOption file to Network.dsc.inc
> >
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > ---
> >  NetworkPkg/Network.dsc.inc             |  5 ++++-
> >  NetworkPkg/NetworkBuildOptions.dsc.inc | 22 ++++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 1 deletion(-)  create mode 100644
> > NetworkPkg/NetworkBuildOptions.dsc.inc
> >
> > diff --git a/NetworkPkg/Network.dsc.inc b/NetworkPkg/Network.dsc.inc
> > index 16f090a187..6cd0c005fc 100644
> > --- a/NetworkPkg/Network.dsc.inc
> > +++ b/NetworkPkg/Network.dsc.inc
> > @@ -7,7 +7,7 @@
> >  # This file defines one build flag PLATFORMX64_ENABLE to support  #
> > IA32 PEI and X64 DXE platform. Its default value is FALSE.
> >  #
> > -# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2019 - 2020, Intel Corporation. All rights
> > +reserved.<BR>
> >  # Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All
> > rights reserved.<BR>  #
> >  #    SPDX-License-Identifier: BSD-2-Clause-Patent
> > @@ -39,3 +39,6 @@
> >  !include NetworkPkg/NetworkComponents.dsc.inc
> >
> >  !endif
> > +
> > +[BuildOptions]
> > +!include NetworkPkg/NetworkBuildOptions.dsc.inc
> > diff --git a/NetworkPkg/NetworkBuildOptions.dsc.inc
> > b/NetworkPkg/NetworkBuildOptions.dsc.inc
> > new file mode 100644
> > index 0000000000..ce3460d472
> > --- /dev/null
> > +++ b/NetworkPkg/NetworkBuildOptions.dsc.inc
> > @@ -0,0 +1,22 @@
> > +## @file
> > +# Network DSC include file for [BuildOptions] sections of all Architectures.
> > +#
> > +# This file can be included in the [BuildOptions*] section(s) of a
> > +platform # DSC file
> 
> (1) runaway "#" sign between the words "platform" and "DSC"; probably a
> consequence of rewrapping; it should be removed
> 
> > +# by using "!include NetworkPkg/NetworkBuildOptions.dsc.inc", to
> > +specify the C language # feature test macros (eg., API deprecation
> > +macros) according to the flags described # in "NetworkDefines.dsc.inc".
> > +#
> > +# Supported tool chain: "GCC:", "INTEL:", "MSFT:", "RVCT".
> 
> (2) The comment is a bit inconsistent: the GCC, INTEL and MSFT toolchain family
> names include a trailing ":", while RVCT doesn't. The colons
> (":") should be removed.
> 
> (3) These identifiers are not toolchains, but toolchain families.
> (Please check the "BaseTools/Conf/tools_def.template" file for the string
> "_FAMILY".) So the comment should say "Supported toolchain families".
> 
> > +#
> > +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> #
> > +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +!if $(NETWORK_ISCSI_ENABLE) == TRUE
> > +  MSFT:*_*_*_CC_FLAGS = /D ENABLE_MD5_DEPRECATED_INTERFACES
> > +  INTEL:*_*_*_CC_FLAGS = /D ENABLE_MD5_DEPRECATED_INTERFACES
> > +  GCC:*_*_*_CC_FLAGS = -D ENABLE_MD5_DEPRECATED_INTERFACES
> > +  RVCT:*_*_*_CC_FLAGS = -DENABLE_MD5_DEPRECATED_INTERFACES
> > +!endif
> >
> 
> With (1) through (3) fixed:
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> (No need to repost just because of this, if a repost is not otherwise required.
> Then the maintainer that merges the series should please correct the above
> points before pushing.)
> 
> Thanks
> Laszlo