public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/5] Add a pipline to check Ecc issues for edk2 on open ci
@ 2020-05-29  8:54 Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2020-05-29  8:54 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 V1 series. So I expect that contributors in
edk2 community can try using this script when reviewing. And I hope
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

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.

Note: For now this pipline can't report out Ecc issues like Tab character
      and trailing white spaceLine. And above issues are listed in CSV file
	  after Ecc scanning traget files. But PatchCheck.py can report out them.
	  I wonder whether we need to enable the above mentioned check
	  points in this pipline, becaue either pipline fails, the commit will not
	  be checked in.

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>
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                 | 426 ++++++++++++++++++
 .../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, 498 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] 6+ messages in thread

* [PATCH 1/5] BaseTools:ECC needs to update the contents of CParser4
  2020-05-29  8:54 [PATCH 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei
@ 2020-05-29  8:55 ` Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 2/5] BaseTools/Scripts: Add EccCheck.py Zhang, Shenglei
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2020-05-29  8:55 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] 6+ messages in thread

* [PATCH 2/5] BaseTools/Scripts: Add EccCheck.py
  2020-05-29  8:54 [PATCH 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei
@ 2020-05-29  8:55 ` Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 3/5] pip-requirements.txt: Add Ecc required lib Zhang, Shenglei
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2020-05-29  8:55 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>
---
 BaseTools/Scripts/EccCheck.py | 426 ++++++++++++++++++++++++++++++++++
 1 file changed, 426 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..479d2af8eac2
--- /dev/null
+++ b/BaseTools/Scripts/EccCheck.py
@@ -0,0 +1,426 @@
+## @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' ]
+    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)
+
+    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] 6+ messages in thread

* [PATCH 3/5] pip-requirements.txt: Add Ecc required lib
  2020-05-29  8:54 [PATCH 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 2/5] BaseTools/Scripts: Add EccCheck.py Zhang, Shenglei
@ 2020-05-29  8:55 ` Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 4/5] .azurepiplines: Add a pipline to check ECC issues for commits Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check Zhang, Shenglei
  4 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2020-05-29  8:55 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] 6+ messages in thread

* [PATCH 4/5] .azurepiplines: Add a pipline to check ECC issues for commits
  2020-05-29  8:54 [PATCH 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei
                   ` (2 preceding siblings ...)
  2020-05-29  8:55 ` [PATCH 3/5] pip-requirements.txt: Add Ecc required lib Zhang, Shenglei
@ 2020-05-29  8:55 ` Zhang, Shenglei
  2020-05-29  8:55 ` [PATCH 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check Zhang, Shenglei
  4 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2020-05-29  8:55 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..3b9126649cb0
--- /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] 6+ messages in thread

* [PATCH 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check
  2020-05-29  8:54 [PATCH 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei
                   ` (3 preceding siblings ...)
  2020-05-29  8:55 ` [PATCH 4/5] .azurepiplines: Add a pipline to check ECC issues for commits Zhang, Shenglei
@ 2020-05-29  8:55 ` Zhang, Shenglei
  4 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2020-05-29  8:55 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] 6+ messages in thread

end of thread, other threads:[~2020-05-29  8:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-29  8:54 [PATCH 0/5] Add a pipline to check Ecc issues for edk2 on open ci Zhang, Shenglei
2020-05-29  8:55 ` [PATCH 1/5] BaseTools:ECC needs to update the contents of CParser4 Zhang, Shenglei
2020-05-29  8:55 ` [PATCH 2/5] BaseTools/Scripts: Add EccCheck.py Zhang, Shenglei
2020-05-29  8:55 ` [PATCH 3/5] pip-requirements.txt: Add Ecc required lib Zhang, Shenglei
2020-05-29  8:55 ` [PATCH 4/5] .azurepiplines: Add a pipline to check ECC issues for commits Zhang, Shenglei
2020-05-29  8:55 ` [PATCH 5/5] MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for Ecc check Zhang, Shenglei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox