* [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: 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
* 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-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: 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
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