From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: devel@edk2.groups.io
Cc: "Feng, Bob C" <bob.c.feng@intel.com>,
"Fan, ZhijuX" <zhijux.fan@intel.com>,
Max Knutsen <maknutse@microsoft.com>,
Philippe Mathieu-Daude <philmd@redhat.com>,
Andrew Fish <afish@apple.com>, Laszlo Ersek <lersek@redhat.com>,
Michael D Kinney <michael.d.kinney@intel.com>,
Liming Gao <liming.gao@intel.com>
Subject: Microsoft imports - was Re: [edk2-devel] [PATCH V2] BaseTools:Add extra debugging message
Date: Fri, 2 Aug 2019 10:55:06 +0100 [thread overview]
Message-ID: <20190802095506.GS22656@bivouac.eciton.net> (raw)
In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E4C776D@SHSMSX104.ccr.corp.intel.com>
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.
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.
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.
/
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.fan@intel.com>
> > Cc: Max Knutsen <maknutse@microsoft.com>; Gao, Liming <liming.gao@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 <maknutse@microsoft.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Fan, ZhijuX
> > <zhijux.fan@intel.com>
> > Subject: [edk2-devel] [PATCH V2] BaseTools:Add extra debugging message
> >
> > From: Max Knutsen <maknutse@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.feng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Zhiju.Fan <zhijux.fan@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
> >
> >
> >
>
>
>
>
next prev parent reply other threads:[~2019-08-02 9:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-25 3:02 [PATCH V2] BaseTools:Add extra debugging message Fan, ZhijuX
2019-07-25 11:04 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-07-26 6:43 ` Liming Gao
2019-08-01 12:19 ` Bob Feng
2019-08-01 12:57 ` Liming Gao
2019-08-02 9:55 ` Leif Lindholm [this message]
2019-08-02 10:06 ` Microsoft imports - was " Philippe Mathieu-Daudé
2019-08-02 21:32 ` rebecca
2019-08-07 11:19 ` Liming Gao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190802095506.GS22656@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox