Leif: >-----Original Message----- >From: Leif Lindholm [mailto:leif.lindh...@linaro.org] >Sent: Friday, August 02, 2019 5:55 PM >To: devel@edk2.groups.io >Cc: Feng, Bob C <bob.c.f...@intel.com>; Fan, ZhijuX <zhijux....@intel.com>; >Max Knutsen <maknu...@microsoft.com>; Philippe Mathieu-Daude ><phi...@redhat.com>; Andrew Fish <af...@apple.com>; Laszlo Ersek ><ler...@redhat.com>; Kinney, Michael D <michael.d.kin...@intel.com>; >Gao, Liming <liming....@intel.com> >Subject: Microsoft imports - was Re: [edk2-devel] [PATCH V2] BaseTools:Add >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. > >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. > >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. 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 <zhijux....@intel.com> >> > Cc: Max Knutsen <maknu...@microsoft.com>; Gao, Liming ><liming....@intel.com> >> > 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 <maknu...@microsoft.com>; Feng, Bob C ><bob.c.f...@intel.com>; Gao, Liming <liming....@intel.com>; Fan, ZhijuX >> > <zhijux....@intel.com> >> > Subject: [edk2-devel] [PATCH V2] BaseTools:Add extra debugging >message >> > >> > From: Max Knutsen <maknu...@microsoft.com> >> > >> > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2014 >> > >> > 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 <bob.c.f...@intel.com> >> > Cc: Liming Gao <liming....@intel.com> >> > Signed-off-by: Zhiju.Fan <zhijux....@intel.com> >> > --- >> > 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 = 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 = 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) >> > + except: >> > + EdkLogger.error("UnicodeStringGather", AUTOGEN_ERROR, >"SearchString: Error while processing file", File=File, >> > RaiseError=False) >> > + 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 = File.readlines() >> > - except: >> > + except IOError: >> > EdkLogger.error("Trim", FILE_OPEN_FAILURE, ExtraData=Source) >> > + expect: >> > >> > Typo here. expect should except >> > >> > + EdkLogger.error("Trim", AUTOGEN_ERROR, "TrimPreprocessedFile: >Error while processing file", File=Source) >> > >> > PreprocessedFile = "" >> > InjectedFile = "" >> > -- >> > 2.14.1.windows.1 >> > >> > >> > >> >> >> >> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44997): https://edk2.groups.io/g/devel/message/44997 Mute This Topic: https://groups.io/mt/32688980/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
