Re: [edk2-devel] [PATCH 1/1] OvmfPkg: Only import DxeTpmMeasurementLib when TPM is enabled

["Gary Lin" <glin@suse.com>]

On Thu, Jul 04, 2019 at 10:33:55AM +0200, Laszlo Ersek wrote:
> On 07/04/19 05:58, Gary Lin wrote:
> > On Wed, Jul 03, 2019 at 09:49:26PM +0200,  Laszlo Ersek  wrote:
> >> Hi Gary,
> >>
> >> On 07/03/19 12:22, Gary Lin wrote:
> >>> DxeTpmMeasurementLib is only useful when TPM is enabled.
> >>>
> >>> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>> Cc: Stefan Berger <stefanb@linux.ibm.com>
> >>> Signed-off-by: Gary Lin <glin@suse.com>
> >>> ---
> >>>  OvmfPkg/OvmfPkgIa32.dsc    | 10 +++++++---
> >>>  OvmfPkg/OvmfPkgIa32X64.dsc | 10 +++++++---
> >>>  OvmfPkg/OvmfPkgX64.dsc     | 10 +++++++---
> >>>  3 files changed, 21 insertions(+), 9 deletions(-)
> >>
> >> This is a good patch, thank you for it. I see two opportunities for
> >> improvement.
> >>
> >> (1) There's something weird going on with your newline characters. The
> >> view I get (in both my INBOX and in my list folder) is identical to
> >> mail-archive.com's view:
> >>
> >>   http://mid.mail-archive.com/20190703102228.25441-1-glin@suse.com
> >>
> >> Can you double check your settings, please?
> >>
> > I didn't change my git settings except the mail server due to our
> > recent server migration. Not sure if it's caused by the new mail server
> > or not...
> 
> Not sure... your v2 on the list doesn't seem to suffer from this issue,
> thankfully :)
> 
I actually strip CRs manually with dos2unix before sending the patch.
It seems that my old server strip CRs automatically but the new server
tends to keep it. When I viewed my first patch with mutt, the viewer
ignored CRs so I didn't notice the newline problem. Then I sent the
patch to a colleague who is using thunderbird, all CRs become LFs.

This is really annoying :-(

Gary Lin

> > 
> >>
> >> (2) The commit message should be more convincing. How about this:
> >>
> > Will follow your suggestion to update the patch.
> > 
> > BTW, just found that there is a TPM2_ENABLE block below the
> > SECURE_BOOT_ENABLE block. I'll move TpmMeasurementLib there to reduce
> > the lines of change.
> 
> Good idea!
> 
> Thanks
> Laszlo
>