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.151, mailfrom: liming.gao@intel.com) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by groups.io with SMTP; Wed, 07 Aug 2019 04:19:14 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Aug 2019 04:19:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,357,1559545200"; d="scan'208";a="174475675" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga008.fm.intel.com with ESMTP; 07 Aug 2019 04:19:13 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 7 Aug 2019 04:19:13 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX154.amr.corp.intel.com (10.18.116.70) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 7 Aug 2019 04:19:13 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.112]) by shsmsx102.ccr.corp.intel.com ([169.254.2.19]) with mapi id 14.03.0439.000; Wed, 7 Aug 2019 19:19:11 +0800 From: "Liming Gao" To: Leif Lindholm , "devel@edk2.groups.io" CC: "Feng, Bob C" , "Fan, ZhijuX" , Max Knutsen , "Philippe Mathieu-Daude" , Andrew Fish , "Laszlo Ersek" , "Kinney, Michael D" Subject: Re: Microsoft imports - was Re: [edk2-devel] [PATCH V2] BaseTools:Add extra debugging message Thread-Topic: Microsoft imports - was Re: [edk2-devel] [PATCH V2] BaseTools:Add extra debugging message Thread-Index: AQHVQpV0voSBS+8oKE2vvA2pji/BHqblu3aAgACQi9CAANlxAIAH0lMA Date: Wed, 7 Aug 2019 11:19:11 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E4CBD89@SHSMSX104.ccr.corp.intel.com> References: <20190725030217.16596-1-zhijux.fan@intel.com> <08650203BA1BD64D8AD9B6D5D74A85D160B513D3@SHSMSX105.ccr.corp.intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E4C776D@SHSMSX104.ccr.corp.intel.com> <20190802095506.GS22656@bivouac.eciton.net> In-Reply-To: <20190802095506.GS22656@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: liming.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Leif: >-----Original Message----- >From: Leif Lindholm [mailto:leif.lindholm@linaro.org] >Sent: Friday, August 02, 2019 5:55 PM >To: devel@edk2.groups.io >Cc: Feng, Bob C ; Fan, ZhijuX ; >Max Knutsen ; Philippe Mathieu-Daude >; Andrew Fish ; Laszlo Ersek >; Kinney, Michael D ; >Gao, Liming >Subject: Microsoft imports - was Re: [edk2-devel] [PATCH V2] BaseTools:Ad= d >extra debugging message > >So, I'm not going to give any of the reviewers a hard time about this >- the patch *looks* right and we've all occasionally given R-b on >things we haven't actually tested because we don't always have the >time. And by the time I hit it, it had already been fixed upstream. > >But what worries me is that not only was this an error that would have >been caught by a simple build test - it was imported from an external >project where the same would have applied. I double check the code. Original code is good. This is a patch sync issue= .=20 > >Chucking patched over the wall from another open source project is no >improvement over chucking internal patches over the wall without >proper (contributor) review or testing. Yes. The developer unit test is important.=20 > >Or was it modified on the way across? > >One change I would like to see enacted *immediately* is that *any* >imports from other open source projects state the repository and the >commit hash that it originated from. In the commit message - and where >BZs are referenced in the message, also copied into the BZ. > Good suggestion. I will update BZ to include those information.=20 Thanks Liming >/ > Leif > >On Thu, Aug 01, 2019 at 12:57:36PM +0000, Liming Gao wrote: >> Good catch. I have pushed this patch. Can you send one new patch to fix= it? >> >> > -----Original Message----- >> > From: Feng, Bob C >> > Sent: Thursday, August 1, 2019 8:20 PM >> > To: devel@edk2.groups.io; Fan, ZhijuX >> > Cc: Max Knutsen ; Gao, Liming > >> > Subject: RE: [edk2-devel] [PATCH V2] BaseTools:Add extra debugging >message >> > >> > Hi Zhiju, >> > >> > There is a typo in this patch. See it inline. >> > >> > Thanks, >> > Bob >> > >> > -----Original Message----- >> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Fan, ZhijuX >> > Sent: Thursday, July 25, 2019 11:02 AM >> > To: devel@edk2.groups.io >> > Cc: Max Knutsen ; Feng, Bob C >; Gao, Liming ; Fan, ZhijuX >> > >> > Subject: [edk2-devel] [PATCH V2] BaseTools:Add extra debugging >message >> > >> > From: Max Knutsen >> > >> > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2014 >> > >> > Add extra debugging to improve error identification. >> > Error while processing file if the file is read incorrectly >> > >> > This patch is going to fix that issue. >> > >> > Cc: Bob Feng >> > Cc: Liming Gao >> > Signed-off-by: Zhiju.Fan >> > --- >> > BaseTools/Source/Python/AutoGen/StrGather.py | 16 ++++++++++------ >> > BaseTools/Source/Python/Trim/Trim.py | 4 +++- >> > 2 files changed, 13 insertions(+), 7 deletions(-) >> > >> > diff --git a/BaseTools/Source/Python/AutoGen/StrGather.py >b/BaseTools/Source/Python/AutoGen/StrGather.py >> > index 2e4671a433..eed30388be 100644 >> > --- a/BaseTools/Source/Python/AutoGen/StrGather.py >> > +++ b/BaseTools/Source/Python/AutoGen/StrGather.py >> > @@ -526,12 +526,16 @@ def SearchString(UniObjectClass, FileList, >IsCompatibleMode): >> > return UniObjectClass >> > >> > for File in FileList: >> > - if os.path.isfile(File): >> > - Lines =3D open(File, 'r') >> > - for Line in Lines: >> > - for StrName in STRING_TOKEN.findall(Line): >> > - EdkLogger.debug(EdkLogger.DEBUG_5, "Found string= identifier: >" + StrName) >> > - UniObjectClass.SetStringReferenced(StrName) >> > + try: >> > + if os.path.isfile(File): >> > + Lines =3D open(File, 'r') >> > + for Line in Lines: >> > + for StrName in STRING_TOKEN.findall(Line): >> > + EdkLogger.debug(EdkLogger.DEBUG_5, "Found st= ring >identifier: " + StrName) >> > + UniObjectClass.SetStringReferenced(StrName) >> > + except: >> > + EdkLogger.error("UnicodeStringGather", AUTOGEN_ERROR, >"SearchString: Error while processing file", File=3DFile, >> > RaiseError=3DFalse) >> > + raise >> > >> > UniObjectClass.ReToken() >> > >> > diff --git a/BaseTools/Source/Python/Trim/Trim.py >b/BaseTools/Source/Python/Trim/Trim.py >> > index 43119bd7ff..8767b67f7e 100644 >> > --- a/BaseTools/Source/Python/Trim/Trim.py >> > +++ b/BaseTools/Source/Python/Trim/Trim.py >> > @@ -73,8 +73,10 @@ def TrimPreprocessedFile(Source, Target, >ConvertHex, TrimLong): >> > try: >> > with open(Source, "r") as File: >> > Lines =3D File.readlines() >> > - except: >> > + except IOError: >> > EdkLogger.error("Trim", FILE_OPEN_FAILURE, ExtraData=3DSourc= e) >> > + expect: >> > >> > Typo here. expect should except >> > >> > + EdkLogger.error("Trim", AUTOGEN_ERROR, "TrimPreprocessedFile= : >Error while processing file", File=3DSource) >> > >> > PreprocessedFile =3D "" >> > InjectedFile =3D "" >> > -- >> > 2.14.1.windows.1 >> > >> > >> > >> >> >>=20 >>