* [PATCH] BaseTools:Makefiles cause data errors on some platforms
@ 2019-04-17 2:02 Fan, ZhijuX
2019-04-17 10:22 ` Bob Feng
2019-04-17 13:23 ` [edk2-devel] " Laszlo Ersek
0 siblings, 2 replies; 4+ messages in thread
From: Fan, ZhijuX @ 2019-04-17 2:02 UTC (permalink / raw)
To: devel@edk2.groups.io; +Cc: Gao, Liming, Feng, Bob C
[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]
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.
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 = []
--
2.14.1.windows.1
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 3904 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] BaseTools:Makefiles cause data errors on some platforms
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 ` [edk2-devel] " Laszlo Ersek
1 sibling, 0 replies; 4+ messages in thread
From: Bob Feng @ 2019-04-17 10:22 UTC (permalink / raw)
To: Fan, ZhijuX, devel@edk2.groups.io; +Cc: Gao, Liming
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
-----Original Message-----
From: Fan, ZhijuX
Sent: Wednesday, April 17, 2019 10:02 AM
To: devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
Subject: [PATCH] BaseTools:Makefiles cause data errors on some platforms
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.
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 = []
--
2.14.1.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH] BaseTools:Makefiles cause data errors on some platforms
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
2019-04-18 2:03 ` Bob Feng
1 sibling, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2019-04-17 13:23 UTC (permalink / raw)
To: devel, zhijux.fan, Feng, Bob C; +Cc: Gao, Liming
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 = []
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH] BaseTools:Makefiles cause data errors on some platforms
2019-04-17 13:23 ` [edk2-devel] " Laszlo Ersek
@ 2019-04-18 2:03 ` Bob Feng
0 siblings, 0 replies; 4+ messages in thread
From: Bob Feng @ 2019-04-18 2:03 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io, Fan, ZhijuX; +Cc: Gao, Liming
Hi Laszlo,
Yes. The commit message is very important. I'll care more on the commit message in future code review.
For this patch I'll ask submitter to enter a BZ and refine this patch commit message.
Thanks,
Bob
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Wednesday, April 17, 2019 9:23 PM
To: devel@edk2.groups.io; Fan, ZhijuX <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
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 = []
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-18 2:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [edk2-devel] " Laszlo Ersek
2019-04-18 2:03 ` Bob Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox