From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.43, mailfrom: bob.c.feng@intel.com) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by groups.io with SMTP; Wed, 17 Apr 2019 19:03:26 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Apr 2019 19:03:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,364,1549958400"; d="scan'208";a="143838621" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga007.fm.intel.com with ESMTP; 17 Apr 2019 19:03:26 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 17 Apr 2019 19:03:26 -0700 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 17 Apr 2019 19:03:26 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.164]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.21]) with mapi id 14.03.0415.000; Thu, 18 Apr 2019 10:03:23 +0800 From: "Bob Feng" To: Laszlo Ersek , "devel@edk2.groups.io" , "Fan, ZhijuX" CC: "Gao, Liming" Subject: Re: [edk2-devel] [PATCH] BaseTools:Makefiles cause data errors on some platforms Thread-Topic: [edk2-devel] [PATCH] BaseTools:Makefiles cause data errors on some platforms Thread-Index: AdT0wYxaVilvLYBySuuuj4yOysYlVgAHBTaAACqL6HA= Date: Thu, 18 Apr 2019 02:03:23 +0000 Message-ID: <08650203BA1BD64D8AD9B6D5D74A85D1600E43BD@SHSMSX101.ccr.corp.intel.com> References: In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: bob.c.feng@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Laszlo, Yes. The commit message is very important. I'll care more on the commit mes= sage in future code review. For this patch I'll ask submitter to enter a BZ and refine this patch commi= t message.=20 Thanks, Bob -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Wednesday, April 17, 2019 9:23 PM To: devel@edk2.groups.io; Fan, ZhijuX ; Feng, Bob C <= bob.c.feng@intel.com> Cc: Gao, Liming Subject: Re: [edk2-devel] [PATCH] BaseTools:Makefiles cause data errors on = some platforms Hi, On 04/17/19 04:02, Fan, ZhijuX wrote: > As the Dict is unordered, an error occurs when using the Dict while=20 > creating a new item.So for now, use OrdereDict instead of Dict. this problem statement is impenetrable. - What is the exact symptom? - What triggers it, and where? - When we make the dict ordered, why does that mitigate the issue? - Do we have a BZ for this? The subject line of the patch is similarly unhelpful: "data errors on some = platforms". It's basically devoid of information. --*-- Recently I've also seen a number of commits (with different code changes) where the subject line was reused identically.... Yup: 1 2914e8153dd3 BaseTools: Fix corner-cases of --hash feature 2 1b8caf0d87ea BaseTools: Fix corner-cases of --hash feature 3 f2b5e04acad2 BaseTools: Fix corner-cases of --hash feature 4 db4d47fd3ae9 BaseTools: Fix corner-cases of --hash feature What's really sad here is that the commit messages are otherwise pretty hel= pful, it's just that the subject lines were chosen in an unfortunate way. Bob: please enforce higher standards for commit messages under BaseTools. P= eople *care*. (I certainly do.) Good commit messages teach details about th= e subsystem or module. Thanks, Laszlo >=20 > Cc: Bob Feng > Cc: Liming Gao > Signed-off-by: Zhiju.Fan > --- > BaseTools/Source/Python/AutoGen/GenMake.py | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) >=20 > diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py=20 > b/BaseTools/Source/Python/AutoGen/GenMake.py > index 7562dc68b3..426e6ca51a 100644 > --- a/BaseTools/Source/Python/AutoGen/GenMake.py > +++ b/BaseTools/Source/Python/AutoGen/GenMake.py > @@ -429,7 +429,7 @@ cleanlib: > self.CommonFileDependency =3D [] > self.FileListMacros =3D {} > self.ListFileMacros =3D {} > - self.ObjTargetDict =3D {} > + self.ObjTargetDict =3D OrderedDict() > self.FileCache =3D {} > self.LibraryBuildCommandList =3D [] > self.LibraryFileList =3D [] > @@ -954,11 +954,9 @@ cleanlib: > self.ListFileMacros[T.IncListFileMacro] =3D [] > if self._AutoGenObject.BuildRuleFamily =3D=3D TAB_COMPIL= ER_MSFT and Type =3D=3D TAB_C_CODE_FILE: > NewFile =3D self.PlaceMacro(str(T), self.Macros) > - if self.ObjTargetDict.get(T.Target.SubDir): > - self.ObjTargetDict[T.Target.SubDir].add(NewFile) > - else: > + if not self.ObjTargetDict.get(T.Target.SubDir): > self.ObjTargetDict[T.Target.SubDir] =3D set() > - self.ObjTargetDict[T.Target.SubDir].add(NewFile) > + self.ObjTargetDict[T.Target.SubDir].add(NewFile) > =20 > Deps =3D [] > CCodeDeps =3D [] >=20