* [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
@ 2016-10-13 9:29 Yonghong Zhu
2016-10-13 9:29 ` [Patch] BaseTools: support PCD value to use expression in the DEC file Yonghong Zhu
2016-10-15 20:13 ` [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Jordan Justen
0 siblings, 2 replies; 9+ messages in thread
From: Yonghong Zhu @ 2016-10-13 9:29 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao
This patch updates PatchCheck.py:
1.Update max line length to 70 that describe in wiki
2.remove the two [] when do the char count calculation
3.report error for EFI_D_* macro if it is used, recommend to use
DEBUG_* format.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
---
BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 455c130..d423220 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -1,9 +1,9 @@
## @file
# Check a patch for various format issues
#
-# Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
#
# This program and the accompanying materials are licensed and made
# available under the terms and conditions of the BSD License which
# accompanies this distribution. The full text of the license may be
# found at http://opensource.org/licenses/bsd-license.php
@@ -14,11 +14,11 @@
#
from __future__ import print_function
VersionNumber = '0.1'
-__copyright__ = "Copyright (c) 2015, Intel Corporation All rights reserved."
+__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
import email
import argparse
import os
import re
@@ -195,11 +195,11 @@ class CommitMessageCheck:
if count <= 0:
self.error('Empty commit message!')
return
- if count >= 1 and len(lines[0]) > 76:
+ if count >= 1 and len(lines[0]) > 70:
self.error('First line of commit message (subject line) ' +
'is too long.')
if count >= 1 and len(lines[0].strip()) == 0:
self.error('First line of commit message (subject line) ' +
@@ -208,11 +208,11 @@ class CommitMessageCheck:
if count >= 2 and lines[1].strip() != '':
self.error('Second line of commit message should be ' +
'empty.')
for i in range(2, count):
- if (len(lines[i]) > 76 and
+ if (len(lines[i]) > 70 and
len(lines[i].split()) > 1 and
not lines[i].startswith('git-svn-id:')):
self.error('Line %d of commit message is too long.' % (i + 1))
last_sig_line = None
@@ -354,10 +354,12 @@ class GitDiffCheck:
line)
if '\t' in line:
self.added_line_error('Tab character used', line)
if len(stripped) < len(line):
self.added_line_error('Trailing whitespace found', line)
+ if 'EFI_D_' in line:
+ self.added_line_error('EFI_D_* used, Please use DEBUG_* format', line)
split_diff_re = re.compile(r'''
(?P<cmd>
^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
)
@@ -471,11 +473,20 @@ class CheckOnePatch:
self.commit_msg = mo.group('commit_message')
self.commit_subject = pmail['subject'].replace('\r\n', '')
self.commit_subject = self.commit_subject.replace('\n', '')
- pfx_start = self.commit_subject.find('[')
+ pfx_start_tmp = self.commit_subject.find('[')
+ pfx_end = self.commit_subject.find(']', pfx_start_tmp)
+ pfx_start = pfx_start_tmp
+ while (pfx_end > pfx_start_tmp):
+ pfx_start_tmp = self.commit_subject.find('[', pfx_end)
+ if pfx_start_tmp >= 0:
+ pfx_end = self.commit_subject.find(']',pfx_start_tmp)
+ else:
+ break
+
if pfx_start >= 0:
pfx_end = self.commit_subject.find(']')
if pfx_end > pfx_start:
self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
self.commit_subject = self.commit_subject[pfx_end + 1 :].lstrip()
--
2.6.1.windows.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Patch] BaseTools: support PCD value to use expression in the DEC file
2016-10-13 9:29 [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Yonghong Zhu
@ 2016-10-13 9:29 ` Yonghong Zhu
2016-10-19 1:32 ` Gao, Liming
2016-10-15 20:13 ` [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Jordan Justen
1 sibling, 1 reply; 9+ messages in thread
From: Yonghong Zhu @ 2016-10-13 9:29 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao
This patch add the support for Pcd value to use expression in the DEC file.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
---
BaseTools/Source/Python/Common/Misc.py | 57 ++++++++++++----------
.../Source/Python/Workspace/MetaFileParser.py | 21 ++++++--
2 files changed, 49 insertions(+), 29 deletions(-)
diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py
index c99716d..3be1f0f 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -1410,36 +1410,11 @@ def ParseConsoleLog(Filename):
Opw.write('%s\n' % Line)
Opr.close()
Opw.close()
-## AnalyzeDscPcd
-#
-# Analyze DSC PCD value, since there is no data type info in DSC
-# This fuction is used to match functions (AnalyzePcdData, AnalyzeHiiPcdData, AnalyzeVpdPcdData) used for retrieving PCD value from database
-# 1. Feature flag: TokenSpace.PcdCName|PcdValue
-# 2. Fix and Patch:TokenSpace.PcdCName|PcdValue[|MaxSize]
-# 3. Dynamic default:
-# TokenSpace.PcdCName|PcdValue[|VOID*[|MaxSize]]
-# TokenSpace.PcdCName|PcdValue
-# 4. Dynamic VPD:
-# TokenSpace.PcdCName|VpdOffset[|VpdValue]
-# TokenSpace.PcdCName|VpdOffset[|MaxSize[|VpdValue]]
-# 5. Dynamic HII:
-# TokenSpace.PcdCName|HiiString|VaiableGuid|VariableOffset[|HiiValue]
-# PCD value needs to be located in such kind of string, and the PCD value might be an expression in which
-# there might have "|" operator, also in string value.
-#
-# @param Setting: String contain information described above with "TokenSpace.PcdCName|" stripped
-# @param PcdType: PCD type: feature, fixed, dynamic default VPD HII
-# @param DataType: The datum type of PCD: VOID*, UNIT, BOOL
-# @retval:
-# ValueList: A List contain fields described above
-# IsValid: True if conforming EBNF, otherwise False
-# Index: The index where PcdValue is in ValueList
-#
-def AnalyzeDscPcd(Setting, PcdType, DataType=''):
+def AnalyzePcdExpression(Setting):
Setting = Setting.strip()
# There might be escaped quote in a string: \", \\\"
Data = Setting.replace('\\\\', '//').replace('\\\"', '\\\'')
# There might be '|' in string and in ( ... | ... ), replace it with '-'
NewStr = ''
@@ -1465,10 +1440,40 @@ def AnalyzeDscPcd(Setting, PcdType, DataType=''):
FieldList.append(Setting[StartPos:].strip())
break
FieldList.append(Setting[StartPos:Pos].strip())
StartPos = Pos + 1
+ return FieldList
+
+## AnalyzeDscPcd
+#
+# Analyze DSC PCD value, since there is no data type info in DSC
+# This fuction is used to match functions (AnalyzePcdData, AnalyzeHiiPcdData, AnalyzeVpdPcdData) used for retrieving PCD value from database
+# 1. Feature flag: TokenSpace.PcdCName|PcdValue
+# 2. Fix and Patch:TokenSpace.PcdCName|PcdValue[|MaxSize]
+# 3. Dynamic default:
+# TokenSpace.PcdCName|PcdValue[|VOID*[|MaxSize]]
+# TokenSpace.PcdCName|PcdValue
+# 4. Dynamic VPD:
+# TokenSpace.PcdCName|VpdOffset[|VpdValue]
+# TokenSpace.PcdCName|VpdOffset[|MaxSize[|VpdValue]]
+# 5. Dynamic HII:
+# TokenSpace.PcdCName|HiiString|VaiableGuid|VariableOffset[|HiiValue]
+# PCD value needs to be located in such kind of string, and the PCD value might be an expression in which
+# there might have "|" operator, also in string value.
+#
+# @param Setting: String contain information described above with "TokenSpace.PcdCName|" stripped
+# @param PcdType: PCD type: feature, fixed, dynamic default VPD HII
+# @param DataType: The datum type of PCD: VOID*, UNIT, BOOL
+# @retval:
+# ValueList: A List contain fields described above
+# IsValid: True if conforming EBNF, otherwise False
+# Index: The index where PcdValue is in ValueList
+#
+def AnalyzeDscPcd(Setting, PcdType, DataType=''):
+ FieldList = AnalyzePcdExpression(Setting)
+
IsValid = True
if PcdType in (MODEL_PCD_FIXED_AT_BUILD, MODEL_PCD_PATCHABLE_IN_MODULE, MODEL_PCD_FEATURE_FLAG):
Value = FieldList[0]
Size = ''
if len(FieldList) > 1:
diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py
index 82d874f..1a5fdf5 100644
--- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
+++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
@@ -24,11 +24,11 @@ import Common.EdkLogger as EdkLogger
import Common.GlobalData as GlobalData
from CommonDataClass.DataClass import *
from Common.DataType import *
from Common.String import *
-from Common.Misc import GuidStructureStringToGuidString, CheckPcdDatum, PathClass, AnalyzePcdData, AnalyzeDscPcd
+from Common.Misc import GuidStructureStringToGuidString, CheckPcdDatum, PathClass, AnalyzePcdData, AnalyzeDscPcd, AnalyzePcdExpression
from Common.Expression import *
from CommonDataClass.Exceptions import *
from Common.LongFilePathSupport import OpenLongFilePath as open
from MetaFileTable import MetaFileStorage
@@ -1633,10 +1633,11 @@ class DecParser(MetaFileParser):
return
MetaFileParser.__init__(self, FilePath, FileType, Arch, Table, -1)
self._Comments = []
self._Version = 0x00010005 # Only EDK2 dec file is supported
self._AllPCDs = [] # Only for check duplicate PCD
+ self._AllPcdDict = {}
## Parser starter
def Start(self):
Content = ''
try:
@@ -1846,14 +1847,14 @@ class DecParser(MetaFileParser):
PtrValue = ValueRe.findall(TokenList[1])
# Has VOID* type string, may contain "|" character in the string.
if len(PtrValue) != 0:
ptrValueList = re.sub(ValueRe, '', TokenList[1])
- ValueList = GetSplitValueList(ptrValueList)
+ ValueList = AnalyzePcdExpression(ptrValueList)
ValueList[0] = PtrValue[0]
else:
- ValueList = GetSplitValueList(TokenList[1])
+ ValueList = AnalyzePcdExpression(TokenList[1])
# check if there's enough datum information given
if len(ValueList) != 3:
EdkLogger.error('Parser', FORMAT_INVALID, "Invalid PCD Datum information given",
@@ -1876,10 +1877,23 @@ class DecParser(MetaFileParser):
if ValueList[2] == '':
EdkLogger.error('Parser', FORMAT_INVALID, "Missing Token in PCD Datum information",
ExtraData=self._CurrentLine + \
" (<TokenSpaceGuidCName>.<PcdCName>|<DefaultValue>|<DatumType>|<Token>)",
File=self.MetaFile, Line=self._LineIndex + 1)
+
+ PcdValue = ValueList[0]
+ if PcdValue:
+ try:
+ ValueList[0] = ValueExpression(PcdValue, self._AllPcdDict)(True)
+ except WrnExpression, Value:
+ ValueList[0] = Value.result
+
+ if ValueList[0] == 'True':
+ ValueList[0] = '1'
+ if ValueList[0] == 'False':
+ ValueList[0] = '0'
+
# check format of default value against the datum type
IsValid, Cause = CheckPcdDatum(ValueList[1], ValueList[0])
if not IsValid:
EdkLogger.error('Parser', FORMAT_INVALID, Cause, ExtraData=self._CurrentLine,
File=self.MetaFile, Line=self._LineIndex + 1)
@@ -1894,10 +1908,11 @@ class DecParser(MetaFileParser):
EdkLogger.error('Parser', FORMAT_INVALID,
"The same PCD name and GUID have been already defined",
ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1)
else:
self._AllPCDs.append((self._Scope[0], self._ValueList[0], self._ValueList[1]))
+ self._AllPcdDict[TAB_SPLIT.join(self._ValueList[0:2])] = ValueList[0]
self._ValueList[2] = ValueList[0].strip() + '|' + ValueList[1].strip() + '|' + ValueList[2].strip()
_SectionParser = {
MODEL_META_DATA_HEADER : MetaFileParser._DefineParser,
--
2.6.1.windows.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
2016-10-13 9:29 [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Yonghong Zhu
2016-10-13 9:29 ` [Patch] BaseTools: support PCD value to use expression in the DEC file Yonghong Zhu
@ 2016-10-15 20:13 ` Jordan Justen
2016-10-16 13:32 ` Zhu, Yonghong
1 sibling, 1 reply; 9+ messages in thread
From: Jordan Justen @ 2016-10-15 20:13 UTC (permalink / raw)
To: Yonghong Zhu, edk2-devel; +Cc: Liming Gao
On 2016-10-13 02:29:46, Yonghong Zhu wrote:
> This patch updates PatchCheck.py:
Can you split this into 3 separate patches?
> 1.Update max line length to 70 that describe in wiki
I think we should update PatchCheck.py and the wiki to say that:
1. The subject line of the commit message should be < 72 characters.
This works well with git log --oneline to still produce output less
than 80 characters.
2. The other lines of the commit message should be < 76 characters.
When using git log, these lines are indented 4 characters, so once
again this will keep the log message readable and under 80 columns.
> 2.remove the two [] when do the char count calculation
How about instead we add a new regex to the class:
subject_prefix_re = \
re.compile(r'''^
\s* (\[
[^\[\]]* # Allow all non-brackets
\])? \s*
''',
re.VERBOSE)
Then you can use it line this:
self.commit_subject = \
self.subject_prefix_re.sub('', self.commit_subject, 1)
This will change:
' a' => 'a'
' [patch 1] a' => 'a'
But, it will ignore this, because I don't think we should try to match
brackets here:
' [a [b]] a'
> 3.report error for EFI_D_* macro if it is used, recommend to use
> DEBUG_* format.
This should definitely be a separate patch since we are starting to
look at code.
-Jordan
>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> ---
> BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 455c130..d423220 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -1,9 +1,9 @@
> ## @file
> # Check a patch for various format issues
> #
> -# Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> #
> # This program and the accompanying materials are licensed and made
> # available under the terms and conditions of the BSD License which
> # accompanies this distribution. The full text of the license may be
> # found at http://opensource.org/licenses/bsd-license.php
> @@ -14,11 +14,11 @@
> #
>
> from __future__ import print_function
>
> VersionNumber = '0.1'
> -__copyright__ = "Copyright (c) 2015, Intel Corporation All rights reserved."
> +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
>
> import email
> import argparse
> import os
> import re
> @@ -195,11 +195,11 @@ class CommitMessageCheck:
>
> if count <= 0:
> self.error('Empty commit message!')
> return
>
> - if count >= 1 and len(lines[0]) > 76:
> + if count >= 1 and len(lines[0]) > 70:
> self.error('First line of commit message (subject line) ' +
> 'is too long.')
>
> if count >= 1 and len(lines[0].strip()) == 0:
> self.error('First line of commit message (subject line) ' +
> @@ -208,11 +208,11 @@ class CommitMessageCheck:
> if count >= 2 and lines[1].strip() != '':
> self.error('Second line of commit message should be ' +
> 'empty.')
>
> for i in range(2, count):
> - if (len(lines[i]) > 76 and
> + if (len(lines[i]) > 70 and
> len(lines[i].split()) > 1 and
> not lines[i].startswith('git-svn-id:')):
> self.error('Line %d of commit message is too long.' % (i + 1))
>
> last_sig_line = None
> @@ -354,10 +354,12 @@ class GitDiffCheck:
> line)
> if ' ' in line:
> self.added_line_error('Tab character used', line)
> if len(stripped) < len(line):
> self.added_line_error('Trailing whitespace found', line)
> + if 'EFI_D_' in line:
> + self.added_line_error('EFI_D_* used, Please use DEBUG_* format', line)
>
> split_diff_re = re.compile(r'''
> (?P<cmd>
> ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> )
> @@ -471,11 +473,20 @@ class CheckOnePatch:
> self.commit_msg = mo.group('commit_message')
>
> self.commit_subject = pmail['subject'].replace('\r\n', '')
> self.commit_subject = self.commit_subject.replace('\n', '')
>
> - pfx_start = self.commit_subject.find('[')
> + pfx_start_tmp = self.commit_subject.find('[')
> + pfx_end = self.commit_subject.find(']', pfx_start_tmp)
> + pfx_start = pfx_start_tmp
> + while (pfx_end > pfx_start_tmp):
> + pfx_start_tmp = self.commit_subject.find('[', pfx_end)
> + if pfx_start_tmp >= 0:
> + pfx_end = self.commit_subject.find(']',pfx_start_tmp)
> + else:
> + break
> +
> if pfx_start >= 0:
> pfx_end = self.commit_subject.find(']')
> if pfx_end > pfx_start:
> self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
> self.commit_subject = self.commit_subject[pfx_end + 1 :].lstrip()
> --
> 2.6.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
2016-10-15 20:13 ` [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Jordan Justen
@ 2016-10-16 13:32 ` Zhu, Yonghong
2016-10-17 5:49 ` Jordan Justen
2016-10-17 5:56 ` Jordan Justen
0 siblings, 2 replies; 9+ messages in thread
From: Zhu, Yonghong @ 2016-10-16 13:32 UTC (permalink / raw)
To: Justen, Jordan L, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
Thanks Jordan. I will separate the patches and send out the updated version.
Besides can you help to update the wiki ?
Best Regards,
Zhu Yonghong
-----Original Message-----
From: Justen, Jordan L
Sent: Sunday, October 16, 2016 4:14 AM
To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
On 2016-10-13 02:29:46, Yonghong Zhu wrote:
> This patch updates PatchCheck.py:
Can you split this into 3 separate patches?
> 1.Update max line length to 70 that describe in wiki
I think we should update PatchCheck.py and the wiki to say that:
1. The subject line of the commit message should be < 72 characters.
This works well with git log --oneline to still produce output less than 80 characters.
2. The other lines of the commit message should be < 76 characters.
When using git log, these lines are indented 4 characters, so once again this will keep the log message readable and under 80 columns.
> 2.remove the two [] when do the char count calculation
How about instead we add a new regex to the class:
subject_prefix_re = \
re.compile(r'''^
\s* (\[
[^\[\]]* # Allow all non-brackets
\])? \s*
''',
re.VERBOSE)
Then you can use it line this:
self.commit_subject = \
self.subject_prefix_re.sub('', self.commit_subject, 1)
This will change:
' a' => 'a'
' [patch 1] a' => 'a'
But, it will ignore this, because I don't think we should try to match brackets here:
' [a [b]] a'
> 3.report error for EFI_D_* macro if it is used, recommend to use
> DEBUG_* format.
This should definitely be a separate patch since we are starting to look at code.
-Jordan
>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> ---
> BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py
> b/BaseTools/Scripts/PatchCheck.py index 455c130..d423220 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -1,9 +1,9 @@
> ## @file
> # Check a patch for various format issues # -# Copyright (c) 2015,
> Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2016, Intel Corporation. All rights
> +reserved.<BR>
> #
> # This program and the accompanying materials are licensed and made
> # available under the terms and conditions of the BSD License which
> # accompanies this distribution. The full text of the license may be
> # found at http://opensource.org/licenses/bsd-license.php
> @@ -14,11 +14,11 @@
> #
>
> from __future__ import print_function
>
> VersionNumber = '0.1'
> -__copyright__ = "Copyright (c) 2015, Intel Corporation All rights reserved."
> +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
>
> import email
> import argparse
> import os
> import re
> @@ -195,11 +195,11 @@ class CommitMessageCheck:
>
> if count <= 0:
> self.error('Empty commit message!')
> return
>
> - if count >= 1 and len(lines[0]) > 76:
> + if count >= 1 and len(lines[0]) > 70:
> self.error('First line of commit message (subject line) ' +
> 'is too long.')
>
> if count >= 1 and len(lines[0].strip()) == 0:
> self.error('First line of commit message (subject line) '
> + @@ -208,11 +208,11 @@ class CommitMessageCheck:
> if count >= 2 and lines[1].strip() != '':
> self.error('Second line of commit message should be ' +
> 'empty.')
>
> for i in range(2, count):
> - if (len(lines[i]) > 76 and
> + if (len(lines[i]) > 70 and
> len(lines[i].split()) > 1 and
> not lines[i].startswith('git-svn-id:')):
> self.error('Line %d of commit message is too long.' %
> (i + 1))
>
> last_sig_line = None
> @@ -354,10 +354,12 @@ class GitDiffCheck:
> line)
> if ' ' in line:
> self.added_line_error('Tab character used', line)
> if len(stripped) < len(line):
> self.added_line_error('Trailing whitespace found', line)
> + if 'EFI_D_' in line:
> + self.added_line_error('EFI_D_* used, Please use DEBUG_*
> + format', line)
>
> split_diff_re = re.compile(r'''
> (?P<cmd>
> ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> )
> @@ -471,11 +473,20 @@ class CheckOnePatch:
> self.commit_msg = mo.group('commit_message')
>
> self.commit_subject = pmail['subject'].replace('\r\n', '')
> self.commit_subject = self.commit_subject.replace('\n', '')
>
> - pfx_start = self.commit_subject.find('[')
> + pfx_start_tmp = self.commit_subject.find('[')
> + pfx_end = self.commit_subject.find(']', pfx_start_tmp)
> + pfx_start = pfx_start_tmp
> + while (pfx_end > pfx_start_tmp):
> + pfx_start_tmp = self.commit_subject.find('[', pfx_end)
> + if pfx_start_tmp >= 0:
> + pfx_end = self.commit_subject.find(']',pfx_start_tmp)
> + else:
> + break
> +
> if pfx_start >= 0:
> pfx_end = self.commit_subject.find(']')
> if pfx_end > pfx_start:
> self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
> self.commit_subject = self.commit_subject[pfx_end + 1
> :].lstrip()
> --
> 2.6.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
2016-10-16 13:32 ` Zhu, Yonghong
@ 2016-10-17 5:49 ` Jordan Justen
2016-10-17 6:53 ` Zhu, Yonghong
2016-10-17 5:56 ` Jordan Justen
1 sibling, 1 reply; 9+ messages in thread
From: Jordan Justen @ 2016-10-17 5:49 UTC (permalink / raw)
To: Zhu, Yonghong, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
On 2016-10-16 06:32:32, Zhu, Yonghong wrote:
> Thanks Jordan. I will separate the patches and send out the updated version.
>
> Besides can you help to update the wiki ?
>
How does it look?
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
-Jordan
>
> -----Original Message-----
> From: Justen, Jordan L
> Sent: Sunday, October 16, 2016 4:14 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
>
> On 2016-10-13 02:29:46, Yonghong Zhu wrote:
> > This patch updates PatchCheck.py:
>
> Can you split this into 3 separate patches?
>
> > 1.Update max line length to 70 that describe in wiki
>
> I think we should update PatchCheck.py and the wiki to say that:
>
> 1. The subject line of the commit message should be < 72 characters.
>
> This works well with git log --oneline to still produce output less than 80 characters.
>
> 2. The other lines of the commit message should be < 76 characters.
>
> When using git log, these lines are indented 4 characters, so once again this will keep the log message readable and under 80 columns.
>
> > 2.remove the two [] when do the char count calculation
>
> How about instead we add a new regex to the class:
>
> subject_prefix_re = \
> re.compile(r'''^
> \s* (\[
> [^\[\]]* # Allow all non-brackets
> \])? \s*
> ''',
> re.VERBOSE)
>
>
> Then you can use it line this:
>
> self.commit_subject = \
> self.subject_prefix_re.sub('', self.commit_subject, 1)
>
> This will change:
>
> ' a' => 'a'
> ' [patch 1] a' => 'a'
>
> But, it will ignore this, because I don't think we should try to match brackets here:
>
> ' [a [b]] a'
>
> > 3.report error for EFI_D_* macro if it is used, recommend to use
> > DEBUG_* format.
>
> This should definitely be a separate patch since we are starting to look at code.
>
> -Jordan
>
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> > ---
> > BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++-----
> > 1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/BaseTools/Scripts/PatchCheck.py
> > b/BaseTools/Scripts/PatchCheck.py index 455c130..d423220 100755
> > --- a/BaseTools/Scripts/PatchCheck.py
> > +++ b/BaseTools/Scripts/PatchCheck.py
> > @@ -1,9 +1,9 @@
> > ## @file
> > # Check a patch for various format issues # -# Copyright (c) 2015,
> > Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2015 - 2016, Intel Corporation. All rights
> > +reserved.<BR>
> > #
> > # This program and the accompanying materials are licensed and made
> > # available under the terms and conditions of the BSD License which
> > # accompanies this distribution. The full text of the license may be
> > # found at http://opensource.org/licenses/bsd-license.php
> > @@ -14,11 +14,11 @@
> > #
> >
> > from __future__ import print_function
> >
> > VersionNumber = '0.1'
> > -__copyright__ = "Copyright (c) 2015, Intel Corporation All rights reserved."
> > +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
> >
> > import email
> > import argparse
> > import os
> > import re
> > @@ -195,11 +195,11 @@ class CommitMessageCheck:
> >
> > if count <= 0:
> > self.error('Empty commit message!')
> > return
> >
> > - if count >= 1 and len(lines[0]) > 76:
> > + if count >= 1 and len(lines[0]) > 70:
> > self.error('First line of commit message (subject line) ' +
> > 'is too long.')
> >
> > if count >= 1 and len(lines[0].strip()) == 0:
> > self.error('First line of commit message (subject line) '
> > + @@ -208,11 +208,11 @@ class CommitMessageCheck:
> > if count >= 2 and lines[1].strip() != '':
> > self.error('Second line of commit message should be ' +
> > 'empty.')
> >
> > for i in range(2, count):
> > - if (len(lines[i]) > 76 and
> > + if (len(lines[i]) > 70 and
> > len(lines[i].split()) > 1 and
> > not lines[i].startswith('git-svn-id:')):
> > self.error('Line %d of commit message is too long.' %
> > (i + 1))
> >
> > last_sig_line = None
> > @@ -354,10 +354,12 @@ class GitDiffCheck:
> > line)
> > if ' ' in line:
> > self.added_line_error('Tab character used', line)
> > if len(stripped) < len(line):
> > self.added_line_error('Trailing whitespace found', line)
> > + if 'EFI_D_' in line:
> > + self.added_line_error('EFI_D_* used, Please use DEBUG_*
> > + format', line)
> >
> > split_diff_re = re.compile(r'''
> > (?P<cmd>
> > ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> > )
> > @@ -471,11 +473,20 @@ class CheckOnePatch:
> > self.commit_msg = mo.group('commit_message')
> >
> > self.commit_subject = pmail['subject'].replace('\r\n', '')
> > self.commit_subject = self.commit_subject.replace('\n', '')
> >
> > - pfx_start = self.commit_subject.find('[')
> > + pfx_start_tmp = self.commit_subject.find('[')
> > + pfx_end = self.commit_subject.find(']', pfx_start_tmp)
> > + pfx_start = pfx_start_tmp
> > + while (pfx_end > pfx_start_tmp):
> > + pfx_start_tmp = self.commit_subject.find('[', pfx_end)
> > + if pfx_start_tmp >= 0:
> > + pfx_end = self.commit_subject.find(']',pfx_start_tmp)
> > + else:
> > + break
> > +
> > if pfx_start >= 0:
> > pfx_end = self.commit_subject.find(']')
> > if pfx_end > pfx_start:
> > self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
> > self.commit_subject = self.commit_subject[pfx_end + 1
> > :].lstrip()
> > --
> > 2.6.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
2016-10-16 13:32 ` Zhu, Yonghong
2016-10-17 5:49 ` Jordan Justen
@ 2016-10-17 5:56 ` Jordan Justen
2016-10-17 6:43 ` Zhu, Yonghong
1 sibling, 1 reply; 9+ messages in thread
From: Jordan Justen @ 2016-10-17 5:56 UTC (permalink / raw)
To: Zhu, Yonghong, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
On 2016-10-16 06:32:32, Zhu, Yonghong wrote:
> Thanks Jordan. I will separate the patches and send out the updated version.
>
One other suggestion for the commit message on your patch. Can you add
this at the bottom before the Contributed-under?
Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=113
What do you think?
Maybe we should update the commit message wiki page to suggest this as
well?
-Jordan
>
> -----Original Message-----
> From: Justen, Jordan L
> Sent: Sunday, October 16, 2016 4:14 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
>
> On 2016-10-13 02:29:46, Yonghong Zhu wrote:
> > This patch updates PatchCheck.py:
>
> Can you split this into 3 separate patches?
>
> > 1.Update max line length to 70 that describe in wiki
>
> I think we should update PatchCheck.py and the wiki to say that:
>
> 1. The subject line of the commit message should be < 72 characters.
>
> This works well with git log --oneline to still produce output less than 80 characters.
>
> 2. The other lines of the commit message should be < 76 characters.
>
> When using git log, these lines are indented 4 characters, so once again this will keep the log message readable and under 80 columns.
>
> > 2.remove the two [] when do the char count calculation
>
> How about instead we add a new regex to the class:
>
> subject_prefix_re = \
> re.compile(r'''^
> \s* (\[
> [^\[\]]* # Allow all non-brackets
> \])? \s*
> ''',
> re.VERBOSE)
>
>
> Then you can use it line this:
>
> self.commit_subject = \
> self.subject_prefix_re.sub('', self.commit_subject, 1)
>
> This will change:
>
> ' a' => 'a'
> ' [patch 1] a' => 'a'
>
> But, it will ignore this, because I don't think we should try to match brackets here:
>
> ' [a [b]] a'
>
> > 3.report error for EFI_D_* macro if it is used, recommend to use
> > DEBUG_* format.
>
> This should definitely be a separate patch since we are starting to look at code.
>
> -Jordan
>
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> > ---
> > BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++-----
> > 1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/BaseTools/Scripts/PatchCheck.py
> > b/BaseTools/Scripts/PatchCheck.py index 455c130..d423220 100755
> > --- a/BaseTools/Scripts/PatchCheck.py
> > +++ b/BaseTools/Scripts/PatchCheck.py
> > @@ -1,9 +1,9 @@
> > ## @file
> > # Check a patch for various format issues # -# Copyright (c) 2015,
> > Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2015 - 2016, Intel Corporation. All rights
> > +reserved.<BR>
> > #
> > # This program and the accompanying materials are licensed and made
> > # available under the terms and conditions of the BSD License which
> > # accompanies this distribution. The full text of the license may be
> > # found at http://opensource.org/licenses/bsd-license.php
> > @@ -14,11 +14,11 @@
> > #
> >
> > from __future__ import print_function
> >
> > VersionNumber = '0.1'
> > -__copyright__ = "Copyright (c) 2015, Intel Corporation All rights reserved."
> > +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
> >
> > import email
> > import argparse
> > import os
> > import re
> > @@ -195,11 +195,11 @@ class CommitMessageCheck:
> >
> > if count <= 0:
> > self.error('Empty commit message!')
> > return
> >
> > - if count >= 1 and len(lines[0]) > 76:
> > + if count >= 1 and len(lines[0]) > 70:
> > self.error('First line of commit message (subject line) ' +
> > 'is too long.')
> >
> > if count >= 1 and len(lines[0].strip()) == 0:
> > self.error('First line of commit message (subject line) '
> > + @@ -208,11 +208,11 @@ class CommitMessageCheck:
> > if count >= 2 and lines[1].strip() != '':
> > self.error('Second line of commit message should be ' +
> > 'empty.')
> >
> > for i in range(2, count):
> > - if (len(lines[i]) > 76 and
> > + if (len(lines[i]) > 70 and
> > len(lines[i].split()) > 1 and
> > not lines[i].startswith('git-svn-id:')):
> > self.error('Line %d of commit message is too long.' %
> > (i + 1))
> >
> > last_sig_line = None
> > @@ -354,10 +354,12 @@ class GitDiffCheck:
> > line)
> > if ' ' in line:
> > self.added_line_error('Tab character used', line)
> > if len(stripped) < len(line):
> > self.added_line_error('Trailing whitespace found', line)
> > + if 'EFI_D_' in line:
> > + self.added_line_error('EFI_D_* used, Please use DEBUG_*
> > + format', line)
> >
> > split_diff_re = re.compile(r'''
> > (?P<cmd>
> > ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> > )
> > @@ -471,11 +473,20 @@ class CheckOnePatch:
> > self.commit_msg = mo.group('commit_message')
> >
> > self.commit_subject = pmail['subject'].replace('\r\n', '')
> > self.commit_subject = self.commit_subject.replace('\n', '')
> >
> > - pfx_start = self.commit_subject.find('[')
> > + pfx_start_tmp = self.commit_subject.find('[')
> > + pfx_end = self.commit_subject.find(']', pfx_start_tmp)
> > + pfx_start = pfx_start_tmp
> > + while (pfx_end > pfx_start_tmp):
> > + pfx_start_tmp = self.commit_subject.find('[', pfx_end)
> > + if pfx_start_tmp >= 0:
> > + pfx_end = self.commit_subject.find(']',pfx_start_tmp)
> > + else:
> > + break
> > +
> > if pfx_start >= 0:
> > pfx_end = self.commit_subject.find(']')
> > if pfx_end > pfx_start:
> > self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
> > self.commit_subject = self.commit_subject[pfx_end + 1
> > :].lstrip()
> > --
> > 2.6.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
2016-10-17 5:56 ` Jordan Justen
@ 2016-10-17 6:43 ` Zhu, Yonghong
0 siblings, 0 replies; 9+ messages in thread
From: Zhu, Yonghong @ 2016-10-17 6:43 UTC (permalink / raw)
To: Justen, Jordan L, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
Sure, I will add this info.
Yes, add this suggestion is good.
Best Regards,
Zhu Yonghong
-----Original Message-----
From: Justen, Jordan L
Sent: Monday, October 17, 2016 1:56 PM
To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: RE: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
On 2016-10-16 06:32:32, Zhu, Yonghong wrote:
> Thanks Jordan. I will separate the patches and send out the updated version.
>
One other suggestion for the commit message on your patch. Can you add this at the bottom before the Contributed-under?
Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=113
What do you think?
Maybe we should update the commit message wiki page to suggest this as well?
-Jordan
>
> -----Original Message-----
> From: Justen, Jordan L
> Sent: Sunday, October 16, 2016 4:14 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki
> and report error for EFI_D_*
>
> On 2016-10-13 02:29:46, Yonghong Zhu wrote:
> > This patch updates PatchCheck.py:
>
> Can you split this into 3 separate patches?
>
> > 1.Update max line length to 70 that describe in wiki
>
> I think we should update PatchCheck.py and the wiki to say that:
>
> 1. The subject line of the commit message should be < 72 characters.
>
> This works well with git log --oneline to still produce output less than 80 characters.
>
> 2. The other lines of the commit message should be < 76 characters.
>
> When using git log, these lines are indented 4 characters, so once again this will keep the log message readable and under 80 columns.
>
> > 2.remove the two [] when do the char count calculation
>
> How about instead we add a new regex to the class:
>
> subject_prefix_re = \
> re.compile(r'''^
> \s* (\[
> [^\[\]]* # Allow all non-brackets
> \])? \s*
> ''',
> re.VERBOSE)
>
>
> Then you can use it line this:
>
> self.commit_subject = \
> self.subject_prefix_re.sub('', self.commit_subject, 1)
>
> This will change:
>
> ' a' => 'a'
> ' [patch 1] a' => 'a'
>
> But, it will ignore this, because I don't think we should try to match brackets here:
>
> ' [a [b]] a'
>
> > 3.report error for EFI_D_* macro if it is used, recommend to use
> > DEBUG_* format.
>
> This should definitely be a separate patch since we are starting to look at code.
>
> -Jordan
>
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> > ---
> > BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++-----
> > 1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/BaseTools/Scripts/PatchCheck.py
> > b/BaseTools/Scripts/PatchCheck.py index 455c130..d423220 100755
> > --- a/BaseTools/Scripts/PatchCheck.py
> > +++ b/BaseTools/Scripts/PatchCheck.py
> > @@ -1,9 +1,9 @@
> > ## @file
> > # Check a patch for various format issues # -# Copyright (c)
> > 2015, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2015 - 2016, Intel Corporation. All rights
> > +reserved.<BR>
> > #
> > # This program and the accompanying materials are licensed and
> > made # available under the terms and conditions of the BSD License
> > which # accompanies this distribution. The full text of the license
> > may be # found at http://opensource.org/licenses/bsd-license.php
> > @@ -14,11 +14,11 @@
> > #
> >
> > from __future__ import print_function
> >
> > VersionNumber = '0.1'
> > -__copyright__ = "Copyright (c) 2015, Intel Corporation All rights reserved."
> > +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
> >
> > import email
> > import argparse
> > import os
> > import re
> > @@ -195,11 +195,11 @@ class CommitMessageCheck:
> >
> > if count <= 0:
> > self.error('Empty commit message!')
> > return
> >
> > - if count >= 1 and len(lines[0]) > 76:
> > + if count >= 1 and len(lines[0]) > 70:
> > self.error('First line of commit message (subject line) ' +
> > 'is too long.')
> >
> > if count >= 1 and len(lines[0].strip()) == 0:
> > self.error('First line of commit message (subject line) '
> > + @@ -208,11 +208,11 @@ class CommitMessageCheck:
> > if count >= 2 and lines[1].strip() != '':
> > self.error('Second line of commit message should be ' +
> > 'empty.')
> >
> > for i in range(2, count):
> > - if (len(lines[i]) > 76 and
> > + if (len(lines[i]) > 70 and
> > len(lines[i].split()) > 1 and
> > not lines[i].startswith('git-svn-id:')):
> > self.error('Line %d of commit message is too long.'
> > % (i + 1))
> >
> > last_sig_line = None
> > @@ -354,10 +354,12 @@ class GitDiffCheck:
> > line)
> > if ' ' in line:
> > self.added_line_error('Tab character used', line)
> > if len(stripped) < len(line):
> > self.added_line_error('Trailing whitespace found',
> > line)
> > + if 'EFI_D_' in line:
> > + self.added_line_error('EFI_D_* used, Please use DEBUG_*
> > + format', line)
> >
> > split_diff_re = re.compile(r'''
> > (?P<cmd>
> > ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> > ) @@ -471,11 +473,20 @@ class
> > CheckOnePatch:
> > self.commit_msg = mo.group('commit_message')
> >
> > self.commit_subject = pmail['subject'].replace('\r\n', '')
> > self.commit_subject = self.commit_subject.replace('\n', '')
> >
> > - pfx_start = self.commit_subject.find('[')
> > + pfx_start_tmp = self.commit_subject.find('[')
> > + pfx_end = self.commit_subject.find(']', pfx_start_tmp)
> > + pfx_start = pfx_start_tmp
> > + while (pfx_end > pfx_start_tmp):
> > + pfx_start_tmp = self.commit_subject.find('[', pfx_end)
> > + if pfx_start_tmp >= 0:
> > + pfx_end = self.commit_subject.find(']',pfx_start_tmp)
> > + else:
> > + break
> > +
> > if pfx_start >= 0:
> > pfx_end = self.commit_subject.find(']')
> > if pfx_end > pfx_start:
> > self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
> > self.commit_subject = self.commit_subject[pfx_end +
> > 1
> > :].lstrip()
> > --
> > 2.6.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
2016-10-17 5:49 ` Jordan Justen
@ 2016-10-17 6:53 ` Zhu, Yonghong
0 siblings, 0 replies; 9+ messages in thread
From: Zhu, Yonghong @ 2016-10-17 6:53 UTC (permalink / raw)
To: Justen, Jordan L, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
It's good to me. I will send patch to align with this wiki.
Best Regards,
Zhu Yonghong
-----Original Message-----
From: Justen, Jordan L
Sent: Monday, October 17, 2016 1:50 PM
To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: RE: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
On 2016-10-16 06:32:32, Zhu, Yonghong wrote:
> Thanks Jordan. I will separate the patches and send out the updated version.
>
> Besides can you help to update the wiki ?
>
How does it look?
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
-Jordan
>
> -----Original Message-----
> From: Justen, Jordan L
> Sent: Sunday, October 16, 2016 4:14 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki
> and report error for EFI_D_*
>
> On 2016-10-13 02:29:46, Yonghong Zhu wrote:
> > This patch updates PatchCheck.py:
>
> Can you split this into 3 separate patches?
>
> > 1.Update max line length to 70 that describe in wiki
>
> I think we should update PatchCheck.py and the wiki to say that:
>
> 1. The subject line of the commit message should be < 72 characters.
>
> This works well with git log --oneline to still produce output less than 80 characters.
>
> 2. The other lines of the commit message should be < 76 characters.
>
> When using git log, these lines are indented 4 characters, so once again this will keep the log message readable and under 80 columns.
>
> > 2.remove the two [] when do the char count calculation
>
> How about instead we add a new regex to the class:
>
> subject_prefix_re = \
> re.compile(r'''^
> \s* (\[
> [^\[\]]* # Allow all non-brackets
> \])? \s*
> ''',
> re.VERBOSE)
>
>
> Then you can use it line this:
>
> self.commit_subject = \
> self.subject_prefix_re.sub('', self.commit_subject, 1)
>
> This will change:
>
> ' a' => 'a'
> ' [patch 1] a' => 'a'
>
> But, it will ignore this, because I don't think we should try to match brackets here:
>
> ' [a [b]] a'
>
> > 3.report error for EFI_D_* macro if it is used, recommend to use
> > DEBUG_* format.
>
> This should definitely be a separate patch since we are starting to look at code.
>
> -Jordan
>
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> > ---
> > BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++-----
> > 1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/BaseTools/Scripts/PatchCheck.py
> > b/BaseTools/Scripts/PatchCheck.py index 455c130..d423220 100755
> > --- a/BaseTools/Scripts/PatchCheck.py
> > +++ b/BaseTools/Scripts/PatchCheck.py
> > @@ -1,9 +1,9 @@
> > ## @file
> > # Check a patch for various format issues # -# Copyright (c)
> > 2015, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2015 - 2016, Intel Corporation. All rights
> > +reserved.<BR>
> > #
> > # This program and the accompanying materials are licensed and
> > made # available under the terms and conditions of the BSD License
> > which # accompanies this distribution. The full text of the license
> > may be # found at http://opensource.org/licenses/bsd-license.php
> > @@ -14,11 +14,11 @@
> > #
> >
> > from __future__ import print_function
> >
> > VersionNumber = '0.1'
> > -__copyright__ = "Copyright (c) 2015, Intel Corporation All rights reserved."
> > +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
> >
> > import email
> > import argparse
> > import os
> > import re
> > @@ -195,11 +195,11 @@ class CommitMessageCheck:
> >
> > if count <= 0:
> > self.error('Empty commit message!')
> > return
> >
> > - if count >= 1 and len(lines[0]) > 76:
> > + if count >= 1 and len(lines[0]) > 70:
> > self.error('First line of commit message (subject line) ' +
> > 'is too long.')
> >
> > if count >= 1 and len(lines[0].strip()) == 0:
> > self.error('First line of commit message (subject line) '
> > + @@ -208,11 +208,11 @@ class CommitMessageCheck:
> > if count >= 2 and lines[1].strip() != '':
> > self.error('Second line of commit message should be ' +
> > 'empty.')
> >
> > for i in range(2, count):
> > - if (len(lines[i]) > 76 and
> > + if (len(lines[i]) > 70 and
> > len(lines[i].split()) > 1 and
> > not lines[i].startswith('git-svn-id:')):
> > self.error('Line %d of commit message is too long.'
> > % (i + 1))
> >
> > last_sig_line = None
> > @@ -354,10 +354,12 @@ class GitDiffCheck:
> > line)
> > if ' ' in line:
> > self.added_line_error('Tab character used', line)
> > if len(stripped) < len(line):
> > self.added_line_error('Trailing whitespace found',
> > line)
> > + if 'EFI_D_' in line:
> > + self.added_line_error('EFI_D_* used, Please use DEBUG_*
> > + format', line)
> >
> > split_diff_re = re.compile(r'''
> > (?P<cmd>
> > ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> > ) @@ -471,11 +473,20 @@ class
> > CheckOnePatch:
> > self.commit_msg = mo.group('commit_message')
> >
> > self.commit_subject = pmail['subject'].replace('\r\n', '')
> > self.commit_subject = self.commit_subject.replace('\n', '')
> >
> > - pfx_start = self.commit_subject.find('[')
> > + pfx_start_tmp = self.commit_subject.find('[')
> > + pfx_end = self.commit_subject.find(']', pfx_start_tmp)
> > + pfx_start = pfx_start_tmp
> > + while (pfx_end > pfx_start_tmp):
> > + pfx_start_tmp = self.commit_subject.find('[', pfx_end)
> > + if pfx_start_tmp >= 0:
> > + pfx_end = self.commit_subject.find(']',pfx_start_tmp)
> > + else:
> > + break
> > +
> > if pfx_start >= 0:
> > pfx_end = self.commit_subject.find(']')
> > if pfx_end > pfx_start:
> > self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
> > self.commit_subject = self.commit_subject[pfx_end +
> > 1
> > :].lstrip()
> > --
> > 2.6.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: support PCD value to use expression in the DEC file
2016-10-13 9:29 ` [Patch] BaseTools: support PCD value to use expression in the DEC file Yonghong Zhu
@ 2016-10-19 1:32 ` Gao, Liming
0 siblings, 0 replies; 9+ messages in thread
From: Gao, Liming @ 2016-10-19 1:32 UTC (permalink / raw)
To: Zhu, Yonghong, edk2-devel@lists.01.org
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Zhu, Yonghong
> Sent: Thursday, October 13, 2016 5:30 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: [Patch] BaseTools: support PCD value to use expression in the DEC
> file
>
> This patch add the support for Pcd value to use expression in the DEC file.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> ---
> BaseTools/Source/Python/Common/Misc.py | 57 ++++++++++++------
> ----
> .../Source/Python/Workspace/MetaFileParser.py | 21 ++++++--
> 2 files changed, 49 insertions(+), 29 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Common/Misc.py
> b/BaseTools/Source/Python/Common/Misc.py
> index c99716d..3be1f0f 100644
> --- a/BaseTools/Source/Python/Common/Misc.py
> +++ b/BaseTools/Source/Python/Common/Misc.py
> @@ -1410,36 +1410,11 @@ def ParseConsoleLog(Filename):
> Opw.write('%s\n' % Line)
>
> Opr.close()
> Opw.close()
>
> -## AnalyzeDscPcd
> -#
> -# Analyze DSC PCD value, since there is no data type info in DSC
> -# This fuction is used to match functions (AnalyzePcdData,
> AnalyzeHiiPcdData, AnalyzeVpdPcdData) used for retrieving PCD value from
> database
> -# 1. Feature flag: TokenSpace.PcdCName|PcdValue
> -# 2. Fix and Patch:TokenSpace.PcdCName|PcdValue[|MaxSize]
> -# 3. Dynamic default:
> -# TokenSpace.PcdCName|PcdValue[|VOID*[|MaxSize]]
> -# TokenSpace.PcdCName|PcdValue
> -# 4. Dynamic VPD:
> -# TokenSpace.PcdCName|VpdOffset[|VpdValue]
> -# TokenSpace.PcdCName|VpdOffset[|MaxSize[|VpdValue]]
> -# 5. Dynamic HII:
> -# TokenSpace.PcdCName|HiiString|VaiableGuid|VariableOffset[|HiiValue]
> -# PCD value needs to be located in such kind of string, and the PCD value
> might be an expression in which
> -# there might have "|" operator, also in string value.
> -#
> -# @param Setting: String contain information described above with
> "TokenSpace.PcdCName|" stripped
> -# @param PcdType: PCD type: feature, fixed, dynamic default VPD HII
> -# @param DataType: The datum type of PCD: VOID*, UNIT, BOOL
> -# @retval:
> -# ValueList: A List contain fields described above
> -# IsValid: True if conforming EBNF, otherwise False
> -# Index: The index where PcdValue is in ValueList
> -#
> -def AnalyzeDscPcd(Setting, PcdType, DataType=''):
> +def AnalyzePcdExpression(Setting):
> Setting = Setting.strip()
> # There might be escaped quote in a string: \", \\\"
> Data = Setting.replace('\\\\', '//').replace('\\\"', '\\\'')
> # There might be '|' in string and in ( ... | ... ), replace it with '-'
> NewStr = ''
> @@ -1465,10 +1440,40 @@ def AnalyzeDscPcd(Setting, PcdType,
> DataType=''):
> FieldList.append(Setting[StartPos:].strip())
> break
> FieldList.append(Setting[StartPos:Pos].strip())
> StartPos = Pos + 1
>
> + return FieldList
> +
> +## AnalyzeDscPcd
> +#
> +# Analyze DSC PCD value, since there is no data type info in DSC
> +# This fuction is used to match functions (AnalyzePcdData,
> AnalyzeHiiPcdData, AnalyzeVpdPcdData) used for retrieving PCD value from
> database
> +# 1. Feature flag: TokenSpace.PcdCName|PcdValue
> +# 2. Fix and Patch:TokenSpace.PcdCName|PcdValue[|MaxSize]
> +# 3. Dynamic default:
> +# TokenSpace.PcdCName|PcdValue[|VOID*[|MaxSize]]
> +# TokenSpace.PcdCName|PcdValue
> +# 4. Dynamic VPD:
> +# TokenSpace.PcdCName|VpdOffset[|VpdValue]
> +# TokenSpace.PcdCName|VpdOffset[|MaxSize[|VpdValue]]
> +# 5. Dynamic HII:
> +#
> TokenSpace.PcdCName|HiiString|VaiableGuid|VariableOffset[|HiiValue]
> +# PCD value needs to be located in such kind of string, and the PCD value
> might be an expression in which
> +# there might have "|" operator, also in string value.
> +#
> +# @param Setting: String contain information described above with
> "TokenSpace.PcdCName|" stripped
> +# @param PcdType: PCD type: feature, fixed, dynamic default VPD HII
> +# @param DataType: The datum type of PCD: VOID*, UNIT, BOOL
> +# @retval:
> +# ValueList: A List contain fields described above
> +# IsValid: True if conforming EBNF, otherwise False
> +# Index: The index where PcdValue is in ValueList
> +#
> +def AnalyzeDscPcd(Setting, PcdType, DataType=''):
> + FieldList = AnalyzePcdExpression(Setting)
> +
> IsValid = True
> if PcdType in (MODEL_PCD_FIXED_AT_BUILD,
> MODEL_PCD_PATCHABLE_IN_MODULE, MODEL_PCD_FEATURE_FLAG):
> Value = FieldList[0]
> Size = ''
> if len(FieldList) > 1:
> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> index 82d874f..1a5fdf5 100644
> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> @@ -24,11 +24,11 @@ import Common.EdkLogger as EdkLogger
> import Common.GlobalData as GlobalData
>
> from CommonDataClass.DataClass import *
> from Common.DataType import *
> from Common.String import *
> -from Common.Misc import GuidStructureStringToGuidString,
> CheckPcdDatum, PathClass, AnalyzePcdData, AnalyzeDscPcd
> +from Common.Misc import GuidStructureStringToGuidString,
> CheckPcdDatum, PathClass, AnalyzePcdData, AnalyzeDscPcd,
> AnalyzePcdExpression
> from Common.Expression import *
> from CommonDataClass.Exceptions import *
> from Common.LongFilePathSupport import OpenLongFilePath as open
>
> from MetaFileTable import MetaFileStorage
> @@ -1633,10 +1633,11 @@ class DecParser(MetaFileParser):
> return
> MetaFileParser.__init__(self, FilePath, FileType, Arch, Table, -1)
> self._Comments = []
> self._Version = 0x00010005 # Only EDK2 dec file is supported
> self._AllPCDs = [] # Only for check duplicate PCD
> + self._AllPcdDict = {}
>
> ## Parser starter
> def Start(self):
> Content = ''
> try:
> @@ -1846,14 +1847,14 @@ class DecParser(MetaFileParser):
> PtrValue = ValueRe.findall(TokenList[1])
>
> # Has VOID* type string, may contain "|" character in the string.
> if len(PtrValue) != 0:
> ptrValueList = re.sub(ValueRe, '', TokenList[1])
> - ValueList = GetSplitValueList(ptrValueList)
> + ValueList = AnalyzePcdExpression(ptrValueList)
> ValueList[0] = PtrValue[0]
> else:
> - ValueList = GetSplitValueList(TokenList[1])
> + ValueList = AnalyzePcdExpression(TokenList[1])
>
>
> # check if there's enough datum information given
> if len(ValueList) != 3:
> EdkLogger.error('Parser', FORMAT_INVALID, "Invalid PCD Datum
> information given",
> @@ -1876,10 +1877,23 @@ class DecParser(MetaFileParser):
> if ValueList[2] == '':
> EdkLogger.error('Parser', FORMAT_INVALID, "Missing Token in PCD
> Datum information",
> ExtraData=self._CurrentLine + \
> "
> (<TokenSpaceGuidCName>.<PcdCName>|<DefaultValue>|<DatumType>|<
> Token>)",
> File=self.MetaFile, Line=self._LineIndex + 1)
> +
> + PcdValue = ValueList[0]
> + if PcdValue:
> + try:
> + ValueList[0] = ValueExpression(PcdValue, self._AllPcdDict)(True)
> + except WrnExpression, Value:
> + ValueList[0] = Value.result
> +
> + if ValueList[0] == 'True':
> + ValueList[0] = '1'
> + if ValueList[0] == 'False':
> + ValueList[0] = '0'
> +
> # check format of default value against the datum type
> IsValid, Cause = CheckPcdDatum(ValueList[1], ValueList[0])
> if not IsValid:
> EdkLogger.error('Parser', FORMAT_INVALID, Cause,
> ExtraData=self._CurrentLine,
> File=self.MetaFile, Line=self._LineIndex + 1)
> @@ -1894,10 +1908,11 @@ class DecParser(MetaFileParser):
> EdkLogger.error('Parser', FORMAT_INVALID,
> "The same PCD name and GUID have been already defined",
> ExtraData=self._CurrentLine, File=self.MetaFile,
> Line=self._LineIndex + 1)
> else:
> self._AllPCDs.append((self._Scope[0], self._ValueList[0],
> self._ValueList[1]))
> + self._AllPcdDict[TAB_SPLIT.join(self._ValueList[0:2])] = ValueList[0]
>
> self._ValueList[2] = ValueList[0].strip() + '|' + ValueList[1].strip() + '|' +
> ValueList[2].strip()
>
> _SectionParser = {
> MODEL_META_DATA_HEADER : MetaFileParser._DefineParser,
> --
> 2.6.1.windows.1
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-10-19 1:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-13 9:29 [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Yonghong Zhu
2016-10-13 9:29 ` [Patch] BaseTools: support PCD value to use expression in the DEC file Yonghong Zhu
2016-10-19 1:32 ` Gao, Liming
2016-10-15 20:13 ` [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Jordan Justen
2016-10-16 13:32 ` Zhu, Yonghong
2016-10-17 5:49 ` Jordan Justen
2016-10-17 6:53 ` Zhu, Yonghong
2016-10-17 5:56 ` Jordan Justen
2016-10-17 6:43 ` Zhu, Yonghong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox