From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, zhijux.fan@intel.com, "Feng,
Bob C" <bob.c.feng@intel.com>
Cc: "Gao, Liming" <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools:Makefiles cause data errors on some platforms
Date: Wed, 17 Apr 2019 15:23:03 +0200 [thread overview]
Message-ID: <af451a5a-354d-ab5d-3c45-c3ace52c15c1@redhat.com> (raw)
In-Reply-To: <FAD0D7E0AE0FA54D987F6E72435CAFD50AF4BD30@SHSMSX101.ccr.corp.intel.com>
Hi,
On 04/17/19 04:02, Fan, ZhijuX wrote:
> As the Dict is unordered, an error occurs when using the Dict
> while 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
helpful, it's just that the subject lines were chosen in an unfortunate way.
Bob: please enforce higher standards for commit messages under
BaseTools. People *care*. (I certainly do.) Good commit messages teach
details about the subsystem or module.
Thanks,
Laszlo
>
> 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/GenMake.py | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 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 = []
> self.FileListMacros = {}
> self.ListFileMacros = {}
> - self.ObjTargetDict = {}
> + self.ObjTargetDict = OrderedDict()
> self.FileCache = {}
> self.LibraryBuildCommandList = []
> self.LibraryFileList = []
> @@ -954,11 +954,9 @@ cleanlib:
> self.ListFileMacros[T.IncListFileMacro] = []
> if self._AutoGenObject.BuildRuleFamily == TAB_COMPILER_MSFT and Type == TAB_C_CODE_FILE:
> NewFile = 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] = set()
> - self.ObjTargetDict[T.Target.SubDir].add(NewFile)
> + self.ObjTargetDict[T.Target.SubDir].add(NewFile)
>
> Deps = []
> CCodeDeps = []
>
next prev parent reply other threads:[~2019-04-17 13:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-17 2:02 [PATCH] BaseTools:Makefiles cause data errors on some platforms Fan, ZhijuX
2019-04-17 10:22 ` Bob Feng
2019-04-17 13:23 ` Laszlo Ersek [this message]
2019-04-18 2:03 ` [edk2-devel] " Bob Feng
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=af451a5a-354d-ab5d-3c45-c3ace52c15c1@redhat.com \
--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