* [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci
@ 2020-06-03 8:48 Zhang, Shenglei
2020-06-03 8:48 ` [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Zhang, Shenglei @ 2020-06-03 8:48 UTC (permalink / raw)
To: devel; +Cc: Bob Feng, Bret Barkelew, Michael D Kinney, Liming Gao,
Sean Brogan
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606
As planed we will enable Ecc check for edk2 on open ci. And they are
ready now, but these are V2 series. So I expect that contributors in
edk2 community can try using this script when reviewing. And I appreciate
receiving feedback and comments if someone find errors or false positive
issues.
I created a pipline of EccCheck for my forked edk2.
https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10&_a=summary
The patch series are big, so the commits are also pushed into my forked tree.
https://github.com/shenglei10/edk2/commits/ECC
Patches
1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not
a pipline or script issue. But it is listed here for people willing
to try this tool.
2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run
on azure servers for open ci, or locally. Its usage is like
PatchCheck.py.
3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local
use, we need to type command
"py -3 -m pip install antlr4-python3-runtime" first.
4/5: Windows-EccCheck.yml is a yaml file to configure the newly added
pipline. The azure uses this to create a pipline.
5/5: We consider some cases that will report out Ecc issues but they won't
be fixed, like submodule and industry standard related things. So we
add two configuration fields "Exception" and "IgnoreFiles" for people
to use. The patch is a example and the contents in the fields will be
empty in final version.
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
v2: Update 2/5, fix the bug that the script can't hanlde multiple commits.
Fan, Zhiju (1):
BaseTools:ECC needs to update the contents of CParser4
Shenglei Zhang (4):
BaseTools/Scripts: Add EccCheck.py
pip-requirements.txt: Add Ecc required lib
.azurepiplines: Add a pipline to check ECC issues for commits
MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check
.azurepipelines/Windows-EccCheck.yml | 38 ++
BaseTools/Scripts/EccCheck.py | 433 ++++++++++++++++++
.../Source/Python/Ecc/CParser4/CLexer.py | 6 +-
.../Source/Python/Ecc/CParser4/CListener.py | 4 +-
.../Source/Python/Ecc/CParser4/CParser.py | 38 +-
MdeModulePkg/MdeModulePkg.ci.yaml | 8 +
pip-requirements.txt | 1 +
7 files changed, 505 insertions(+), 23 deletions(-)
create mode 100644 .azurepipelines/Windows-EccCheck.yml
create mode 100644 BaseTools/Scripts/EccCheck.py
--
2.18.0.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei @ 2020-06-03 8:48 ` Zhang, Shenglei 2020-06-05 4:38 ` [edk2-devel] " Yuwei Chen 2020-06-03 8:48 ` [PATCH v2 2/5] BaseTools/Scripts: Add EccCheck.py Zhang, Shenglei ` (5 subsequent siblings) 6 siblings, 1 reply; 12+ messages in thread From: Zhang, Shenglei @ 2020-06-03 8:48 UTC (permalink / raw) To: devel; +Cc: Fan, Zhiju, Bob Feng, Liming Gao From: "Fan, Zhiju" <zhijux.fan@intel.com> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2676 Because Ecc relies on the antlr extension package, When the antlr version is updated, we need to change the code in it. Currently, if you use the latest version antlr4.8, Ecc will fail We will update the version to support the use of antlr4.8 and all previous versions will have errors.This can be resolved by installing the antlr4.8 version. Installation method: pip install antlr4-python3-runtime==4.8 This patch is going to fixed this issue Cc: Bob Feng <bob.c.feng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com> --- .../Source/Python/Ecc/CParser4/CLexer.py | 6 +-- .../Source/Python/Ecc/CParser4/CListener.py | 4 +- .../Source/Python/Ecc/CParser4/CParser.py | 38 ++++++++++--------- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/BaseTools/Source/Python/Ecc/CParser4/CLexer.py b/BaseTools/Source/Python/Ecc/CParser4/CLexer.py index a2cc5bf56e66..40e2afbf1a1f 100644 --- a/BaseTools/Source/Python/Ecc/CParser4/CLexer.py +++ b/BaseTools/Source/Python/Ecc/CParser4/CLexer.py @@ -1,4 +1,4 @@ -# Generated from C.g4 by ANTLR 4.7.1 +# Generated from C.g4 by ANTLR 4.8 from antlr4 import * from io import StringIO from typing.io import TextIO @@ -12,7 +12,7 @@ import sys # This file is generated by running: # java org.antlr.Tool C.g # -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -584,7 +584,7 @@ class CLexer(Lexer): # @param output= sys.stdout Type: TextIO def __init__(self,input=None,output= sys.stdout): super().__init__(input, output) - self.checkVersion("4.7.1") + self.checkVersion("4.8") self._interp = LexerATNSimulator(self, self.atn, self.decisionsToDFA, PredictionContextCache()) self._actions = None self._predicates = None diff --git a/BaseTools/Source/Python/Ecc/CParser4/CListener.py b/BaseTools/Source/Python/Ecc/CParser4/CListener.py index bb4351d9249a..ba7e70892680 100644 --- a/BaseTools/Source/Python/Ecc/CParser4/CListener.py +++ b/BaseTools/Source/Python/Ecc/CParser4/CListener.py @@ -1,4 +1,4 @@ -# Generated from C.g4 by ANTLR 4.7.1 +# Generated from C.g4 by ANTLR 4.8 from antlr4 import * if __name__ is not None and "." in __name__: from .CParser import CParser @@ -12,7 +12,7 @@ else: # This file is generated by running: # java org.antlr.Tool C.g # -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # diff --git a/BaseTools/Source/Python/Ecc/CParser4/CParser.py b/BaseTools/Source/Python/Ecc/CParser4/CParser.py index 31d23d55aa57..30d70a74e669 100644 --- a/BaseTools/Source/Python/Ecc/CParser4/CParser.py +++ b/BaseTools/Source/Python/Ecc/CParser4/CParser.py @@ -1,10 +1,12 @@ -# Generated from C.g4 by ANTLR 4.7.1 +# Generated from C.g4 by ANTLR 4.8 # encoding: utf-8 from antlr4 import * from io import StringIO -from typing.io import TextIO import sys - +if sys.version_info[1] > 5: + from typing import TextIO +else: + from typing.io import TextIO ## @file # The file defines the parser for C source files. @@ -13,7 +15,7 @@ import sys # This file is generated by running: # java org.antlr.Tool C.g # -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -739,7 +741,7 @@ class CParser ( Parser ): # @param output= sys.stdout Type: TextIO def __init__(self,input,output= sys.stdout): super().__init__(input, output) - self.checkVersion("4.7.1") + self.checkVersion("4.8") self._interp = ParserATNSimulator(self, self.atn, self.decisionsToDFA, self.sharedContextCache) self._predicates = None @@ -1062,10 +1064,10 @@ class CParser ( Parser ): if localctx.d != None: - ModifierText = (None if localctx._declaration_specifiers is None else self._input.getText((localctx._declaration_specifiers.start,localctx._declaration_specifiers.stop))) + ModifierText = (None if localctx._declaration_specifiers is None else self._input.getText(localctx._declaration_specifiers.start,localctx._declaration_specifiers.stop)) else: ModifierText = '' - DeclText = (None if localctx._declarator is None else self._input.getText((localctx._declarator.start,localctx._declarator.stop))) + DeclText = (None if localctx._declarator is None else self._input.getText(localctx._declarator.start,localctx._declarator.stop)) DeclLine = (None if localctx._declarator is None else localctx._declarator.start).line DeclOffset = (None if localctx._declarator is None else localctx._declarator.start).column if localctx.a != None: @@ -1245,9 +1247,9 @@ class CParser ( Parser ): localctx.d = self.match(CParser.T__1) if localctx.b is not None: - self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 if localctx.d is None else localctx.d.line), localctx.d.column, (None if localctx.b is None else self._input.getText((localctx.b.start,localctx.b.stop))), (None if localctx.c is None else self._input.getText((localctx.c.start,localctx.c.stop)))) + self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 if localctx.d is None else localctx.d.line), localctx.d.column, (None if localctx.b is None else self._input.getText(localctx.b.start,localctx.b.stop)), (None if localctx.c is None else self._input.getText(localctx.c.start,localctx.c.stop))) else: - self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 if localctx.d is None else localctx.d.line), localctx.d.column, '', (None if localctx.c is None else self._input.getText((localctx.c.start,localctx.c.stop)))) + self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 if localctx.d is None else localctx.d.line), localctx.d.column, '', (None if localctx.c is None else self._input.getText(localctx.c.start,localctx.c.stop))) pass elif token in [CParser.T__5, CParser.T__6, CParser.T__7, CParser.T__8, CParser.T__9, CParser.T__10, CParser.T__11, CParser.T__12, CParser.T__13, CParser.T__14, CParser.T__15, CParser.T__16, CParser.T__17, CParser.T__18, CParser.T__20, CParser.T__21, CParser.T__23, CParser.T__24, CParser.T__25, CParser.T__26, CParser.T__27, CParser.T__28, CParser.T__29, CParser.T__30, CParser.T__31, CParser.T__32, CParser.T__33, CParser.T__34, CParser.T__35, CParser.T__36, CParser.IDENTIFIER]: @@ -1266,7 +1268,7 @@ class CParser ( Parser ): localctx.e = self.match(CParser.T__1) if localctx.t is not None: - self.StoreVariableDeclaration((None if localctx.s is None else localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, (None if localctx.t is None else localctx.t.start).line, (None if localctx.t is None else localctx.t.start).column, (None if localctx.s is None else self._input.getText((localctx.s.start,localctx.s.stop))), (None if localctx.t is None else self._input.getText((localctx.t.start,localctx.t.stop)))) + self.StoreVariableDeclaration((None if localctx.s is None else localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, (None if localctx.t is None else localctx.t.start).line, (None if localctx.t is None else localctx.t.start).column, (None if localctx.s is None else self._input.getText(localctx.s.start,localctx.s.stop)), (None if localctx.t is None else self._input.getText(localctx.t.start,localctx.t.stop))) pass else: @@ -1568,7 +1570,7 @@ class CParser ( Parser ): localctx.s = self.struct_or_union_specifier() if localctx.s.stop is not None: - self.StoreStructUnionDefinition((None if localctx.s is None else localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, (None if localctx.s is None else localctx.s.stop).line, (None if localctx.s is None else localctx.s.stop).column, (None if localctx.s is None else self._input.getText((localctx.s.start,localctx.s.stop)))) + self.StoreStructUnionDefinition((None if localctx.s is None else localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, (None if localctx.s is None else localctx.s.stop).line, (None if localctx.s is None else localctx.s.stop).column, (None if localctx.s is None else self._input.getText(localctx.s.start,localctx.s.stop))) pass @@ -1578,7 +1580,7 @@ class CParser ( Parser ): localctx.e = self.enum_specifier() if localctx.e.stop is not None: - self.StoreEnumerationDefinition((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText((localctx.e.start,localctx.e.stop)))) + self.StoreEnumerationDefinition((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText(localctx.e.start,localctx.e.stop))) pass @@ -4022,7 +4024,7 @@ class CParser ( Parser ): self.enterOuterAlt(localctx, 1) self.state = 569 localctx.p = self.primary_expression() - self.FuncCallText += (None if localctx.p is None else self._input.getText((localctx.p.start,localctx.p.stop))) + self.FuncCallText += (None if localctx.p is None else self._input.getText(localctx.p.start,localctx.p.stop)) self.state = 600 self._errHandler.sync(self) _alt = self._interp.adaptivePredict(self._input,73,self._ctx) @@ -4055,7 +4057,7 @@ class CParser ( Parser ): localctx.c = self.argument_expression_list() self.state = 580 localctx.b = self.match(CParser.T__38) - self.StoreFunctionCalling((None if localctx.p is None else localctx.p.start).line, (None if localctx.p is None else localctx.p.start).column, (0 if localctx.b is None else localctx.b.line), localctx.b.column, self.FuncCallText, (None if localctx.c is None else self._input.getText((localctx.c.start,localctx.c.stop)))) + self.StoreFunctionCalling((None if localctx.p is None else localctx.p.start).line, (None if localctx.p is None else localctx.p.start).column, (0 if localctx.b is None else localctx.b.line), localctx.b.column, self.FuncCallText, (None if localctx.c is None else self._input.getText(localctx.c.start,localctx.c.stop))) pass elif la_ == 4: @@ -4770,7 +4772,7 @@ class CParser ( Parser ): self.match(CParser.T__22) self.state = 674 self.conditional_expression() - self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText((localctx.e.start,localctx.e.stop)))) + self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText(localctx.e.start,localctx.e.stop))) except RecognitionException as re: @@ -6053,7 +6055,7 @@ class CParser ( Parser ): localctx.e = self.expression() self.state = 845 self.match(CParser.T__38) - self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText((localctx.e.start,localctx.e.stop)))) + self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText(localctx.e.start,localctx.e.stop))) self.state = 847 self.statement() self.state = 850 @@ -6144,7 +6146,7 @@ class CParser ( Parser ): self.match(CParser.T__38) self.state = 864 self.statement() - self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText((localctx.e.start,localctx.e.stop)))) + self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText(localctx.e.start,localctx.e.stop))) pass elif token in [CParser.T__87]: self.enterOuterAlt(localctx, 2) @@ -6162,7 +6164,7 @@ class CParser ( Parser ): self.match(CParser.T__38) self.state = 873 self.match(CParser.T__1) - self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText((localctx.e.start,localctx.e.stop)))) + self.StorePredicateExpression((None if localctx.e is None else localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is None else localctx.e.stop).column, (None if localctx.e is None else self._input.getText(localctx.e.start,localctx.e.stop))) pass else: raise NoViableAltException(self) -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 2020-06-03 8:48 ` [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei @ 2020-06-05 4:38 ` Yuwei Chen 2020-06-05 5:11 ` Zhang, Shenglei 0 siblings, 1 reply; 12+ messages in thread From: Yuwei Chen @ 2020-06-05 4:38 UTC (permalink / raw) To: devel@edk2.groups.io, Zhang, Shenglei Cc: Fan, ZhijuX, Feng, Bob C, Gao, Liming Hi, Shenglei Since the input arguments number of the getText() function changed, the build failed. May be you should check about the getText() function's input. While how to choose the CParser4 and CParser3, I am not familiar with it, would you like to explain for me? Thanks, Yuwei > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang, > Shenglei > Sent: Wednesday, June 3, 2020 4:48 PM > To: devel@edk2.groups.io > Cc: Fan, ZhijuX <zhijux.fan@intel.com>; Feng, Bob C > <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com> > Subject: [edk2-devel] [PATCH v2 1/5] BaseTools:ECC needs to update the > contents of CParser4 > > From: "Fan, Zhiju" <zhijux.fan@intel.com> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2676 > > Because Ecc relies on the antlr extension package, When the antlr version is > updated, we need to change the code in it. > Currently, if you use the latest version antlr4.8, Ecc will fail > > We will update the version to support the use of antlr4.8 and all previous > versions will have errors.This can be resolved by installing the antlr4.8 version. > Installation method: pip install antlr4-python3-runtime==4.8 > > This patch is going to fixed this issue > > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com> > --- > .../Source/Python/Ecc/CParser4/CLexer.py | 6 +-- > .../Source/Python/Ecc/CParser4/CListener.py | 4 +- > .../Source/Python/Ecc/CParser4/CParser.py | 38 ++++++++++--------- > 3 files changed, 25 insertions(+), 23 deletions(-) > > diff --git a/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > b/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > index a2cc5bf56e66..40e2afbf1a1f 100644 > --- a/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > +++ b/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > @@ -1,4 +1,4 @@ > -# Generated from C.g4 by ANTLR 4.7.1 > +# Generated from C.g4 by ANTLR 4.8 > from antlr4 import * > from io import StringIO > from typing.io import TextIO > @@ -12,7 +12,7 @@ import sys > # This file is generated by running: > # java org.antlr.Tool C.g > # > -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. > +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -584,7 +584,7 @@ > class CLexer(Lexer): > # @param output= sys.stdout Type: TextIO > def __init__(self,input=None,output= sys.stdout): > super().__init__(input, output) > - self.checkVersion("4.7.1") > + self.checkVersion("4.8") > self._interp = LexerATNSimulator(self, self.atn, self.decisionsToDFA, > PredictionContextCache()) > self._actions = None > self._predicates = None > diff --git a/BaseTools/Source/Python/Ecc/CParser4/CListener.py > b/BaseTools/Source/Python/Ecc/CParser4/CListener.py > index bb4351d9249a..ba7e70892680 100644 > --- a/BaseTools/Source/Python/Ecc/CParser4/CListener.py > +++ b/BaseTools/Source/Python/Ecc/CParser4/CListener.py > @@ -1,4 +1,4 @@ > -# Generated from C.g4 by ANTLR 4.7.1 > +# Generated from C.g4 by ANTLR 4.8 > from antlr4 import * > if __name__ is not None and "." in __name__: > from .CParser import CParser > @@ -12,7 +12,7 @@ else: > # This file is generated by running: > # java org.antlr.Tool C.g > # > -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. > +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # diff --git > a/BaseTools/Source/Python/Ecc/CParser4/CParser.py > b/BaseTools/Source/Python/Ecc/CParser4/CParser.py > index 31d23d55aa57..30d70a74e669 100644 > --- a/BaseTools/Source/Python/Ecc/CParser4/CParser.py > +++ b/BaseTools/Source/Python/Ecc/CParser4/CParser.py > @@ -1,10 +1,12 @@ > -# Generated from C.g4 by ANTLR 4.7.1 > +# Generated from C.g4 by ANTLR 4.8 > # encoding: utf-8 > from antlr4 import * > from io import StringIO > -from typing.io import TextIO > import sys > - > +if sys.version_info[1] > 5: > + from typing import TextIO > +else: > + from typing.io import TextIO > > ## @file > # The file defines the parser for C source files. > @@ -13,7 +15,7 @@ import sys > # This file is generated by running: > # java org.antlr.Tool C.g > # > -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. > +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -739,7 +741,7 @@ > class CParser ( Parser ): > # @param output= sys.stdout Type: TextIO > def __init__(self,input,output= sys.stdout): > super().__init__(input, output) > - self.checkVersion("4.7.1") > + self.checkVersion("4.8") > self._interp = ParserATNSimulator(self, self.atn, self.decisionsToDFA, > self.sharedContextCache) > self._predicates = None > > @@ -1062,10 +1064,10 @@ class CParser ( Parser ): > > > if localctx.d != None: > - ModifierText = (None if localctx._declaration_specifiers is None else > self._input.getText((localctx._declaration_specifiers.start,localctx._declarati > on_specifiers.stop))) > + ModifierText = (None if > + localctx._declaration_specifiers is None else > + self._input.getText(localctx._declaration_specifiers.start,localctx._d > + eclaration_specifiers.stop)) > else: > ModifierText = '' > - DeclText = (None if localctx._declarator is None else > self._input.getText((localctx._declarator.start,localctx._declarator.stop))) > + DeclText = (None if localctx._declarator is None else > + self._input.getText(localctx._declarator.start,localctx._declarator.st > + op)) > DeclLine = (None if localctx._declarator is None else > localctx._declarator.start).line > DeclOffset = (None if localctx._declarator is None else > localctx._declarator.start).column > if localctx.a != None: > @@ -1245,9 +1247,9 @@ class CParser ( Parser ): > localctx.d = self.match(CParser.T__1) > > if localctx.b is not None: > - self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 if > localctx.d is None else localctx.d.line), localctx.d.column, (None if localctx.b is > None else self._input.getText((localctx.b.start,localctx.b.stop))), (None if > localctx.c is None else self._input.getText((localctx.c.start,localctx.c.stop)))) > + self.StoreTypedefDefinition(localctx.a.line, > + localctx.a.column, (0 if localctx.d is None else localctx.d.line), > + localctx.d.column, (None if localctx.b is None else > + self._input.getText(localctx.b.start,localctx.b.stop)), (None if > + localctx.c is None else > + self._input.getText(localctx.c.start,localctx.c.stop))) > else: > - self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 if > localctx.d is None else localctx.d.line), localctx.d.column, '', (None if localctx.c > is None else self._input.getText((localctx.c.start,localctx.c.stop)))) > + self.StoreTypedefDefinition(localctx.a.line, > + localctx.a.column, (0 if localctx.d is None else localctx.d.line), > + localctx.d.column, '', (None if localctx.c is None else > + self._input.getText(localctx.c.start,localctx.c.stop))) > > pass > elif token in [CParser.T__5, CParser.T__6, CParser.T__7, CParser.T__8, > CParser.T__9, CParser.T__10, CParser.T__11, CParser.T__12, CParser.T__13, > CParser.T__14, CParser.T__15, CParser.T__16, CParser.T__17, CParser.T__18, > CParser.T__20, CParser.T__21, CParser.T__23, CParser.T__24, CParser.T__25, > CParser.T__26, CParser.T__27, CParser.T__28, CParser.T__29, CParser.T__30, > CParser.T__31, CParser.T__32, CParser.T__33, CParser.T__34, CParser.T__35, > CParser.T__36, CParser.IDENTIFIER]: > @@ -1266,7 +1268,7 @@ class CParser ( Parser ): > localctx.e = self.match(CParser.T__1) > > if localctx.t is not None: > - self.StoreVariableDeclaration((None if localctx.s is None else > localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, > (None if localctx.t is None else localctx.t.start).line, (None if localctx.t is None > else localctx.t.start).column, (None if localctx.s is None else > self._input.getText((localctx.s.start,localctx.s.stop))), (None if localctx.t is > None else self._input.getText((localctx.t.start,localctx.t.stop)))) > + self.StoreVariableDeclaration((None if localctx.s > + is None else localctx.s.start).line, (None if localctx.s is None else > + localctx.s.start).column, (None if localctx.t is None else > + localctx.t.start).line, (None if localctx.t is None else > + localctx.t.start).column, (None if localctx.s is None else > + self._input.getText(localctx.s.start,localctx.s.stop)), (None if > + localctx.t is None else > + self._input.getText(localctx.t.start,localctx.t.stop))) > > pass > else: > @@ -1568,7 +1570,7 @@ class CParser ( Parser ): > localctx.s = self.struct_or_union_specifier() > > if localctx.s.stop is not None: > - self.StoreStructUnionDefinition((None if localctx.s is None else > localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, > (None if localctx.s is None else localctx.s.stop).line, (None if localctx.s is None > else localctx.s.stop).column, (None if localctx.s is None else > self._input.getText((localctx.s.start,localctx.s.stop)))) > + self.StoreStructUnionDefinition((None if localctx.s > + is None else localctx.s.start).line, (None if localctx.s is None else > + localctx.s.start).column, (None if localctx.s is None else > + localctx.s.stop).line, (None if localctx.s is None else > + localctx.s.stop).column, (None if localctx.s is None else > + self._input.getText(localctx.s.start,localctx.s.stop))) > > pass > > @@ -1578,7 +1580,7 @@ class CParser ( Parser ): > localctx.e = self.enum_specifier() > > if localctx.e.stop is not None: > - self.StoreEnumerationDefinition((None if localctx.e is None else > localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > None else localctx.e.stop).column, (None if localctx.e is None else > self._input.getText((localctx.e.start,localctx.e.stop)))) > + self.StoreEnumerationDefinition((None if localctx.e > + is None else localctx.e.start).line, (None if localctx.e is None else > + localctx.e.start).column, (None if localctx.e is None else > + localctx.e.stop).line, (None if localctx.e is None else > + localctx.e.stop).column, (None if localctx.e is None else > + self._input.getText(localctx.e.start,localctx.e.stop))) > > pass > > @@ -4022,7 +4024,7 @@ class CParser ( Parser ): > self.enterOuterAlt(localctx, 1) > self.state = 569 > localctx.p = self.primary_expression() > - self.FuncCallText += (None if localctx.p is None else > self._input.getText((localctx.p.start,localctx.p.stop))) > + self.FuncCallText += (None if localctx.p is None else > + self._input.getText(localctx.p.start,localctx.p.stop)) > self.state = 600 > self._errHandler.sync(self) > _alt = self._interp.adaptivePredict(self._input,73,self._ctx) > @@ -4055,7 +4057,7 @@ class CParser ( Parser ): > localctx.c = self.argument_expression_list() > self.state = 580 > localctx.b = self.match(CParser.T__38) > - self.StoreFunctionCalling((None if localctx.p is None else > localctx.p.start).line, (None if localctx.p is None else localctx.p.start).column, > (0 if localctx.b is None else localctx.b.line), localctx.b.column, > self.FuncCallText, (None if localctx.c is None else > self._input.getText((localctx.c.start,localctx.c.stop)))) > + self.StoreFunctionCalling((None if localctx.p > + is None else localctx.p.start).line, (None if localctx.p is None else > + localctx.p.start).column, (0 if localctx.b is None else > + localctx.b.line), localctx.b.column, self.FuncCallText, (None if > + localctx.c is None else > + self._input.getText(localctx.c.start,localctx.c.stop))) > pass > > elif la_ == 4: > @@ -4770,7 +4772,7 @@ class CParser ( Parser ): > self.match(CParser.T__22) > self.state = 674 > self.conditional_expression() > - self.StorePredicateExpression((None if localctx.e is None else > localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > None else localctx.e.stop).column, (None if localctx.e is None else > self._input.getText((localctx.e.start,localctx.e.stop)))) > + self.StorePredicateExpression((None if localctx.e is > + None else localctx.e.start).line, (None if localctx.e is None else > + localctx.e.start).column, (None if localctx.e is None else > + localctx.e.stop).line, (None if localctx.e is None else > + localctx.e.stop).column, (None if localctx.e is None else > + self._input.getText(localctx.e.start,localctx.e.stop))) > > > except RecognitionException as re: > @@ -6053,7 +6055,7 @@ class CParser ( Parser ): > localctx.e = self.expression() > self.state = 845 > self.match(CParser.T__38) > - self.StorePredicateExpression((None if localctx.e is None else > localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > None else localctx.e.stop).column, (None if localctx.e is None else > self._input.getText((localctx.e.start,localctx.e.stop)))) > + self.StorePredicateExpression((None if localctx.e is > + None else localctx.e.start).line, (None if localctx.e is None else > + localctx.e.start).column, (None if localctx.e is None else > + localctx.e.stop).line, (None if localctx.e is None else > + localctx.e.stop).column, (None if localctx.e is None else > + self._input.getText(localctx.e.start,localctx.e.stop))) > self.state = 847 > self.statement() > self.state = 850 > @@ -6144,7 +6146,7 @@ class CParser ( Parser ): > self.match(CParser.T__38) > self.state = 864 > self.statement() > - self.StorePredicateExpression((None if localctx.e is None else > localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > None else localctx.e.stop).column, (None if localctx.e is None else > self._input.getText((localctx.e.start,localctx.e.stop)))) > + self.StorePredicateExpression((None if localctx.e is > + None else localctx.e.start).line, (None if localctx.e is None else > + localctx.e.start).column, (None if localctx.e is None else > + localctx.e.stop).line, (None if localctx.e is None else > + localctx.e.stop).column, (None if localctx.e is None else > + self._input.getText(localctx.e.start,localctx.e.stop))) > pass > elif token in [CParser.T__87]: > self.enterOuterAlt(localctx, 2) @@ -6162,7 +6164,7 @@ class CParser > ( Parser ): > self.match(CParser.T__38) > self.state = 873 > self.match(CParser.T__1) > - self.StorePredicateExpression((None if localctx.e is None else > localctx.e.start).line, (None if localctx.e is None else localctx.e.start).column, > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > None else localctx.e.stop).column, (None if localctx.e is None else > self._input.getText((localctx.e.start,localctx.e.stop)))) > + self.StorePredicateExpression((None if localctx.e is > + None else localctx.e.start).line, (None if localctx.e is None else > + localctx.e.start).column, (None if localctx.e is None else > + localctx.e.stop).line, (None if localctx.e is None else > + localctx.e.stop).column, (None if localctx.e is None else > + self._input.getText(localctx.e.start,localctx.e.stop))) > pass > else: > raise NoViableAltException(self) > -- > 2.18.0.windows.1 > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 2020-06-05 4:38 ` [edk2-devel] " Yuwei Chen @ 2020-06-05 5:11 ` Zhang, Shenglei 0 siblings, 0 replies; 12+ messages in thread From: Zhang, Shenglei @ 2020-06-05 5:11 UTC (permalink / raw) To: Chen, Yuwei, devel@edk2.groups.io, Fan, ZhijuX; +Cc: Feng, Bob C, Gao, Liming Yuwei, I'm also not familiar with that. I attached this patch in patch series because people can't run Ecc with py38 if they want to use my script. Zhiju is the original author. Zhiju, Could you help answer Yuwei's question? Thanks, Shenglei > -----Original Message----- > From: Chen, Yuwei > Sent: Friday, June 5, 2020 12:39 PM > To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@intel.com> > Cc: Fan, ZhijuX <zhijux.fan@intel.com>; Feng, Bob C > <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com> > Subject: RE: [edk2-devel] [PATCH v2 1/5] BaseTools:ECC needs to update the > contents of CParser4 > > Hi, Shenglei > > Since the input arguments number of the getText() function changed, the > build failed. May be you should check about the getText() function's input. > While how to choose the CParser4 and CParser3, I am not familiar with it, > would you like to explain for me? > > Thanks, > Yuwei > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang, > > Shenglei > > Sent: Wednesday, June 3, 2020 4:48 PM > > To: devel@edk2.groups.io > > Cc: Fan, ZhijuX <zhijux.fan@intel.com>; Feng, Bob C > > <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com> > > Subject: [edk2-devel] [PATCH v2 1/5] BaseTools:ECC needs to update the > > contents of CParser4 > > > > From: "Fan, Zhiju" <zhijux.fan@intel.com> > > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2676 > > > > Because Ecc relies on the antlr extension package, When the antlr version is > > updated, we need to change the code in it. > > Currently, if you use the latest version antlr4.8, Ecc will fail > > > > We will update the version to support the use of antlr4.8 and all previous > > versions will have errors.This can be resolved by installing the antlr4.8 > version. > > Installation method: pip install antlr4-python3-runtime==4.8 > > > > This patch is going to fixed this issue > > > > Cc: Bob Feng <bob.c.feng@intel.com> > > Cc: Liming Gao <liming.gao@intel.com> > > Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com> > > --- > > .../Source/Python/Ecc/CParser4/CLexer.py | 6 +-- > > .../Source/Python/Ecc/CParser4/CListener.py | 4 +- > > .../Source/Python/Ecc/CParser4/CParser.py | 38 ++++++++++--------- > > 3 files changed, 25 insertions(+), 23 deletions(-) > > > > diff --git a/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > > b/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > > index a2cc5bf56e66..40e2afbf1a1f 100644 > > --- a/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > > +++ b/BaseTools/Source/Python/Ecc/CParser4/CLexer.py > > @@ -1,4 +1,4 @@ > > -# Generated from C.g4 by ANTLR 4.7.1 > > +# Generated from C.g4 by ANTLR 4.8 > > from antlr4 import * > > from io import StringIO > > from typing.io import TextIO > > @@ -12,7 +12,7 @@ import sys > > # This file is generated by running: > > # java org.antlr.Tool C.g > > # > > -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. > > +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -584,7 +584,7 @@ > > class CLexer(Lexer): > > # @param output= sys.stdout Type: TextIO > > def __init__(self,input=None,output= sys.stdout): > > super().__init__(input, output) > > - self.checkVersion("4.7.1") > > + self.checkVersion("4.8") > > self._interp = LexerATNSimulator(self, self.atn, self.decisionsToDFA, > > PredictionContextCache()) > > self._actions = None > > self._predicates = None > > diff --git a/BaseTools/Source/Python/Ecc/CParser4/CListener.py > > b/BaseTools/Source/Python/Ecc/CParser4/CListener.py > > index bb4351d9249a..ba7e70892680 100644 > > --- a/BaseTools/Source/Python/Ecc/CParser4/CListener.py > > +++ b/BaseTools/Source/Python/Ecc/CParser4/CListener.py > > @@ -1,4 +1,4 @@ > > -# Generated from C.g4 by ANTLR 4.7.1 > > +# Generated from C.g4 by ANTLR 4.8 > > from antlr4 import * > > if __name__ is not None and "." in __name__: > > from .CParser import CParser > > @@ -12,7 +12,7 @@ else: > > # This file is generated by running: > > # java org.antlr.Tool C.g > > # > > -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. > > +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # diff --git > > a/BaseTools/Source/Python/Ecc/CParser4/CParser.py > > b/BaseTools/Source/Python/Ecc/CParser4/CParser.py > > index 31d23d55aa57..30d70a74e669 100644 > > --- a/BaseTools/Source/Python/Ecc/CParser4/CParser.py > > +++ b/BaseTools/Source/Python/Ecc/CParser4/CParser.py > > @@ -1,10 +1,12 @@ > > -# Generated from C.g4 by ANTLR 4.7.1 > > +# Generated from C.g4 by ANTLR 4.8 > > # encoding: utf-8 > > from antlr4 import * > > from io import StringIO > > -from typing.io import TextIO > > import sys > > - > > +if sys.version_info[1] > 5: > > + from typing import TextIO > > +else: > > + from typing.io import TextIO > > > > ## @file > > # The file defines the parser for C source files. > > @@ -13,7 +15,7 @@ import sys > > # This file is generated by running: > > # java org.antlr.Tool C.g > > # > > -# Copyright (c) 2009 - 2010, Intel Corporation All rights reserved. > > +# Copyright (c) 2009 - 2020, Intel Corporation All rights reserved. > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -739,7 +741,7 @@ > > class CParser ( Parser ): > > # @param output= sys.stdout Type: TextIO > > def __init__(self,input,output= sys.stdout): > > super().__init__(input, output) > > - self.checkVersion("4.7.1") > > + self.checkVersion("4.8") > > self._interp = ParserATNSimulator(self, self.atn, self.decisionsToDFA, > > self.sharedContextCache) > > self._predicates = None > > > > @@ -1062,10 +1064,10 @@ class CParser ( Parser ): > > > > > > if localctx.d != None: > > - ModifierText = (None if localctx._declaration_specifiers is None > else > > > self._input.getText((localctx._declaration_specifiers.start,localctx._declarati > > on_specifiers.stop))) > > + ModifierText = (None if > > + localctx._declaration_specifiers is None else > > + self._input.getText(localctx._declaration_specifiers.start,localctx._d > > + eclaration_specifiers.stop)) > > else: > > ModifierText = '' > > - DeclText = (None if localctx._declarator is None else > > self._input.getText((localctx._declarator.start,localctx._declarator.stop))) > > + DeclText = (None if localctx._declarator is None else > > + self._input.getText(localctx._declarator.start,localctx._declarator.st > > + op)) > > DeclLine = (None if localctx._declarator is None else > > localctx._declarator.start).line > > DeclOffset = (None if localctx._declarator is None else > > localctx._declarator.start).column > > if localctx.a != None: > > @@ -1245,9 +1247,9 @@ class CParser ( Parser ): > > localctx.d = self.match(CParser.T__1) > > > > if localctx.b is not None: > > - self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 > if > > localctx.d is None else localctx.d.line), localctx.d.column, (None if localctx.b > is > > None else self._input.getText((localctx.b.start,localctx.b.stop))), (None if > > localctx.c is None else self._input.getText((localctx.c.start,localctx.c.stop)))) > > + self.StoreTypedefDefinition(localctx.a.line, > > + localctx.a.column, (0 if localctx.d is None else localctx.d.line), > > + localctx.d.column, (None if localctx.b is None else > > + self._input.getText(localctx.b.start,localctx.b.stop)), (None if > > + localctx.c is None else > > + self._input.getText(localctx.c.start,localctx.c.stop))) > > else: > > - self.StoreTypedefDefinition(localctx.a.line, localctx.a.column, (0 > if > > localctx.d is None else localctx.d.line), localctx.d.column, '', (None if > localctx.c > > is None else self._input.getText((localctx.c.start,localctx.c.stop)))) > > + self.StoreTypedefDefinition(localctx.a.line, > > + localctx.a.column, (0 if localctx.d is None else localctx.d.line), > > + localctx.d.column, '', (None if localctx.c is None else > > + self._input.getText(localctx.c.start,localctx.c.stop))) > > > > pass > > elif token in [CParser.T__5, CParser.T__6, CParser.T__7, > CParser.T__8, > > CParser.T__9, CParser.T__10, CParser.T__11, CParser.T__12, > CParser.T__13, > > CParser.T__14, CParser.T__15, CParser.T__16, CParser.T__17, > CParser.T__18, > > CParser.T__20, CParser.T__21, CParser.T__23, CParser.T__24, > CParser.T__25, > > CParser.T__26, CParser.T__27, CParser.T__28, CParser.T__29, > CParser.T__30, > > CParser.T__31, CParser.T__32, CParser.T__33, CParser.T__34, > CParser.T__35, > > CParser.T__36, CParser.IDENTIFIER]: > > @@ -1266,7 +1268,7 @@ class CParser ( Parser ): > > localctx.e = self.match(CParser.T__1) > > > > if localctx.t is not None: > > - self.StoreVariableDeclaration((None if localctx.s is None else > > localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, > > (None if localctx.t is None else localctx.t.start).line, (None if localctx.t is > None > > else localctx.t.start).column, (None if localctx.s is None else > > self._input.getText((localctx.s.start,localctx.s.stop))), (None if localctx.t is > > None else self._input.getText((localctx.t.start,localctx.t.stop)))) > > + self.StoreVariableDeclaration((None if localctx.s > > + is None else localctx.s.start).line, (None if localctx.s is None else > > + localctx.s.start).column, (None if localctx.t is None else > > + localctx.t.start).line, (None if localctx.t is None else > > + localctx.t.start).column, (None if localctx.s is None else > > + self._input.getText(localctx.s.start,localctx.s.stop)), (None if > > + localctx.t is None else > > + self._input.getText(localctx.t.start,localctx.t.stop))) > > > > pass > > else: > > @@ -1568,7 +1570,7 @@ class CParser ( Parser ): > > localctx.s = self.struct_or_union_specifier() > > > > if localctx.s.stop is not None: > > - self.StoreStructUnionDefinition((None if localctx.s is None else > > localctx.s.start).line, (None if localctx.s is None else localctx.s.start).column, > > (None if localctx.s is None else localctx.s.stop).line, (None if localctx.s is > None > > else localctx.s.stop).column, (None if localctx.s is None else > > self._input.getText((localctx.s.start,localctx.s.stop)))) > > + self.StoreStructUnionDefinition((None if localctx.s > > + is None else localctx.s.start).line, (None if localctx.s is None else > > + localctx.s.start).column, (None if localctx.s is None else > > + localctx.s.stop).line, (None if localctx.s is None else > > + localctx.s.stop).column, (None if localctx.s is None else > > + self._input.getText(localctx.s.start,localctx.s.stop))) > > > > pass > > > > @@ -1578,7 +1580,7 @@ class CParser ( Parser ): > > localctx.e = self.enum_specifier() > > > > if localctx.e.stop is not None: > > - self.StoreEnumerationDefinition((None if localctx.e is None else > > localctx.e.start).line, (None if localctx.e is None else > localctx.e.start).column, > > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > > None else localctx.e.stop).column, (None if localctx.e is None else > > self._input.getText((localctx.e.start,localctx.e.stop)))) > > + self.StoreEnumerationDefinition((None if localctx.e > > + is None else localctx.e.start).line, (None if localctx.e is None else > > + localctx.e.start).column, (None if localctx.e is None else > > + localctx.e.stop).line, (None if localctx.e is None else > > + localctx.e.stop).column, (None if localctx.e is None else > > + self._input.getText(localctx.e.start,localctx.e.stop))) > > > > pass > > > > @@ -4022,7 +4024,7 @@ class CParser ( Parser ): > > self.enterOuterAlt(localctx, 1) > > self.state = 569 > > localctx.p = self.primary_expression() > > - self.FuncCallText += (None if localctx.p is None else > > self._input.getText((localctx.p.start,localctx.p.stop))) > > + self.FuncCallText += (None if localctx.p is None else > > + self._input.getText(localctx.p.start,localctx.p.stop)) > > self.state = 600 > > self._errHandler.sync(self) > > _alt = self._interp.adaptivePredict(self._input,73,self._ctx) > > @@ -4055,7 +4057,7 @@ class CParser ( Parser ): > > localctx.c = self.argument_expression_list() > > self.state = 580 > > localctx.b = self.match(CParser.T__38) > > - self.StoreFunctionCalling((None if localctx.p is None else > > localctx.p.start).line, (None if localctx.p is None else > localctx.p.start).column, > > (0 if localctx.b is None else localctx.b.line), localctx.b.column, > > self.FuncCallText, (None if localctx.c is None else > > self._input.getText((localctx.c.start,localctx.c.stop)))) > > + self.StoreFunctionCalling((None if localctx.p > > + is None else localctx.p.start).line, (None if localctx.p is None else > > + localctx.p.start).column, (0 if localctx.b is None else > > + localctx.b.line), localctx.b.column, self.FuncCallText, (None if > > + localctx.c is None else > > + self._input.getText(localctx.c.start,localctx.c.stop))) > > pass > > > > elif la_ == 4: > > @@ -4770,7 +4772,7 @@ class CParser ( Parser ): > > self.match(CParser.T__22) > > self.state = 674 > > self.conditional_expression() > > - self.StorePredicateExpression((None if localctx.e is None else > > localctx.e.start).line, (None if localctx.e is None else > localctx.e.start).column, > > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > > None else localctx.e.stop).column, (None if localctx.e is None else > > self._input.getText((localctx.e.start,localctx.e.stop)))) > > + self.StorePredicateExpression((None if localctx.e is > > + None else localctx.e.start).line, (None if localctx.e is None else > > + localctx.e.start).column, (None if localctx.e is None else > > + localctx.e.stop).line, (None if localctx.e is None else > > + localctx.e.stop).column, (None if localctx.e is None else > > + self._input.getText(localctx.e.start,localctx.e.stop))) > > > > > > except RecognitionException as re: > > @@ -6053,7 +6055,7 @@ class CParser ( Parser ): > > localctx.e = self.expression() > > self.state = 845 > > self.match(CParser.T__38) > > - self.StorePredicateExpression((None if localctx.e is None else > > localctx.e.start).line, (None if localctx.e is None else > localctx.e.start).column, > > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > > None else localctx.e.stop).column, (None if localctx.e is None else > > self._input.getText((localctx.e.start,localctx.e.stop)))) > > + self.StorePredicateExpression((None if localctx.e is > > + None else localctx.e.start).line, (None if localctx.e is None else > > + localctx.e.start).column, (None if localctx.e is None else > > + localctx.e.stop).line, (None if localctx.e is None else > > + localctx.e.stop).column, (None if localctx.e is None else > > + self._input.getText(localctx.e.start,localctx.e.stop))) > > self.state = 847 > > self.statement() > > self.state = 850 > > @@ -6144,7 +6146,7 @@ class CParser ( Parser ): > > self.match(CParser.T__38) > > self.state = 864 > > self.statement() > > - self.StorePredicateExpression((None if localctx.e is None else > > localctx.e.start).line, (None if localctx.e is None else > localctx.e.start).column, > > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > > None else localctx.e.stop).column, (None if localctx.e is None else > > self._input.getText((localctx.e.start,localctx.e.stop)))) > > + self.StorePredicateExpression((None if localctx.e is > > + None else localctx.e.start).line, (None if localctx.e is None else > > + localctx.e.start).column, (None if localctx.e is None else > > + localctx.e.stop).line, (None if localctx.e is None else > > + localctx.e.stop).column, (None if localctx.e is None else > > + self._input.getText(localctx.e.start,localctx.e.stop))) > > pass > > elif token in [CParser.T__87]: > > self.enterOuterAlt(localctx, 2) @@ -6162,7 +6164,7 @@ class > CParser > > ( Parser ): > > self.match(CParser.T__38) > > self.state = 873 > > self.match(CParser.T__1) > > - self.StorePredicateExpression((None if localctx.e is None else > > localctx.e.start).line, (None if localctx.e is None else > localctx.e.start).column, > > (None if localctx.e is None else localctx.e.stop).line, (None if localctx.e is > > None else localctx.e.stop).column, (None if localctx.e is None else > > self._input.getText((localctx.e.start,localctx.e.stop)))) > > + self.StorePredicateExpression((None if localctx.e is > > + None else localctx.e.start).line, (None if localctx.e is None else > > + localctx.e.start).column, (None if localctx.e is None else > > + localctx.e.stop).line, (None if localctx.e is None else > > + localctx.e.stop).column, (None if localctx.e is None else > > + self._input.getText(localctx.e.start,localctx.e.stop))) > > pass > > else: > > raise NoViableAltException(self) > > -- > > 2.18.0.windows.1 > > > > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/5] BaseTools/Scripts: Add EccCheck.py 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei @ 2020-06-03 8:48 ` Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 3/5] pip-requirements.txt: Add Ecc required lib Zhang, Shenglei ` (4 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Zhang, Shenglei @ 2020-06-03 8:48 UTC (permalink / raw) To: devel; +Cc: Bob Feng, Liming Gao REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606 EccCheck.py is a tool to report Ecc issues for commits, which will be run on open ci. But note not each kind of issue could be reported out. It can only handle the issues, whose line number in CSV report accurately map with their code in source code files. And comment issues can also be handled. Its usage is similar to PatchCheck.py. Type EccCheck.py -h and then learn how to use it. If a patch passes EccCheck, "Ecc Pass" will show up. Otherwise, "Ecc error detected" alerts the users and the details are also presented. Cc: Bob Feng <bob.c.feng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> --- v2: Update 2/5, fix the bug that the script can't hanlde multiple commits. BaseTools/Scripts/EccCheck.py | 433 ++++++++++++++++++++++++++++++++++ 1 file changed, 433 insertions(+) create mode 100644 BaseTools/Scripts/EccCheck.py diff --git a/BaseTools/Scripts/EccCheck.py b/BaseTools/Scripts/EccCheck.py new file mode 100644 index 000000000000..034a9965a79b --- /dev/null +++ b/BaseTools/Scripts/EccCheck.py @@ -0,0 +1,433 @@ +## @file +# Check a patch for various format issues +# +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# + +import os +import re +import csv +import subprocess +import argparse +import sys +import yaml +import xml.dom.minidom + +__copyright__ = "Copyright (c) 2020, Intel Corporation All rights reserved." +ReModifyFile = re.compile(r'[B-Q,S-Z]+[\d]*\t(.*?)\n') +FindModifyFile = re.compile(r'\+\+\+ b\/(.*)') +LineScopePattern = (r'@@ -\d*\,*\d* \+\d*\,*\d* @@.*') +LineNumRange = re.compile(r'@@ -\d*\,*\d* \+(\d*)\,*(\d*) @@.*') + +EnvList = os.environ +GlobalSymbol = {} + +def AppendException(ExceptionList, ExceptionFile): + error_code_list = ExceptionList[::2] + keyword_list = ExceptionList[1::2] + domTree = xml.dom.minidom.parse(ExceptionFile) + rootNode = domTree.documentElement + for error_code, keyword in zip(error_code_list, keyword_list): + customer_node = domTree.createElement("Exception") + keyword_node = domTree.createElement("KeyWord") + keyword_node_text_value = domTree.createTextNode(keyword) + keyword_node.appendChild(keyword_node_text_value) + customer_node.appendChild(keyword_node) + error_code_node = domTree.createElement("ErrorID") + error_code_text_value = domTree.createTextNode(error_code) + error_code_node.appendChild(error_code_text_value) + customer_node.appendChild(error_code_node) + rootNode.appendChild(customer_node) + + with open(ExceptionFile, 'w') as f: + domTree.writexml(f,indent='',addindent='',newl='\n',encoding='UTF-8') + +def GetPkgList(EnvList): + WORKDIR = EnvList['WORKDIR'] + dirs = os.listdir(WORKDIR) + PkgList = [] + for directory in dirs: + if directory.endswith('Pkg'): + PkgList.append(directory) + return PkgList + +def GenerateEccReport(EnvList, ModifyDirList, EccDiffRange): + IsECCNeed = False + IsTrue = True + PkgList = GetPkgList(EnvList) + + for Line in ModifyDirList: + print('echo Run ECC tool for the commit in %s' % Line) + GlobalSymbol['GenerateEccReport'] = True + for Pkg in PkgList: + if Pkg in Line: + IsECCNeed = True + Ecc_cmd = ["ecc", "-c", "%WORKDIR%\BaseTools\Source\Python\Ecc\config.ini", + "-e", "%WORKDIR%\BaseTools\Source\Python\Ecc\exception.xml", + "-t", "%WORKDIR%\{}".format(Line), + "-r", "%WORKDIR%\Ecc.csv"] + _, _, result, return_code = ExecuteScript(Ecc_cmd, EnvList, shell=True) + if return_code != 0: + IsTrue = False + break + + if not IsTrue: + print('Fail to run ECC tool') + GlobalSymbol['SCRIPT_ERROR'] = True + EndDelFile(EnvList) + + if GlobalSymbol.get('GenerateEccReport'): + ParseEccReport(EnvList, EccDiffRange) + else: + print("Patch check tool or ECC tool don't detect error") + + if IsECCNeed: + revert_cmd = ["git", "checkout", "--", "%WORKDIR%\BaseTools\Source\Python\Ecc\exception.xml"] + _, _, result, return_code = ExecuteScript(revert_cmd, EnvList, shell=True) + else: + print("Doesn't need run ECC check") + return + +def ParseEccReport(EnvList, EccDiffRange): + WorkDir = EnvList['WORKDIR'] + EccLog = os.path.join(WorkDir, "Ecc.log") + EccsCsv = "Ecc.csv" + FileList = os.listdir(WorkDir) + rowLines = [] + if EccsCsv in FileList: + with open(EccsCsv) as Csv: + reader = csv.reader(Csv) + for row in reader: + for ModifyFile in EccDiffRange: + if ModifyFile in row[3]: + for i in EccDiffRange[ModifyFile]: + LineNo = int(row[4]) + if i[0] <= LineNo <= i[1]: + row[0] = '\nEFI coding style error' + row[1] = 'Error code: ' + row[1] + row[3] = 'File: ' + row[3] + row[4] = 'Line number: ' + row[4] + rowLine = '\n *'.join(row) + rowLines.append(rowLine) + break + break + if rowLines: + GlobalSymbol['ECC_PASS'] = False + + with open(EccLog, 'a') as EccFile: + AllLine = '\n'.join(rowLines) + AllLine = AllLine + '\n' + EccFile.writelines(AllLine) + +def RemoveFile(File): + if os.path.exists(File): + os.remove(File) + +def ExecuteScript(command, env_variables, collect_env=False, + enable_std_pipe=False, shell=True): + """launches a process that executes a script/shell command passed to it + + :param command: The command/script with its commandline + arguments to be executed + :type command: List:String + :param env_variables: Environment variables passed to the process + :type env_variables: String + :param collect_env: Enables the collection of evironment variables + when process execution is done + :type collect_env: Boolean + :param enable_std_pipe: Enables process out to be piped to + :type enable_std_pipe: String + :returns: a tuple of std_out, stderr , environment variables, + return code + :rtype: Tuple: (std_out, stderr , enVar, return_code) + """ + + env_marker = '-----env-----' + env = {} + kwarg = {"env": env_variables, + "universal_newlines": True, + "shell": shell, + "cwd": env_variables["WORKSPACE"]} + + if enable_std_pipe or collect_env: + kwarg["stdout"] = subprocess.PIPE + kwarg["stderr"] = subprocess.PIPE + + if collect_env: + # get the binary that prints environment variables based on os + if os.name == 'nt': + get_var_command = "set" + else: + get_var_command = "env" + # modify the command to print the environment variables + if isinstance(command, list): + command += ["&&", "echo", env_marker, "&&", + get_var_command, "&&", "echo", env_marker] + else: + command += " " + " ".join(["&&", "echo", env_marker, + "&&", get_var_command, + "&&", "echo", env_marker]) + + # execute the command + execute = subprocess.Popen(command, **kwarg) + std_out, stderr = execute.communicate() + code = execute.returncode + + # wait for process to be done + execute.wait() + + # if collect enviroment variables + if collect_env: + # get the new environment variables + std_out, env = GetEnvironmentVariables(std_out, env_marker) + return (std_out, stderr, env, code) + +def GetEnvironmentVariables(std_out_str, marker): + """Gets the environment variables from a process + + :param std_out_str: The std_out pipe + :type std_out_str: String + :param marker: A begining and end mark of environment + variables printed to std_out + :type marker: String + :returns: The environment variables read from the process' std_out pipe + :rtype: Tuple + """ + start_env_update = False + environment_vars = {} + out_put = "" + for line in std_out_str.split("\n"): + if start_env_update and len(line.split("=")) == 2: + key, value = line.split("=") + environment_vars[key] = value + else: + out_put += "\n" + line.replace(marker, "") + + if marker in line: + if start_env_update: + start_env_update = False + else: + start_env_update = True + return (out_put, environment_vars) + +def EndDelFile(EnvList): + WORKDIR = EnvList['WORKDIR'] + ModifyFileListLog = os.path.join(WORKDIR, 'PatchModifyFiles.log') + RemoveFile(ModifyFileListLog) + PatchLog = os.path.join(WORKDIR, 'PatchFile.log') + RemoveFile(PatchLog) + FileLog = os.path.join(WORKDIR, "File.log") + RemoveFile(FileLog) + if GlobalSymbol.get('GenerateEccReport'): + FileList = os.listdir(WORKDIR) + CsvList = [os.path.join(WORKDIR, File) for File in FileList if File.endswith('.csv')] + for Csv in CsvList: + RemoveFile(Csv) + if GlobalSymbol.get('SCRIPT_ERROR'): + print('ECC tool detect error') + exit(1) + +def EdksetupRebuild(EnvList, WORKDIR): + EnvList['WORKDIR'] = WORKDIR + EnvList['WORKSPACE'] = WORKDIR + edk2_setup_cmd = ["edksetup", "Rebuild"] + _, _, result, return_code = ExecuteScript(edk2_setup_cmd, EnvList, collect_env=True, shell=True) + if return_code == 0: + for Key in result: + EnvList[Key] = result[Key] + return True + return False + +def GetDiffrange(EnvList, commit): + WORKDIR = EnvList['WORKDIR'] + ModifyFileListLog = EnvList['ModifyFileListLog'] + RangeDirectory = {} + PatchLog = os.path.join(WORKDIR, 'PatchFile.log') + Format_Patch_cmd = ["git", "show", str(commit), "--unified=0", ">", PatchLog] + _, _, result, return_code = ExecuteScript(Format_Patch_cmd, EnvList, shell=True) + if return_code != 0: + print('Fail to run GIT') + GlobalSymbol['SCRIPT_ERROR'] = True + EndDelFile(EnvList) + with open(PatchLog, encoding='utf8') as PatchLogFile: + Filelines = PatchLogFile.readlines() + IsDelete = True + StartCheck = False + for line in Filelines: + ModifyFile = FindModifyFile.findall(line) + if ModifyFile and not StartCheck and os.path.isfile(ModifyFile[0]): + ModifyFileCommentDic = GetCommentRange(EnvList, ModifyFile[0]) + IsDelete = False + StartCheck = True + ModifyFilieDic = ModifyFile[0] + ModifyFilieDic = ModifyFilieDic.replace("/","\\") + RangeDirectory[ModifyFilieDic] = [] + elif line.startswith('--- '): + StartCheck = False + elif re.match(LineScopePattern, line, re.I) and not IsDelete and StartCheck: + startline = LineNumRange.search(line).group(1) + linerange = LineNumRange.search(line).group(2) + if not linerange: + linerange = '1' + RangeDirectory[ModifyFilieDic].append((int(startline), int(startline) + int(linerange) - 1)) + for i in ModifyFileCommentDic: + if i[0] <= int(startline) <= i[1]: + RangeDirectory[ModifyFilieDic].append(i) + return RangeDirectory + +def GetCommentRange(EnvList, ModifyFile): + WORKDIR = EnvList['WORKDIR'] + ModifyFilePath = os.path.join(WORKDIR, ModifyFile) + with open(ModifyFilePath) as f: + LineNo = 1 + CommentRange = [] + Start = False + for line in f: + if line.startswith('/**'): + startno = LineNo + Start = True + if line.startswith('**/') and Start: + endno = LineNo + Start = False + CommentRange.append((int(startno), int(endno))) + LineNo += 1 + + if CommentRange and CommentRange[0][0] == 1: + del CommentRange[0] + return CommentRange + +def GetModifyDir(EnvList, commit): + WORKDIR = EnvList['WORKDIR'] + ModifyDirList = [] + ModifyFileListLog = os.path.join(WORKDIR, 'PatchModifyFiles.log') + EnvList['ModifyFileListLog'] = ModifyFileListLog + Patch_Modify_cmd = ["git", "diff", "--name-status", str(commit), str(commit)+"~1", ">", ModifyFileListLog] + _, _, result, return_code = ExecuteScript(Patch_Modify_cmd, EnvList, shell=True) + if return_code != 0: + print('Fail to run GIT') + GlobalSymbol['SCRIPT_ERROR'] = True + EndDelFile(EnvList) + with open(ModifyFileListLog) as ModifyFile: + Filelines = ModifyFile.readlines() + for Line in Filelines: + FilePath = ReModifyFile.findall(Line) + if FilePath: + FileDir = os.path.dirname(FilePath[0]) + else: + continue + PkgList = GetPkgList(EnvList) + if FileDir in PkgList or not FileDir: + continue + else: + ModifyDirList.append('%s'%FileDir) + + ModifyDirList = list(set(ModifyDirList)) + return ModifyDirList + +def ApplyConfig(EnvList, ModifyDirList, EccDiffRange): + WORKDIR = EnvList['WORKDIR'] + ModifyPkgList = [] + for ModifyDir in ModifyDirList: + ModifyPkg = ModifyDir.split("/")[0] + ModifyPkgList.append(ModifyPkg) + ModifyPkgList = list(set(ModifyPkgList)) + for ModifyPkg in ModifyPkgList: + pkg_config_file = os.path.join(WORKDIR, ModifyPkg, ModifyPkg + ".ci.yaml") + if os.path.exists(pkg_config_file): + with open(pkg_config_file, 'r') as f: + pkg_config = yaml.safe_load(f) + if "EccCheck" in pkg_config: + EccConfig = pkg_config["EccCheck"] + # + # Add exceptions + # + ExceptionList = EccConfig["ExceptionList"] + ExceptionFile = os.path.join(WORKDIR, "BaseTools", "Source", "Python", "Ecc", "exception.xml") + if os.path.exists(ExceptionFile): + AppendException(ExceptionList, ExceptionFile) + # + # Exclude ignored files + # + IgnoreFilesList = EccConfig['IgnoreFiles'] + for ignore_file in IgnoreFilesList: + ignore_file = ignore_file.replace("/", "\\") + ignore_file = os.path.join(ModifyPkg, ignore_file) + if ignore_file in EccDiffRange: + del EccDiffRange[ignore_file] + return + +def CheckOneCommit(commit): + WORKDIR = os.getcwd() + EdksetupRebuild(EnvList, WORKDIR) + ModifyDirList = GetModifyDir(EnvList, commit) + EccDiffRange = GetDiffrange(EnvList, commit) + ApplyConfig(EnvList, ModifyDirList, EccDiffRange) + GenerateEccReport(EnvList, ModifyDirList, EccDiffRange) + EndDelFile(EnvList) + +def parse_options(): + parser = argparse.ArgumentParser(description=__copyright__) + parser.add_argument('commits', nargs='*', + help='[commit(s) ID | number of commits, like "-3" means check first 3 commits]') + args = parser.parse_args() + return args + +def process_one_arg(self, arg): + if len(arg) >= 2 and arg[0] == '-': + count = int(arg[1:]) + CheckOneArg(arg, self.count).ok + + +def read_commit_list_from_git(start_commit, count): + cmd = ['git', 'rev-list', '--abbrev-commit', '--no-walk' ] + if count is not None: + cmd.append('--max-count=' + str(count)) + cmd.append(start_commit) + p = subprocess.Popen(cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) + result = p.communicate() + out = result[0].decode('utf-8', 'ignore') if result[0] and result[0].find(b"fatal")!=0 else None + return out.split() if out else [] + +def ReleaseReport(EnvList): + WORKDIR = EnvList['WORKDIR'] + EccLog = os.path.join(WORKDIR, "ECC.log") + if GlobalSymbol['ECC_PASS']: + print('\n===================Ecc pass===================') + RemoveFile(EccLog) + return 0 + else: + print('\n===================Ecc error detected===================') + with open(EccLog) as output: + print(output.read()) + RemoveFile(EccLog) + return -1 + +def main(): + commits = parse_options().commits + + if len(commits) == 0: + commits = [ 'HEAD' ] + + if len(commits[0]) >= 2 and commits[0][0] == '-': + count = int(commits[0][1:]) + commits = read_commit_list_from_git('HEAD', count) + + """ + This 'if' block is only used for creating pull request. + """ + if ".." in commits[0]: + commits = read_commit_list_from_git(commits[0], None) + + GlobalSymbol['ECC_PASS'] = True + for commit in commits: + CheckOneCommit(commit) + + retval = ReleaseReport(EnvList) + return retval + +if __name__ == "__main__": + sys.exit(main()) \ No newline at end of file -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/5] pip-requirements.txt: Add Ecc required lib 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 2/5] BaseTools/Scripts: Add EccCheck.py Zhang, Shenglei @ 2020-06-03 8:48 ` Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 4/5] .azurepiplines: Add a pipline to check ECC issues for commits Zhang, Shenglei ` (3 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Zhang, Shenglei @ 2020-06-03 8:48 UTC (permalink / raw) To: devel; +Cc: Sean Brogan, Bret Barkelew, Michael D Kinney, Liming Gao antlr4-python3-runtime is a lib to support Ecc run with Py3.x. Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> --- pip-requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/pip-requirements.txt b/pip-requirements.txt index 574dac43b1a6..7e83ddd9b3c9 100644 --- a/pip-requirements.txt +++ b/pip-requirements.txt @@ -14,3 +14,4 @@ edk2-pytool-library==0.10.* edk2-pytool-extensions~=0.13.3 +antlr4-python3-runtime \ No newline at end of file -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/5] .azurepiplines: Add a pipline to check ECC issues for commits 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei ` (2 preceding siblings ...) 2020-06-03 8:48 ` [PATCH v2 3/5] pip-requirements.txt: Add Ecc required lib Zhang, Shenglei @ 2020-06-03 8:48 ` Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check Zhang, Shenglei ` (2 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Zhang, Shenglei @ 2020-06-03 8:48 UTC (permalink / raw) To: devel; +Cc: Sean Brogan, Bret Barkelew, Michael D Kinney, Liming Gao REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606 Add a pipeline to run the ECC checks on Azure Pipelines agents for edk2 open ci. Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> --- .azurepipelines/Windows-EccCheck.yml | 38 ++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .azurepipelines/Windows-EccCheck.yml diff --git a/.azurepipelines/Windows-EccCheck.yml b/.azurepipelines/Windows-EccCheck.yml new file mode 100644 index 000000000000..0d20551efb5f --- /dev/null +++ b/.azurepipelines/Windows-EccCheck.yml @@ -0,0 +1,38 @@ +## @file +# Azure Pipielines YML file that evalues the patch series in a PR using the +# python script BaseTools/Scripts/PatchCheck.py. +# +# NOTE: This example monitors pull requests against the edk2-ci branch. Most +# environments would replace 'edk2-ci' with 'master'. +# +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# https://github.com/tianocore +# +## + +trigger: none + +pr: +- master + +pool: + vmImage: 'windows-latest' + +steps: +- checkout: self + clean: true + +- task: UsePythonVersion@0 + inputs: + versionSpec: '3.8.x' + architecture: 'x64' + +- script: pip install -r pip-requirements.txt + displayName: 'Install/Upgrade pip modules' + +- script: | + git fetch origin $(System.PullRequest.TargetBranch):$(System.PullRequest.TargetBranch) + py -3 BaseTools/Scripts/EccCheck.py $(System.PullRequest.TargetBranch)..$(System.PullRequest.SourceCommitId) + displayName: 'Use EccCheck.py to verify patch series in pull request' -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei ` (3 preceding siblings ...) 2020-06-03 8:48 ` [PATCH v2 4/5] .azurepiplines: Add a pipline to check ECC issues for commits Zhang, Shenglei @ 2020-06-03 8:48 ` Zhang, Shenglei 2020-06-03 14:27 ` [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Laszlo Ersek 2020-06-09 13:10 ` Liming Gao 6 siblings, 0 replies; 12+ messages in thread From: Zhang, Shenglei @ 2020-06-03 8:48 UTC (permalink / raw) To: devel; +Cc: Jian J Wang, Hao A Wu This is a sample for users to add exceptions and ignored files for their own use. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> --- MdeModulePkg/MdeModulePkg.ci.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.ci.yaml b/MdeModulePkg/MdeModulePkg.ci.yaml index 1cfc1328390e..8ccf667d12f3 100644 --- a/MdeModulePkg/MdeModulePkg.ci.yaml +++ b/MdeModulePkg/MdeModulePkg.ci.yaml @@ -5,6 +5,14 @@ # SPDX-License-Identifier: BSD-2-Clause-Patent ## { + "EccCheck": { + "ExceptionList": [ + "8001", "aaabbb" + ], + "IgnoreFiles": [ + "Library/BaseBmpSupportLib/BmpSupportLib.c" + ] + }, ## options defined ci/Plugin/CompilerPlugin "CompilerPlugin": { "DscPath": "MdeModulePkg.dsc" -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei ` (4 preceding siblings ...) 2020-06-03 8:48 ` [PATCH v2 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check Zhang, Shenglei @ 2020-06-03 14:27 ` Laszlo Ersek 2020-06-04 5:38 ` Zhang, Shenglei 2020-06-09 13:10 ` Liming Gao 6 siblings, 1 reply; 12+ messages in thread From: Laszlo Ersek @ 2020-06-03 14:27 UTC (permalink / raw) To: devel, shenglei.zhang Cc: Bob Feng, Bret Barkelew, Michael D Kinney, Liming Gao, Sean Brogan On 06/03/20 10:48, Zhang, Shenglei wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606 > As planed we will enable Ecc check for edk2 on open ci. And they are > ready now, but these are V2 series. So I expect that contributors in > edk2 community can try using this script when reviewing. And I appreciate > receiving feedback and comments if someone find errors or false positive > issues. > > I created a pipline of EccCheck for my forked edk2. > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10&_a=summary > > The patch series are big, so the commits are also pushed into my forked tree. > https://github.com/shenglei10/edk2/commits/ECC > > Patches > 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not > a pipline or script issue. But it is listed here for people willing > to try this tool. > 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run > on azure servers for open ci, or locally. Its usage is like > PatchCheck.py. > 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local > use, we need to type command > "py -3 -m pip install antlr4-python3-runtime" first. > 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added > pipline. The azure uses this to create a pipline. > 5/5: We consider some cases that will report out Ecc issues but they won't > be fixed, like submodule and industry standard related things. So we > add two configuration fields "Exception" and "IgnoreFiles" for people > to use. The patch is a example and the contents in the fields will be > empty in final version. > > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Sean Brogan <sean.brogan@microsoft.com> > > v2: Update 2/5, fix the bug that the script can't hanlde multiple commits. Thanks for the ExceptionList / IgnoreFiles features; I think they are really important. I've run ECC in the past, and in some cases it is *way* too strict and opinionated, so I'm sure we'll end up "training" the ExceptionList entry for OvmfPkg. Can you please explain how (if?) ECC is restricted to new code added by a patch series? Patch#2 seems related, but I don't fully understand. It says, "It can only handle the issues, whose line number in CSV report accurately map with their code in source code files." Does that mean that CI performs a full ECC check, but filters out all warning / error messages that do not refer to code lines added in the patch series? Thanks, Laszlo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci 2020-06-03 14:27 ` [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Laszlo Ersek @ 2020-06-04 5:38 ` Zhang, Shenglei 2020-06-09 12:24 ` Laszlo Ersek 0 siblings, 1 reply; 12+ messages in thread From: Zhang, Shenglei @ 2020-06-04 5:38 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io Cc: Feng, Bob C, Bret Barkelew, Kinney, Michael D, Gao, Liming, Sean Brogan Hi Laszlo, > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Wednesday, June 3, 2020 10:27 PM > To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@intel.com> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Bret Barkelew > <Bret.Barkelew@microsoft.com>; Kinney, Michael D > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean > Brogan <sean.brogan@microsoft.com> > Subject: Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for > edk2 on open ci > > On 06/03/20 10:48, Zhang, Shenglei wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606 > > As planed we will enable Ecc check for edk2 on open ci. And they are > > ready now, but these are V2 series. So I expect that contributors in > > edk2 community can try using this script when reviewing. And I appreciate > > receiving feedback and comments if someone find errors or false positive > > issues. > > > > I created a pipline of EccCheck for my forked edk2. > > > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10 > &_a=summary > > > > The patch series are big, so the commits are also pushed into my forked > tree. > > https://github.com/shenglei10/edk2/commits/ECC > > > > Patches > > 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not > > a pipline or script issue. But it is listed here for people willing > > to try this tool. > > 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run > > on azure servers for open ci, or locally. Its usage is like > > PatchCheck.py. > > 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local > > use, we need to type command > > "py -3 -m pip install antlr4-python3-runtime" first. > > 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added > > pipline. The azure uses this to create a pipline. > > 5/5: We consider some cases that will report out Ecc issues but they won't > > be fixed, like submodule and industry standard related things. So we > > add two configuration fields "Exception" and "IgnoreFiles" for people > > to use. The patch is a example and the contents in the fields will be > > empty in final version. > > > > Cc: Bob Feng <bob.c.feng@intel.com> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Cc: Liming Gao <liming.gao@intel.com> > > Cc: Sean Brogan <sean.brogan@microsoft.com> > > > > v2: Update 2/5, fix the bug that the script can't hanlde multiple commits. > > Thanks for the ExceptionList / IgnoreFiles features; I think they are > really important. I've run ECC in the past, and in some cases it is > *way* too strict and opinionated, so I'm sure we'll end up "training" > the ExceptionList entry for OvmfPkg. > > Can you please explain how (if?) ECC is restricted to new code added by > a patch series? Patch#2 seems related, but I don't fully understand. It > says, > > "It can only handle the issues, whose line number in CSV report > accurately map with their code in source code files." > > Does that mean that CI performs a full ECC check, but filters out all > warning / error messages that do not refer to code lines added in the > patch series? > Yes, I think you get the point. Since there are plenty of Ecc issues existing in edk2, we need to filter out the issues that are not caused by the patch to be checked in. The actual implementation is to create a dictionary for modified files, in which the key word is the modified file and the content is the line number scope for added code. Then if the issues in CSV report can be mapped with the dictionary, they are marked as real issues. And the scanning scope is the folder that the change is located in. For example, Someone modifies "FmpDevicePkg\Library\FmpDependencyLib\FmpDependencyLib.c". Then the scan scope is "FmpDevicePkg\Library\FmpDependencyLib". It's not a full check for edk2, because it will cost much time. Thanks, Shenglei > Thanks, > Laszlo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci 2020-06-04 5:38 ` Zhang, Shenglei @ 2020-06-09 12:24 ` Laszlo Ersek 0 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2020-06-09 12:24 UTC (permalink / raw) To: Zhang, Shenglei, devel@edk2.groups.io Cc: Feng, Bob C, Bret Barkelew, Kinney, Michael D, Gao, Liming, Sean Brogan On 06/04/20 07:38, Zhang, Shenglei wrote: > Hi Laszlo, > >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Wednesday, June 3, 2020 10:27 PM >> To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@intel.com> >> Cc: Feng, Bob C <bob.c.feng@intel.com>; Bret Barkelew >> <Bret.Barkelew@microsoft.com>; Kinney, Michael D >> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean >> Brogan <sean.brogan@microsoft.com> >> Subject: Re: [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for >> edk2 on open ci >> >> On 06/03/20 10:48, Zhang, Shenglei wrote: >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606 >>> As planed we will enable Ecc check for edk2 on open ci. And they are >>> ready now, but these are V2 series. So I expect that contributors in >>> edk2 community can try using this script when reviewing. And I appreciate >>> receiving feedback and comments if someone find errors or false positive >>> issues. >>> >>> I created a pipline of EccCheck for my forked edk2. >>> >> https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10 >> &_a=summary >>> >>> The patch series are big, so the commits are also pushed into my forked >> tree. >>> https://github.com/shenglei10/edk2/commits/ECC >>> >>> Patches >>> 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not >>> a pipline or script issue. But it is listed here for people willing >>> to try this tool. >>> 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run >>> on azure servers for open ci, or locally. Its usage is like >>> PatchCheck.py. >>> 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local >>> use, we need to type command >>> "py -3 -m pip install antlr4-python3-runtime" first. >>> 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added >>> pipline. The azure uses this to create a pipline. >>> 5/5: We consider some cases that will report out Ecc issues but they won't >>> be fixed, like submodule and industry standard related things. So we >>> add two configuration fields "Exception" and "IgnoreFiles" for people >>> to use. The patch is a example and the contents in the fields will be >>> empty in final version. >>> >>> Cc: Bob Feng <bob.c.feng@intel.com> >>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> >>> Cc: Michael D Kinney <michael.d.kinney@intel.com> >>> Cc: Liming Gao <liming.gao@intel.com> >>> Cc: Sean Brogan <sean.brogan@microsoft.com> >>> >>> v2: Update 2/5, fix the bug that the script can't hanlde multiple commits. >> >> Thanks for the ExceptionList / IgnoreFiles features; I think they are >> really important. I've run ECC in the past, and in some cases it is >> *way* too strict and opinionated, so I'm sure we'll end up "training" >> the ExceptionList entry for OvmfPkg. >> >> Can you please explain how (if?) ECC is restricted to new code added by >> a patch series? Patch#2 seems related, but I don't fully understand. It >> says, >> >> "It can only handle the issues, whose line number in CSV report >> accurately map with their code in source code files." >> >> Does that mean that CI performs a full ECC check, but filters out all >> warning / error messages that do not refer to code lines added in the >> patch series? >> > > Yes, I think you get the point. > Since there are plenty of Ecc issues existing in edk2, we need to filter out the > issues that are not caused by the patch to be checked in. The actual implementation > is to create a dictionary for modified files, in which the key word is the modified file and > the content is the line number scope for added code. Then if the issues in CSV report can > be mapped with the dictionary, they are marked as real issues. > > And the scanning scope is the folder that the change is located in. For example, > Someone modifies "FmpDevicePkg\Library\FmpDependencyLib\FmpDependencyLib.c". > Then the scan scope is "FmpDevicePkg\Library\FmpDependencyLib". It's not a full check > for edk2, because it will cost much time. Thank you for the explanation! Laszlo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei ` (5 preceding siblings ...) 2020-06-03 14:27 ` [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Laszlo Ersek @ 2020-06-09 13:10 ` Liming Gao 6 siblings, 0 replies; 12+ messages in thread From: Liming Gao @ 2020-06-09 13:10 UTC (permalink / raw) To: Zhang, Shenglei, devel@edk2.groups.io Cc: Feng, Bob C, Bret Barkelew, Kinney, Michael D, Sean Brogan Shenglei: Seemly, ECC tool update is not related to this patch serial. Can you separate it? Besides, please update package yaml file with new ECC checker. Thanks Liming > -----Original Message----- > From: Zhang, Shenglei <shenglei.zhang@intel.com> > Sent: Wednesday, June 3, 2020 4:48 PM > To: devel@edk2.groups.io > Cc: Feng, Bob C <bob.c.feng@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Kinney, Michael D > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com> > Subject: [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2606 > As planed we will enable Ecc check for edk2 on open ci. And they are > ready now, but these are V2 series. So I expect that contributors in > edk2 community can try using this script when reviewing. And I appreciate > receiving feedback and comments if someone find errors or false positive > issues. > > I created a pipline of EccCheck for my forked edk2. > https://dev.azure.com/shengleizhang/shengleizhang/_build?definitionId=10&_a=summary > > The patch series are big, so the commits are also pushed into my forked tree. > https://github.com/shenglei10/edk2/commits/ECC > > Patches > 1/5: This is a patch to enable python 3.8 for Ecc. It is a tool issues not > a pipline or script issue. But it is listed here for people willing > to try this tool. > 2/5: EccCheck.py is a tool to report Ecc issues for commits. It can be run > on azure servers for open ci, or locally. Its usage is like > PatchCheck.py. > 3/5: It's a lib necessary for py3 to run Ecc on azure servers. For local > use, we need to type command > "py -3 -m pip install antlr4-python3-runtime" first. > 4/5: Windows-EccCheck.yml is a yaml file to configure the newly added > pipline. The azure uses this to create a pipline. > 5/5: We consider some cases that will report out Ecc issues but they won't > be fixed, like submodule and industry standard related things. So we > add two configuration fields "Exception" and "IgnoreFiles" for people > to use. The patch is a example and the contents in the fields will be > empty in final version. > > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Sean Brogan <sean.brogan@microsoft.com> > > v2: Update 2/5, fix the bug that the script can't hanlde multiple commits. > > Fan, Zhiju (1): > BaseTools:ECC needs to update the contents of CParser4 > > Shenglei Zhang (4): > BaseTools/Scripts: Add EccCheck.py > pip-requirements.txt: Add Ecc required lib > .azurepiplines: Add a pipline to check ECC issues for commits > MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check > > .azurepipelines/Windows-EccCheck.yml | 38 ++ > BaseTools/Scripts/EccCheck.py | 433 ++++++++++++++++++ > .../Source/Python/Ecc/CParser4/CLexer.py | 6 +- > .../Source/Python/Ecc/CParser4/CListener.py | 4 +- > .../Source/Python/Ecc/CParser4/CParser.py | 38 +- > MdeModulePkg/MdeModulePkg.ci.yaml | 8 + > pip-requirements.txt | 1 + > 7 files changed, 505 insertions(+), 23 deletions(-) > create mode 100644 .azurepipelines/Windows-EccCheck.yml > create mode 100644 BaseTools/Scripts/EccCheck.py > > -- > 2.18.0.windows.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-06-09 13:10 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-06-03 8:48 [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei 2020-06-05 4:38 ` [edk2-devel] " Yuwei Chen 2020-06-05 5:11 ` Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 2/5] BaseTools/Scripts: Add EccCheck.py Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 3/5] pip-requirements.txt: Add Ecc required lib Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 4/5] .azurepiplines: Add a pipline to check ECC issues for commits Zhang, Shenglei 2020-06-03 8:48 ` [PATCH v2 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check Zhang, Shenglei 2020-06-03 14:27 ` [edk2-devel] [PATCH v2 0/5] Add a pipline to check Ecc issues for edk2 on open ci Laszlo Ersek 2020-06-04 5:38 ` Zhang, Shenglei 2020-06-09 12:24 ` Laszlo Ersek 2020-06-09 13:10 ` Liming Gao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox