public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
@ 2019-06-17 10:54 Fan, ZhijuX
  2019-06-17 17:18 ` Michael D Kinney
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Fan, ZhijuX @ 2019-06-17 10:54 UTC (permalink / raw)
  To: devel@edk2.groups.io
  Cc: Gao, Liming, Feng, Bob C, Zhang, Shenglei, Kinney, Michael D

[-- Attachment #1: Type: text/plain, Size: 8687 bytes --]

GenBiosId is a tool to generate the BIOS ID binary file which uses
the data from the configuration file.
https://bugzilla.tianocore.org/show_bug.cgi?id=1846

v2:v1 is a tool of C type and v2 is python type.

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/BinWrappers/PosixLike/GenBiosId       |  14 +++
 BaseTools/BinWrappers/WindowsLike/GenBiosId.bat |   3 +
 BaseTools/Source/Python/GenBiosId/BiosId.env    |  27 ++++++
 BaseTools/Source/Python/GenBiosId/GenBiosId.py  | 118 ++++++++++++++++++++++++
 4 files changed, 162 insertions(+)
 create mode 100644 BaseTools/BinWrappers/PosixLike/GenBiosId
 create mode 100644 BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
 create mode 100644 BaseTools/Source/Python/GenBiosId/BiosId.env
 create mode 100644 BaseTools/Source/Python/GenBiosId/GenBiosId.py

diff --git a/BaseTools/BinWrappers/PosixLike/GenBiosId b/BaseTools/BinWrappers/PosixLike/GenBiosId
new file mode 100644
index 0000000000..1dd28e9662
--- /dev/null
+++ b/BaseTools/BinWrappers/PosixLike/GenBiosId
@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+#python `dirname $0`/RunToolFromSource.py `basename $0` $*
+
+# If a ${PYTHON_COMMAND} command is available, use it in preference to python
+if command -v ${PYTHON_COMMAND} >/dev/null 2>&1; then
+    python_exe=${PYTHON_COMMAND}
+fi
+
+full_cmd=${BASH_SOURCE:-$0} # see http://mywiki.wooledge.org/BashFAQ/028 for a discussion of why $0 is not a good choice here
+dir=$(dirname "$full_cmd")
+exe=$(basename "$full_cmd")
+
+export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
+exec "${python_exe:-python}" "$dir/../../Source/Python/$exe/$exe.py" "$@"
diff --git a/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
new file mode 100644
index 0000000000..e1f61382c8
--- /dev/null
+++ b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
@@ -0,0 +1,3 @@
+@setlocal
+@set ToolName=%~n0%
+@%PYTHON_COMMAND% %BASE_TOOLS_PATH%\Source\Python\%ToolName%\%ToolName%.py %*
diff --git a/BaseTools/Source/Python/GenBiosId/BiosId.env b/BaseTools/Source/Python/GenBiosId/BiosId.env
new file mode 100644
index 0000000000..e1e913da76
--- /dev/null
+++ b/BaseTools/Source/Python/GenBiosId/BiosId.env
@@ -0,0 +1,27 @@
+## @file
+#  This file is used to define the BIOS ID parameters of the build.
+#  This file is processed by GenBiosId.
+#  Here, it is just a template and can be customized by user.
+#
+#  BIOS ID string format:
+#    $(BOARD_ID)$(BOARD_REV).$(BOARD_EXT).$(VERSION_MAJOR).$(BUILD_TYPE)$(VERSION_MINOR).YYMMDDHHMM
+#  All fields must have a fixed length. YYMMDDHHMM is UTC time.
+#    Example: "EMLATOR1.000.0001.D01.1906141517"
+#
+#  If DATE is specified for YYMMDD and TIME is specified for HHMM like below,
+#  GenBiosId will use the value of DATE and TIME to fill YYMMDDHHMM,
+#  otherwise GenBiosId will fill YYMMDDHHMM with current UTC time of the build machine.
+#    DATE          = 190614
+#    TIME          = 1517
+#
+#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+[config]
+BOARD_ID      = EMLATOR
+BOARD_REV     = 1
+BOARD_EXT     = 000
+BUILD_TYPE    = D
+VERSION_MAJOR = 0001
+VERSION_MINOR = 01
diff --git a/BaseTools/Source/Python/GenBiosId/GenBiosId.py b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
new file mode 100644
index 0000000000..8259b17afd
--- /dev/null
+++ b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
@@ -0,0 +1,118 @@
+## @file
+# Trim files preprocessed by compiler
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+##
+# Import Modules
+#
+import Common.LongFilePathOs as os
+import sys
+import struct
+import time
+import datetime
+import argparse
+try:
+    from configparser import ConfigParser
+except:
+    from ConfigParser import ConfigParser
+from Common.BuildToolError import *
+from Common.Misc import *
+from Common.DataType import *
+from Common.BuildVersion import gBUILD_VERSION
+import Common.EdkLogger as EdkLogger
+from Common.LongFilePathSupport import OpenLongFilePath as open
+
+_BIOS_Signature = "$IBIOSI$"
+_SectionKeyName = '__name__'
+_SectionName = 'config'
+
+__prog__ = 'GenBiosld'
+__description__ = 'Trim files preprocessed by compiler'
+__copyright__ = 'Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> '
+__version__ = '%s Version %s' % (__prog__, '0.1 ')
+
+_ConfigItem = {
+    "BOARD_ID"   : {'Value' : '', 'Length' : 7},
+    "BOARD_REV"  : {'Value' : '', 'Length' : 1},
+    "BOARD_EXT"  : { 'Value' : '', 'Length' : 3},
+    "BUILD_TYPE" : {'Value' : '', 'Length' :1},
+    "VERSION_MAJOR" : {'Value' : '0000', 'Length' : 4},
+    "VERSION_MINOR" : {'Value' : '00', 'Length' : 2},
+
+}
+
+
+_Usage = "Usage: GenBiosId -i Configfile -o OutputFile [-ob OutputBatchFile]"
+_ConfigSectionNotDefine = "Not support the config file format, need config section"
+_ConfigLenInvalid = "Config item %s length is invalid"
+_ConfigItemInvalid = "Item %s is invalid"
+
+def Main():
+    parser = argparse.ArgumentParser(prog=__prog__,
+                                     description=__description__ + __copyright__ + _Usage,
+                                     conflict_handler='resolve')
+    parser.add_argument('-v', '--version', action='version', version=__version__,
+                        help="show program's version number and exit")
+    parser.add_argument('-i', '--int', metavar='FILENAME', dest='InputFile', help="Input Config file")
+    parser.add_argument('-o', '--out', metavar='FILENAME', dest='OutputFile', help="Output file")
+    parser.add_argument('-ob', '--batch', metavar='FILENAME', dest='OutputBatchFile', help="OutputBatch file")
+    Options = parser.parse_args()
+    try:
+        EdkLogger.Initialize()
+        if len(sys.argv) !=5 and not (len(sys.argv) == 7 and Options.OutputBatchFile):
+            EdkLogger.error("GenBiosId", OPTION_MISSING, ExtraData=_Usage)
+        elif not Options.InputFile or not Options.OutputFile:
+            EdkLogger.error("GenBiosId", OPTION_MISSING, ExtraData=_Usage)
+    except FatalError as X:
+        return 1
+    InputFile = Options.InputFile
+    OutputFile = Options.OutputFile
+    OutputBatchFile = Options.OutputBatchFile
+    if not os.path.exists(InputFile):
+        EdkLogger.error("GenBiosId", FILE_NOT_FOUND, ExtraData="Input file not found")
+    cf = ConfigParser()
+    cf.optionxform = str
+    cf.read(InputFile)
+    if _SectionName not in cf._sections:
+        EdkLogger.error("GenBiosId", FORMAT_NOT_SUPPORTED, ExtraData=_ConfigSectionNotDefine)
+    for Item in cf._sections[_SectionName]:
+        if Item == _SectionKeyName:
+            continue
+        if Item not in _ConfigItem:
+            EdkLogger.error("GenBiosId", FORMAT_INVALID, ExtraData=_ConfigItemInvalid % Item)
+        _ConfigItem[Item]['Value'] = cf._sections[_SectionName][Item]
+        if len(_ConfigItem[Item]['Value']) != _ConfigItem[Item]['Length']:
+            EdkLogger.error("GenBiosId", FORMAT_INVALID, ExtraData=_ConfigLenInvalid % Item)
+    for Item in _ConfigItem:
+        if not _ConfigItem[Item]['Value']:
+            EdkLogger.error("GenBiosId", FORMAT_UNKNOWN_ERROR, ExtraData="Item %s is missing" % Item)
+    utcnow = datetime.datetime.utcnow()
+    TimeStamp = time.strftime("%y%m%d%H%M", utcnow.timetuple())
+
+    Id_Str = _ConfigItem['BOARD_ID']['Value'] + _ConfigItem['BOARD_REV']['Value'] + '.' + _ConfigItem['BOARD_EXT']['Value'] + '.' + _ConfigItem['VERSION_MAJOR']['Value'] + \
+             '.' + _ConfigItem["BUILD_TYPE"]['Value'] + _ConfigItem['VERSION_MINOR']['Value'] + '.' + TimeStamp
+    with open(OutputFile, 'wb') as FdOut:
+        for i in _BIOS_Signature:
+            FdOut.write(struct.pack('B', ord(i)))
+
+        for i in Id_Str:
+            FdOut.write(struct.pack('H', ord(i)))
+
+        FdOut.write(struct.pack('H', 0x00))
+    if OutputBatchFile:
+        with open(OutputBatchFile, 'w') as FdOut:
+            if sys.platform.startswith('win'):
+                Id_Str = 'SET BIOS_ID=' + Id_Str
+            else:
+                Id_Str = 'export BIOS_ID=' + Id_Str
+            FdOut.write(Id_Str)
+    return 0
+
+if __name__ == '__main__':
+    r = Main()
+    ## 0-127 is a safe return range, and 1 is a standard default error
+    if r < 0 or r > 127: r = 1
+    sys.exit(r)
-- 
2.14.1.windows.1


[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 6825 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
  2019-06-17 10:54 [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId Fan, ZhijuX
@ 2019-06-17 17:18 ` Michael D Kinney
  2019-06-18 13:14   ` Liming Gao
       [not found]   ` <15A94D43BBFC7A00.29221@groups.io>
  2019-06-17 19:05 ` rebecca
  2019-06-17 19:11 ` rebecca
  2 siblings, 2 replies; 8+ messages in thread
From: Michael D Kinney @ 2019-06-17 17:18 UTC (permalink / raw)
  To: Fan, ZhijuX, devel@edk2.groups.io, Kinney, Michael D
  Cc: Gao, Liming, Feng, Bob C, Zhang, Shenglei

We would like to move to Python based tools for 
everything with no requirements to use batch files
or shell scripts.

Do we really need the -ob --batch flag?  I looks 
like the feature it provides is a text string of the
BIOSID.  Can't we send that to stdout or just put 
that text string in an output file without using
any batch file or shell script specific syntax?

Thanks,

Mike



> -----Original Message-----
> From: Fan, ZhijuX
> Sent: Monday, June 17, 2019 3:54 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>; Zhang, Shenglei
> <shenglei.zhang@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [PATCH V2] BaseTools/GenBiosId: Add a new tool
> GenBiosId
> 
> GenBiosId is a tool to generate the BIOS ID binary file
> which uses the data from the configuration file.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1846
> 
> v2:v1 is a tool of C type and v2 is python type.
> 
> 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/BinWrappers/PosixLike/GenBiosId       |  14
> +++
>  BaseTools/BinWrappers/WindowsLike/GenBiosId.bat |   3 +
>  BaseTools/Source/Python/GenBiosId/BiosId.env    |  27
> ++++++
>  BaseTools/Source/Python/GenBiosId/GenBiosId.py  | 118
> ++++++++++++++++++++++++
>  4 files changed, 162 insertions(+)
>  create mode 100644
> BaseTools/BinWrappers/PosixLike/GenBiosId
>  create mode 100644
> BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
>  create mode 100644
> BaseTools/Source/Python/GenBiosId/BiosId.env
>  create mode 100644
> BaseTools/Source/Python/GenBiosId/GenBiosId.py
> 
> diff --git a/BaseTools/BinWrappers/PosixLike/GenBiosId
> b/BaseTools/BinWrappers/PosixLike/GenBiosId
> new file mode 100644
> index 0000000000..1dd28e9662
> --- /dev/null
> +++ b/BaseTools/BinWrappers/PosixLike/GenBiosId
> @@ -0,0 +1,14 @@
> +#!/usr/bin/env bash
> +#python `dirname $0`/RunToolFromSource.py `basename $0`
> $*
> +
> +# If a ${PYTHON_COMMAND} command is available, use it in
> preference to
> +python if command -v ${PYTHON_COMMAND} >/dev/null 2>&1;
> then
> +    python_exe=${PYTHON_COMMAND}
> +fi
> +
> +full_cmd=${BASH_SOURCE:-$0} # see
> +http://mywiki.wooledge.org/BashFAQ/028 for a discussion
> of why $0 is
> +not a good choice here dir=$(dirname "$full_cmd")
> exe=$(basename
> +"$full_cmd")
> +
> +export
> PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTH
> ONPATH"}"
> +exec "${python_exe:-python}"
> "$dir/../../Source/Python/$exe/$exe.py" "$@"
> diff --git
> a/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> new file mode 100644
> index 0000000000..e1f61382c8
> --- /dev/null
> +++ b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> @@ -0,0 +1,3 @@
> +@setlocal
> +@set ToolName=%~n0%
> +@%PYTHON_COMMAND%
> +%BASE_TOOLS_PATH%\Source\Python\%ToolName%\%ToolName%.py
> %*
> diff --git a/BaseTools/Source/Python/GenBiosId/BiosId.env
> b/BaseTools/Source/Python/GenBiosId/BiosId.env
> new file mode 100644
> index 0000000000..e1e913da76
> --- /dev/null
> +++ b/BaseTools/Source/Python/GenBiosId/BiosId.env
> @@ -0,0 +1,27 @@
> +## @file
> +#  This file is used to define the BIOS ID parameters of
> the build.
> +#  This file is processed by GenBiosId.
> +#  Here, it is just a template and can be customized by
> user.
> +#
> +#  BIOS ID string format:
> +#
> $(BOARD_ID)$(BOARD_REV).$(BOARD_EXT).$(VERSION_MAJOR).$(B
> UILD_TYPE)$(VERSION_MINOR).YYMMDDHHMM
> +#  All fields must have a fixed length. YYMMDDHHMM is
> UTC time.
> +#    Example: "EMLATOR1.000.0001.D01.1906141517"
> +#
> +#  If DATE is specified for YYMMDD and TIME is specified
> for HHMM like
> +below, #  GenBiosId will use the value of DATE and TIME
> to fill
> +YYMMDDHHMM, #  otherwise GenBiosId will fill YYMMDDHHMM
> with current UTC time of the build machine.
> +#    DATE          = 190614
> +#    TIME          = 1517
> +#
> +#  Copyright (c) 2019, Intel Corporation. All rights
> reserved.<BR> #
> +SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> [config]
> +BOARD_ID      = EMLATOR
> +BOARD_REV     = 1
> +BOARD_EXT     = 000
> +BUILD_TYPE    = D
> +VERSION_MAJOR = 0001
> +VERSION_MINOR = 01
> diff --git
> a/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> new file mode 100644
> index 0000000000..8259b17afd
> --- /dev/null
> +++ b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> @@ -0,0 +1,118 @@
> +## @file
> +# Trim files preprocessed by compiler
> +#
> +# Copyright (c) 2019, Intel Corporation. All rights
> reserved.<BR> #
> +SPDX-License-Identifier: BSD-2-Clause-Patent #
> +
> +##
> +# Import Modules
> +#
> +import Common.LongFilePathOs as os
> +import sys
> +import struct
> +import time
> +import datetime
> +import argparse
> +try:
> +    from configparser import ConfigParser
> +except:
> +    from ConfigParser import ConfigParser from
> Common.BuildToolError
> +import * from Common.Misc import * from Common.DataType
> import * from
> +Common.BuildVersion import gBUILD_VERSION import
> Common.EdkLogger as
> +EdkLogger from Common.LongFilePathSupport import
> OpenLongFilePath as
> +open
> +
> +_BIOS_Signature = "$IBIOSI$"
> +_SectionKeyName = '__name__'
> +_SectionName = 'config'
> +
> +__prog__ = 'GenBiosld'
> +__description__ = 'Trim files preprocessed by compiler'
> +__copyright__ = 'Copyright (c) 2019, Intel Corporation.
> All rights reserved.<BR> '
> +__version__ = '%s Version %s' % (__prog__, '0.1 ')
> +
> +_ConfigItem = {
> +    "BOARD_ID"   : {'Value' : '', 'Length' : 7},
> +    "BOARD_REV"  : {'Value' : '', 'Length' : 1},
> +    "BOARD_EXT"  : { 'Value' : '', 'Length' : 3},
> +    "BUILD_TYPE" : {'Value' : '', 'Length' :1},
> +    "VERSION_MAJOR" : {'Value' : '0000', 'Length' : 4},
> +    "VERSION_MINOR" : {'Value' : '00', 'Length' : 2},
> +
> +}
> +
> +
> +_Usage = "Usage: GenBiosId -i Configfile -o OutputFile
> [-ob OutputBatchFile]"
> +_ConfigSectionNotDefine = "Not support the config file
> format, need config section"
> +_ConfigLenInvalid = "Config item %s length is invalid"
> +_ConfigItemInvalid = "Item %s is invalid"
> +
> +def Main():
> +    parser = argparse.ArgumentParser(prog=__prog__,
> +
> description=__description__ + __copyright__ + _Usage,
> +
> conflict_handler='resolve')
> +    parser.add_argument('-v', '--version',
> action='version', version=__version__,
> +                        help="show program's version
> number and exit")
> +    parser.add_argument('-i', '--int',
> metavar='FILENAME', dest='InputFile', help="Input Config
> file")
> +    parser.add_argument('-o', '--out',
> metavar='FILENAME', dest='OutputFile', help="Output
> file")
> +    parser.add_argument('-ob', '--batch',
> metavar='FILENAME', dest='OutputBatchFile',
> help="OutputBatch file")
> +    Options = parser.parse_args()
> +    try:
> +        EdkLogger.Initialize()
> +        if len(sys.argv) !=5 and not (len(sys.argv) == 7
> and Options.OutputBatchFile):
> +            EdkLogger.error("GenBiosId", OPTION_MISSING,
> ExtraData=_Usage)
> +        elif not Options.InputFile or not
> Options.OutputFile:
> +            EdkLogger.error("GenBiosId", OPTION_MISSING,
> ExtraData=_Usage)
> +    except FatalError as X:
> +        return 1
> +    InputFile = Options.InputFile
> +    OutputFile = Options.OutputFile
> +    OutputBatchFile = Options.OutputBatchFile
> +    if not os.path.exists(InputFile):
> +        EdkLogger.error("GenBiosId", FILE_NOT_FOUND,
> ExtraData="Input file not found")
> +    cf = ConfigParser()
> +    cf.optionxform = str
> +    cf.read(InputFile)
> +    if _SectionName not in cf._sections:
> +        EdkLogger.error("GenBiosId",
> FORMAT_NOT_SUPPORTED, ExtraData=_ConfigSectionNotDefine)
> +    for Item in cf._sections[_SectionName]:
> +        if Item == _SectionKeyName:
> +            continue
> +        if Item not in _ConfigItem:
> +            EdkLogger.error("GenBiosId", FORMAT_INVALID,
> ExtraData=_ConfigItemInvalid % Item)
> +        _ConfigItem[Item]['Value'] =
> cf._sections[_SectionName][Item]
> +        if len(_ConfigItem[Item]['Value']) !=
> _ConfigItem[Item]['Length']:
> +            EdkLogger.error("GenBiosId", FORMAT_INVALID,
> ExtraData=_ConfigLenInvalid % Item)
> +    for Item in _ConfigItem:
> +        if not _ConfigItem[Item]['Value']:
> +            EdkLogger.error("GenBiosId",
> FORMAT_UNKNOWN_ERROR, ExtraData="Item %s is missing" %
> Item)
> +    utcnow = datetime.datetime.utcnow()
> +    TimeStamp = time.strftime("%y%m%d%H%M",
> utcnow.timetuple())
> +
> +    Id_Str = _ConfigItem['BOARD_ID']['Value'] +
> _ConfigItem['BOARD_REV']['Value'] + '.' +
> _ConfigItem['BOARD_EXT']['Value'] + '.' +
> _ConfigItem['VERSION_MAJOR']['Value'] + \
> +             '.' + _ConfigItem["BUILD_TYPE"]['Value'] +
> _ConfigItem['VERSION_MINOR']['Value'] + '.' + TimeStamp
> +    with open(OutputFile, 'wb') as FdOut:
> +        for i in _BIOS_Signature:
> +            FdOut.write(struct.pack('B', ord(i)))
> +
> +        for i in Id_Str:
> +            FdOut.write(struct.pack('H', ord(i)))
> +
> +        FdOut.write(struct.pack('H', 0x00))
> +    if OutputBatchFile:
> +        with open(OutputBatchFile, 'w') as FdOut:
> +            if sys.platform.startswith('win'):
> +                Id_Str = 'SET BIOS_ID=' + Id_Str
> +            else:
> +                Id_Str = 'export BIOS_ID=' + Id_Str
> +            FdOut.write(Id_Str)
> +    return 0
> +
> +if __name__ == '__main__':
> +    r = Main()
> +    ## 0-127 is a safe return range, and 1 is a standard
> default error
> +    if r < 0 or r > 127: r = 1
> +    sys.exit(r)
> --
> 2.14.1.windows.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
  2019-06-17 10:54 [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId Fan, ZhijuX
  2019-06-17 17:18 ` Michael D Kinney
@ 2019-06-17 19:05 ` rebecca
  2019-06-18 13:26   ` Liming Gao
  2019-06-17 19:11 ` rebecca
  2 siblings, 1 reply; 8+ messages in thread
From: rebecca @ 2019-06-17 19:05 UTC (permalink / raw)
  To: devel, zhijux.fan
  Cc: Gao, Liming, Feng, Bob C, Zhang, Shenglei, Kinney, Michael D

On 2019-06-17 04:54, Fan, ZhijuX wrote:
> GenBiosId is a tool to generate the BIOS ID binary file which uses
> the data from the configuration file.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1846


pylama (https://github.com/klen/pylama) reports the following issues:


bcran@photon:~/workspace/edk2/BaseTools/Source/Python/GenBiosId % pylama .
GenBiosId.py:1:1: E266 too many leading '#' for block comment [pycodestyle]
GenBiosId.py:19:1: E722 do not use bare 'except' [pycodestyle]
GenBiosId.py:21:1: W0401 'from Common.BuildToolError import *' used;
unable to detect undefined names [pyflakes]
GenBiosId.py:22:1: W0401 'from Common.Misc import *' used; unable to
detect undefined names [pyflakes]
GenBiosId.py:23:1: W0401 'from Common.DataType import *' used; unable to
detect undefined names [pyflakes]
GenBiosId.py:24:1: W0611 'Common.BuildVersion.gBUILD_VERSION' imported
but unused [pyflakes]
GenBiosId.py:34:80: E501 line too long (82 > 79 characters) [pycodestyle]
GenBiosId.py:38:17: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:38:28: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:38:43: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:39:17: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:39:28: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:39:43: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:40:17: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:40:21: E201 whitespace after '{' [pycodestyle]
GenBiosId.py:40:29: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:40:44: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:41:17: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:41:28: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:41:43: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:41:44: E231 missing whitespace after ':' [pycodestyle]
GenBiosId.py:42:20: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:42:31: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:42:50: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:43:20: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:43:31: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:43:48: E203 whitespace before ':' [pycodestyle]
GenBiosId.py:49:80: E501 line too long (83 > 79 characters) [pycodestyle]
GenBiosId.py:53:1: E302 expected 2 blank lines, found 1 [pycodestyle]
GenBiosId.py:53:1: C901 'Main' is too complex (17) [mccabe]
GenBiosId.py:55:80: E501 line too long (90 > 79 characters) [pycodestyle]
GenBiosId.py:57:80: E501 line too long (81 > 79 characters) [pycodestyle]
GenBiosId.py:59:80: E501 line too long (102 > 79 characters) [pycodestyle]
GenBiosId.py:60:80: E501 line too long (97 > 79 characters) [pycodestyle]
GenBiosId.py:61:80: E501 line too long (110 > 79 characters) [pycodestyle]
GenBiosId.py:65:80: E501 line too long (86 > 79 characters) [pycodestyle]
GenBiosId.py:65:28: E225 missing whitespace around operator [pycodestyle]
GenBiosId.py:66:1: W0401 'OPTION_MISSING may be undefined, or defined
from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
[pyflakes]
GenBiosId.py:68:1: W0401 'OPTION_MISSING may be undefined, or defined
from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
[pyflakes]
GenBiosId.py:69:1: W0401 'FatalError may be undefined, or defined from
star imports: Common.BuildToolError, Common.DataType, Common.Misc'
[pyflakes]
GenBiosId.py:69:1: W0612 local variable 'X' is assigned to but never
used [pyflakes]
GenBiosId.py:75:80: E501 line too long (86 > 79 characters) [pycodestyle]
GenBiosId.py:75:1: W0401 'FILE_NOT_FOUND may be undefined, or defined
from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
[pyflakes]
GenBiosId.py:80:80: E501 line too long (93 > 79 characters) [pycodestyle]
GenBiosId.py:80:1: W0401 'FORMAT_NOT_SUPPORTED may be undefined, or
defined from star imports: Common.BuildToolError, Common.DataType,
Common.Misc' [pyflakes]
GenBiosId.py:85:80: E501 line too long (93 > 79 characters) [pycodestyle]
GenBiosId.py:85:1: W0401 'FORMAT_INVALID may be undefined, or defined
from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
[pyflakes]
GenBiosId.py:88:80: E501 line too long (92 > 79 characters) [pycodestyle]
GenBiosId.py:88:1: W0401 'FORMAT_INVALID may be undefined, or defined
from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
[pyflakes]
GenBiosId.py:91:80: E501 line too long (101 > 79 characters) [pycodestyle]
GenBiosId.py:91:1: W0401 'FORMAT_UNKNOWN_ERROR may be undefined, or
defined from star imports: Common.BuildToolError, Common.DataType,
Common.Misc' [pyflakes]
GenBiosId.py:95:80: E501 line too long (173 > 79 characters) [pycodestyle]
GenBiosId.py:96:80: E501 line too long (111 > 79 characters) [pycodestyle]
GenBiosId.py:96:14: E127 continuation line over-indented for visual
indent [pycodestyle]
GenBiosId.py:114:1: E305 expected 2 blank lines after class or function
definition, found 1 [pycodestyle]
GenBiosId.py:116:5: E266 too many leading '#' for block comment
[pycodestyle]
GenBiosId.py:117:24: E701 multiple statements on one line (colon)
[pycodestyle]


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
  2019-06-17 10:54 [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId Fan, ZhijuX
  2019-06-17 17:18 ` Michael D Kinney
  2019-06-17 19:05 ` rebecca
@ 2019-06-17 19:11 ` rebecca
  2 siblings, 0 replies; 8+ messages in thread
From: rebecca @ 2019-06-17 19:11 UTC (permalink / raw)
  To: devel, zhijux.fan
  Cc: Gao, Liming, Feng, Bob C, Zhang, Shenglei, Kinney, Michael D

On 2019-06-17 04:54, Fan, ZhijuX wrote:
> +_ConfigSectionNotDefine = "Not support the config file format, need config section"



This might be better worded as:


"Invalid config file format: need a config section"

Or maybe:

"Unsupported config file format: need a config section"


-- 
Rebecca Cran


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
  2019-06-17 17:18 ` Michael D Kinney
@ 2019-06-18 13:14   ` Liming Gao
       [not found]   ` <15A94D43BBFC7A00.29221@groups.io>
  1 sibling, 0 replies; 8+ messages in thread
From: Liming Gao @ 2019-06-18 13:14 UTC (permalink / raw)
  To: Kinney, Michael D, Fan, ZhijuX, devel@edk2.groups.io
  Cc: Feng, Bob C, Zhang, Shenglei

Mike:
  I agree this suggestion. How about introduce -ot option for text file to include this string?

Thanks
Liming
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Tuesday, June 18, 2019 1:18 AM
> To: Fan, ZhijuX <zhijux.fan@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>
> Subject: RE: [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
> 
> We would like to move to Python based tools for
> everything with no requirements to use batch files
> or shell scripts.
> 
> Do we really need the -ob --batch flag?  I looks
> like the feature it provides is a text string of the
> BIOSID.  Can't we send that to stdout or just put
> that text string in an output file without using
> any batch file or shell script specific syntax?
> 
> Thanks,
> 
> Mike
> 
> 
> 
> > -----Original Message-----
> > From: Fan, ZhijuX
> > Sent: Monday, June 17, 2019 3:54 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C
> > <bob.c.feng@intel.com>; Zhang, Shenglei
> > <shenglei.zhang@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: [PATCH V2] BaseTools/GenBiosId: Add a new tool
> > GenBiosId
> >
> > GenBiosId is a tool to generate the BIOS ID binary file
> > which uses the data from the configuration file.
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1846
> >
> > v2:v1 is a tool of C type and v2 is python type.
> >
> > 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/BinWrappers/PosixLike/GenBiosId       |  14
> > +++
> >  BaseTools/BinWrappers/WindowsLike/GenBiosId.bat |   3 +
> >  BaseTools/Source/Python/GenBiosId/BiosId.env    |  27
> > ++++++
> >  BaseTools/Source/Python/GenBiosId/GenBiosId.py  | 118
> > ++++++++++++++++++++++++
> >  4 files changed, 162 insertions(+)
> >  create mode 100644
> > BaseTools/BinWrappers/PosixLike/GenBiosId
> >  create mode 100644
> > BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> >  create mode 100644
> > BaseTools/Source/Python/GenBiosId/BiosId.env
> >  create mode 100644
> > BaseTools/Source/Python/GenBiosId/GenBiosId.py
> >
> > diff --git a/BaseTools/BinWrappers/PosixLike/GenBiosId
> > b/BaseTools/BinWrappers/PosixLike/GenBiosId
> > new file mode 100644
> > index 0000000000..1dd28e9662
> > --- /dev/null
> > +++ b/BaseTools/BinWrappers/PosixLike/GenBiosId
> > @@ -0,0 +1,14 @@
> > +#!/usr/bin/env bash
> > +#python `dirname $0`/RunToolFromSource.py `basename $0`
> > $*
> > +
> > +# If a ${PYTHON_COMMAND} command is available, use it in
> > preference to
> > +python if command -v ${PYTHON_COMMAND} >/dev/null 2>&1;
> > then
> > +    python_exe=${PYTHON_COMMAND}
> > +fi
> > +
> > +full_cmd=${BASH_SOURCE:-$0} # see
> > +http://mywiki.wooledge.org/BashFAQ/028 for a discussion
> > of why $0 is
> > +not a good choice here dir=$(dirname "$full_cmd")
> > exe=$(basename
> > +"$full_cmd")
> > +
> > +export
> > PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTH
> > ONPATH"}"
> > +exec "${python_exe:-python}"
> > "$dir/../../Source/Python/$exe/$exe.py" "$@"
> > diff --git
> > a/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> > b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> > new file mode 100644
> > index 0000000000..e1f61382c8
> > --- /dev/null
> > +++ b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> > @@ -0,0 +1,3 @@
> > +@setlocal
> > +@set ToolName=%~n0%
> > +@%PYTHON_COMMAND%
> > +%BASE_TOOLS_PATH%\Source\Python\%ToolName%\%ToolName%.py
> > %*
> > diff --git a/BaseTools/Source/Python/GenBiosId/BiosId.env
> > b/BaseTools/Source/Python/GenBiosId/BiosId.env
> > new file mode 100644
> > index 0000000000..e1e913da76
> > --- /dev/null
> > +++ b/BaseTools/Source/Python/GenBiosId/BiosId.env
> > @@ -0,0 +1,27 @@
> > +## @file
> > +#  This file is used to define the BIOS ID parameters of
> > the build.
> > +#  This file is processed by GenBiosId.
> > +#  Here, it is just a template and can be customized by
> > user.
> > +#
> > +#  BIOS ID string format:
> > +#
> > $(BOARD_ID)$(BOARD_REV).$(BOARD_EXT).$(VERSION_MAJOR).$(B
> > UILD_TYPE)$(VERSION_MINOR).YYMMDDHHMM
> > +#  All fields must have a fixed length. YYMMDDHHMM is
> > UTC time.
> > +#    Example: "EMLATOR1.000.0001.D01.1906141517"
> > +#
> > +#  If DATE is specified for YYMMDD and TIME is specified
> > for HHMM like
> > +below, #  GenBiosId will use the value of DATE and TIME
> > to fill
> > +YYMMDDHHMM, #  otherwise GenBiosId will fill YYMMDDHHMM
> > with current UTC time of the build machine.
> > +#    DATE          = 190614
> > +#    TIME          = 1517
> > +#
> > +#  Copyright (c) 2019, Intel Corporation. All rights
> > reserved.<BR> #
> > +SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> > [config]
> > +BOARD_ID      = EMLATOR
> > +BOARD_REV     = 1
> > +BOARD_EXT     = 000
> > +BUILD_TYPE    = D
> > +VERSION_MAJOR = 0001
> > +VERSION_MINOR = 01
> > diff --git
> > a/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> > b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> > new file mode 100644
> > index 0000000000..8259b17afd
> > --- /dev/null
> > +++ b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> > @@ -0,0 +1,118 @@
> > +## @file
> > +# Trim files preprocessed by compiler
> > +#
> > +# Copyright (c) 2019, Intel Corporation. All rights
> > reserved.<BR> #
> > +SPDX-License-Identifier: BSD-2-Clause-Patent #
> > +
> > +##
> > +# Import Modules
> > +#
> > +import Common.LongFilePathOs as os
> > +import sys
> > +import struct
> > +import time
> > +import datetime
> > +import argparse
> > +try:
> > +    from configparser import ConfigParser
> > +except:
> > +    from ConfigParser import ConfigParser from
> > Common.BuildToolError
> > +import * from Common.Misc import * from Common.DataType
> > import * from
> > +Common.BuildVersion import gBUILD_VERSION import
> > Common.EdkLogger as
> > +EdkLogger from Common.LongFilePathSupport import
> > OpenLongFilePath as
> > +open
> > +
> > +_BIOS_Signature = "$IBIOSI$"
> > +_SectionKeyName = '__name__'
> > +_SectionName = 'config'
> > +
> > +__prog__ = 'GenBiosld'
> > +__description__ = 'Trim files preprocessed by compiler'
> > +__copyright__ = 'Copyright (c) 2019, Intel Corporation.
> > All rights reserved.<BR> '
> > +__version__ = '%s Version %s' % (__prog__, '0.1 ')
> > +
> > +_ConfigItem = {
> > +    "BOARD_ID"   : {'Value' : '', 'Length' : 7},
> > +    "BOARD_REV"  : {'Value' : '', 'Length' : 1},
> > +    "BOARD_EXT"  : { 'Value' : '', 'Length' : 3},
> > +    "BUILD_TYPE" : {'Value' : '', 'Length' :1},
> > +    "VERSION_MAJOR" : {'Value' : '0000', 'Length' : 4},
> > +    "VERSION_MINOR" : {'Value' : '00', 'Length' : 2},
> > +
> > +}
> > +
> > +
> > +_Usage = "Usage: GenBiosId -i Configfile -o OutputFile
> > [-ob OutputBatchFile]"
> > +_ConfigSectionNotDefine = "Not support the config file
> > format, need config section"
> > +_ConfigLenInvalid = "Config item %s length is invalid"
> > +_ConfigItemInvalid = "Item %s is invalid"
> > +
> > +def Main():
> > +    parser = argparse.ArgumentParser(prog=__prog__,
> > +
> > description=__description__ + __copyright__ + _Usage,
> > +
> > conflict_handler='resolve')
> > +    parser.add_argument('-v', '--version',
> > action='version', version=__version__,
> > +                        help="show program's version
> > number and exit")
> > +    parser.add_argument('-i', '--int',
> > metavar='FILENAME', dest='InputFile', help="Input Config
> > file")
> > +    parser.add_argument('-o', '--out',
> > metavar='FILENAME', dest='OutputFile', help="Output
> > file")
> > +    parser.add_argument('-ob', '--batch',
> > metavar='FILENAME', dest='OutputBatchFile',
> > help="OutputBatch file")
> > +    Options = parser.parse_args()
> > +    try:
> > +        EdkLogger.Initialize()
> > +        if len(sys.argv) !=5 and not (len(sys.argv) == 7
> > and Options.OutputBatchFile):
> > +            EdkLogger.error("GenBiosId", OPTION_MISSING,
> > ExtraData=_Usage)
> > +        elif not Options.InputFile or not
> > Options.OutputFile:
> > +            EdkLogger.error("GenBiosId", OPTION_MISSING,
> > ExtraData=_Usage)
> > +    except FatalError as X:
> > +        return 1
> > +    InputFile = Options.InputFile
> > +    OutputFile = Options.OutputFile
> > +    OutputBatchFile = Options.OutputBatchFile
> > +    if not os.path.exists(InputFile):
> > +        EdkLogger.error("GenBiosId", FILE_NOT_FOUND,
> > ExtraData="Input file not found")
> > +    cf = ConfigParser()
> > +    cf.optionxform = str
> > +    cf.read(InputFile)
> > +    if _SectionName not in cf._sections:
> > +        EdkLogger.error("GenBiosId",
> > FORMAT_NOT_SUPPORTED, ExtraData=_ConfigSectionNotDefine)
> > +    for Item in cf._sections[_SectionName]:
> > +        if Item == _SectionKeyName:
> > +            continue
> > +        if Item not in _ConfigItem:
> > +            EdkLogger.error("GenBiosId", FORMAT_INVALID,
> > ExtraData=_ConfigItemInvalid % Item)
> > +        _ConfigItem[Item]['Value'] =
> > cf._sections[_SectionName][Item]
> > +        if len(_ConfigItem[Item]['Value']) !=
> > _ConfigItem[Item]['Length']:
> > +            EdkLogger.error("GenBiosId", FORMAT_INVALID,
> > ExtraData=_ConfigLenInvalid % Item)
> > +    for Item in _ConfigItem:
> > +        if not _ConfigItem[Item]['Value']:
> > +            EdkLogger.error("GenBiosId",
> > FORMAT_UNKNOWN_ERROR, ExtraData="Item %s is missing" %
> > Item)
> > +    utcnow = datetime.datetime.utcnow()
> > +    TimeStamp = time.strftime("%y%m%d%H%M",
> > utcnow.timetuple())
> > +
> > +    Id_Str = _ConfigItem['BOARD_ID']['Value'] +
> > _ConfigItem['BOARD_REV']['Value'] + '.' +
> > _ConfigItem['BOARD_EXT']['Value'] + '.' +
> > _ConfigItem['VERSION_MAJOR']['Value'] + \
> > +             '.' + _ConfigItem["BUILD_TYPE"]['Value'] +
> > _ConfigItem['VERSION_MINOR']['Value'] + '.' + TimeStamp
> > +    with open(OutputFile, 'wb') as FdOut:
> > +        for i in _BIOS_Signature:
> > +            FdOut.write(struct.pack('B', ord(i)))
> > +
> > +        for i in Id_Str:
> > +            FdOut.write(struct.pack('H', ord(i)))
> > +
> > +        FdOut.write(struct.pack('H', 0x00))
> > +    if OutputBatchFile:
> > +        with open(OutputBatchFile, 'w') as FdOut:
> > +            if sys.platform.startswith('win'):
> > +                Id_Str = 'SET BIOS_ID=' + Id_Str
> > +            else:
> > +                Id_Str = 'export BIOS_ID=' + Id_Str
> > +            FdOut.write(Id_Str)
> > +    return 0
> > +
> > +if __name__ == '__main__':
> > +    r = Main()
> > +    ## 0-127 is a safe return range, and 1 is a standard
> > default error
> > +    if r < 0 or r > 127: r = 1
> > +    sys.exit(r)
> > --
> > 2.14.1.windows.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
  2019-06-17 19:05 ` rebecca
@ 2019-06-18 13:26   ` Liming Gao
  2019-06-18 21:03     ` rebecca
  0 siblings, 1 reply; 8+ messages in thread
From: Liming Gao @ 2019-06-18 13:26 UTC (permalink / raw)
  To: devel@edk2.groups.io, rebecca@bluestop.org, Fan, ZhijuX
  Cc: Feng, Bob C, Zhang, Shenglei, Kinney, Michael D

Rebecca:
  Thanks for your report. Now, pylama is not the requirement for BaseTools python script. Patch contributor can base on the report to fix the critical issue. 

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of rebecca@bluestop.org
> Sent: Tuesday, June 18, 2019 3:05 AM
> To: devel@edk2.groups.io; Fan, ZhijuX <zhijux.fan@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
> 
> On 2019-06-17 04:54, Fan, ZhijuX wrote:
> > GenBiosId is a tool to generate the BIOS ID binary file which uses
> > the data from the configuration file.
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1846
> 
> 
> pylama (https://github.com/klen/pylama) reports the following issues:
> 
> 
> bcran@photon:~/workspace/edk2/BaseTools/Source/Python/GenBiosId % pylama .
> GenBiosId.py:1:1: E266 too many leading '#' for block comment [pycodestyle]
> GenBiosId.py:19:1: E722 do not use bare 'except' [pycodestyle]
> GenBiosId.py:21:1: W0401 'from Common.BuildToolError import *' used;
> unable to detect undefined names [pyflakes]
> GenBiosId.py:22:1: W0401 'from Common.Misc import *' used; unable to
> detect undefined names [pyflakes]
> GenBiosId.py:23:1: W0401 'from Common.DataType import *' used; unable to
> detect undefined names [pyflakes]
> GenBiosId.py:24:1: W0611 'Common.BuildVersion.gBUILD_VERSION' imported
> but unused [pyflakes]
> GenBiosId.py:34:80: E501 line too long (82 > 79 characters) [pycodestyle]
> GenBiosId.py:38:17: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:38:28: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:38:43: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:39:17: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:39:28: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:39:43: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:40:17: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:40:21: E201 whitespace after '{' [pycodestyle]
> GenBiosId.py:40:29: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:40:44: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:41:17: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:41:28: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:41:43: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:41:44: E231 missing whitespace after ':' [pycodestyle]
> GenBiosId.py:42:20: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:42:31: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:42:50: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:43:20: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:43:31: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:43:48: E203 whitespace before ':' [pycodestyle]
> GenBiosId.py:49:80: E501 line too long (83 > 79 characters) [pycodestyle]
> GenBiosId.py:53:1: E302 expected 2 blank lines, found 1 [pycodestyle]
> GenBiosId.py:53:1: C901 'Main' is too complex (17) [mccabe]
> GenBiosId.py:55:80: E501 line too long (90 > 79 characters) [pycodestyle]
> GenBiosId.py:57:80: E501 line too long (81 > 79 characters) [pycodestyle]
> GenBiosId.py:59:80: E501 line too long (102 > 79 characters) [pycodestyle]
> GenBiosId.py:60:80: E501 line too long (97 > 79 characters) [pycodestyle]
> GenBiosId.py:61:80: E501 line too long (110 > 79 characters) [pycodestyle]
> GenBiosId.py:65:80: E501 line too long (86 > 79 characters) [pycodestyle]
> GenBiosId.py:65:28: E225 missing whitespace around operator [pycodestyle]
> GenBiosId.py:66:1: W0401 'OPTION_MISSING may be undefined, or defined
> from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
> [pyflakes]
> GenBiosId.py:68:1: W0401 'OPTION_MISSING may be undefined, or defined
> from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
> [pyflakes]
> GenBiosId.py:69:1: W0401 'FatalError may be undefined, or defined from
> star imports: Common.BuildToolError, Common.DataType, Common.Misc'
> [pyflakes]
> GenBiosId.py:69:1: W0612 local variable 'X' is assigned to but never
> used [pyflakes]
> GenBiosId.py:75:80: E501 line too long (86 > 79 characters) [pycodestyle]
> GenBiosId.py:75:1: W0401 'FILE_NOT_FOUND may be undefined, or defined
> from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
> [pyflakes]
> GenBiosId.py:80:80: E501 line too long (93 > 79 characters) [pycodestyle]
> GenBiosId.py:80:1: W0401 'FORMAT_NOT_SUPPORTED may be undefined, or
> defined from star imports: Common.BuildToolError, Common.DataType,
> Common.Misc' [pyflakes]
> GenBiosId.py:85:80: E501 line too long (93 > 79 characters) [pycodestyle]
> GenBiosId.py:85:1: W0401 'FORMAT_INVALID may be undefined, or defined
> from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
> [pyflakes]
> GenBiosId.py:88:80: E501 line too long (92 > 79 characters) [pycodestyle]
> GenBiosId.py:88:1: W0401 'FORMAT_INVALID may be undefined, or defined
> from star imports: Common.BuildToolError, Common.DataType, Common.Misc'
> [pyflakes]
> GenBiosId.py:91:80: E501 line too long (101 > 79 characters) [pycodestyle]
> GenBiosId.py:91:1: W0401 'FORMAT_UNKNOWN_ERROR may be undefined, or
> defined from star imports: Common.BuildToolError, Common.DataType,
> Common.Misc' [pyflakes]
> GenBiosId.py:95:80: E501 line too long (173 > 79 characters) [pycodestyle]
> GenBiosId.py:96:80: E501 line too long (111 > 79 characters) [pycodestyle]
> GenBiosId.py:96:14: E127 continuation line over-indented for visual
> indent [pycodestyle]
> GenBiosId.py:114:1: E305 expected 2 blank lines after class or function
> definition, found 1 [pycodestyle]
> GenBiosId.py:116:5: E266 too many leading '#' for block comment
> [pycodestyle]
> GenBiosId.py:117:24: E701 multiple statements on one line (colon)
> [pycodestyle]
> 
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
       [not found]   ` <15A94D43BBFC7A00.29221@groups.io>
@ 2019-06-18 14:00     ` Liming Gao
  0 siblings, 0 replies; 8+ messages in thread
From: Liming Gao @ 2019-06-18 14:00 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming, Kinney, Michael D, Fan, ZhijuX
  Cc: Feng, Bob C, Zhang, Shenglei

Zhiju:
  For this tool, I see BiosLib is added into edk2-platforms/Platform/Intel/BoardModulePkg. I would suggest to add GenBiosId into edk2-platforms/Platform/Intel/Tools directory. Can you help update this tool as one standalone script?

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Liming Gao
> Sent: Tuesday, June 18, 2019 9:15 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Fan, ZhijuX <zhijux.fan@intel.com>; devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>
> Subject: Re: [edk2-devel] [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
> 
> Mike:
>   I agree this suggestion. How about introduce -ot option for text file to include this string?
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Tuesday, June 18, 2019 1:18 AM
> > To: Fan, ZhijuX <zhijux.fan@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>
> > Subject: RE: [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
> >
> > We would like to move to Python based tools for
> > everything with no requirements to use batch files
> > or shell scripts.
> >
> > Do we really need the -ob --batch flag?  I looks
> > like the feature it provides is a text string of the
> > BIOSID.  Can't we send that to stdout or just put
> > that text string in an output file without using
> > any batch file or shell script specific syntax?
> >
> > Thanks,
> >
> > Mike
> >
> >
> >
> > > -----Original Message-----
> > > From: Fan, ZhijuX
> > > Sent: Monday, June 17, 2019 3:54 AM
> > > To: devel@edk2.groups.io
> > > Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C
> > > <bob.c.feng@intel.com>; Zhang, Shenglei
> > > <shenglei.zhang@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: [PATCH V2] BaseTools/GenBiosId: Add a new tool
> > > GenBiosId
> > >
> > > GenBiosId is a tool to generate the BIOS ID binary file
> > > which uses the data from the configuration file.
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1846
> > >
> > > v2:v1 is a tool of C type and v2 is python type.
> > >
> > > 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/BinWrappers/PosixLike/GenBiosId       |  14
> > > +++
> > >  BaseTools/BinWrappers/WindowsLike/GenBiosId.bat |   3 +
> > >  BaseTools/Source/Python/GenBiosId/BiosId.env    |  27
> > > ++++++
> > >  BaseTools/Source/Python/GenBiosId/GenBiosId.py  | 118
> > > ++++++++++++++++++++++++
> > >  4 files changed, 162 insertions(+)
> > >  create mode 100644
> > > BaseTools/BinWrappers/PosixLike/GenBiosId
> > >  create mode 100644
> > > BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> > >  create mode 100644
> > > BaseTools/Source/Python/GenBiosId/BiosId.env
> > >  create mode 100644
> > > BaseTools/Source/Python/GenBiosId/GenBiosId.py
> > >
> > > diff --git a/BaseTools/BinWrappers/PosixLike/GenBiosId
> > > b/BaseTools/BinWrappers/PosixLike/GenBiosId
> > > new file mode 100644
> > > index 0000000000..1dd28e9662
> > > --- /dev/null
> > > +++ b/BaseTools/BinWrappers/PosixLike/GenBiosId
> > > @@ -0,0 +1,14 @@
> > > +#!/usr/bin/env bash
> > > +#python `dirname $0`/RunToolFromSource.py `basename $0`
> > > $*
> > > +
> > > +# If a ${PYTHON_COMMAND} command is available, use it in
> > > preference to
> > > +python if command -v ${PYTHON_COMMAND} >/dev/null 2>&1;
> > > then
> > > +    python_exe=${PYTHON_COMMAND}
> > > +fi
> > > +
> > > +full_cmd=${BASH_SOURCE:-$0} # see
> > > +http://mywiki.wooledge.org/BashFAQ/028 for a discussion
> > > of why $0 is
> > > +not a good choice here dir=$(dirname "$full_cmd")
> > > exe=$(basename
> > > +"$full_cmd")
> > > +
> > > +export
> > > PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTH
> > > ONPATH"}"
> > > +exec "${python_exe:-python}"
> > > "$dir/../../Source/Python/$exe/$exe.py" "$@"
> > > diff --git
> > > a/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> > > b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> > > new file mode 100644
> > > index 0000000000..e1f61382c8
> > > --- /dev/null
> > > +++ b/BaseTools/BinWrappers/WindowsLike/GenBiosId.bat
> > > @@ -0,0 +1,3 @@
> > > +@setlocal
> > > +@set ToolName=%~n0%
> > > +@%PYTHON_COMMAND%
> > > +%BASE_TOOLS_PATH%\Source\Python\%ToolName%\%ToolName%.py
> > > %*
> > > diff --git a/BaseTools/Source/Python/GenBiosId/BiosId.env
> > > b/BaseTools/Source/Python/GenBiosId/BiosId.env
> > > new file mode 100644
> > > index 0000000000..e1e913da76
> > > --- /dev/null
> > > +++ b/BaseTools/Source/Python/GenBiosId/BiosId.env
> > > @@ -0,0 +1,27 @@
> > > +## @file
> > > +#  This file is used to define the BIOS ID parameters of
> > > the build.
> > > +#  This file is processed by GenBiosId.
> > > +#  Here, it is just a template and can be customized by
> > > user.
> > > +#
> > > +#  BIOS ID string format:
> > > +#
> > > $(BOARD_ID)$(BOARD_REV).$(BOARD_EXT).$(VERSION_MAJOR).$(B
> > > UILD_TYPE)$(VERSION_MINOR).YYMMDDHHMM
> > > +#  All fields must have a fixed length. YYMMDDHHMM is
> > > UTC time.
> > > +#    Example: "EMLATOR1.000.0001.D01.1906141517"
> > > +#
> > > +#  If DATE is specified for YYMMDD and TIME is specified
> > > for HHMM like
> > > +below, #  GenBiosId will use the value of DATE and TIME
> > > to fill
> > > +YYMMDDHHMM, #  otherwise GenBiosId will fill YYMMDDHHMM
> > > with current UTC time of the build machine.
> > > +#    DATE          = 190614
> > > +#    TIME          = 1517
> > > +#
> > > +#  Copyright (c) 2019, Intel Corporation. All rights
> > > reserved.<BR> #
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> > > [config]
> > > +BOARD_ID      = EMLATOR
> > > +BOARD_REV     = 1
> > > +BOARD_EXT     = 000
> > > +BUILD_TYPE    = D
> > > +VERSION_MAJOR = 0001
> > > +VERSION_MINOR = 01
> > > diff --git
> > > a/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> > > b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> > > new file mode 100644
> > > index 0000000000..8259b17afd
> > > --- /dev/null
> > > +++ b/BaseTools/Source/Python/GenBiosId/GenBiosId.py
> > > @@ -0,0 +1,118 @@
> > > +## @file
> > > +# Trim files preprocessed by compiler
> > > +#
> > > +# Copyright (c) 2019, Intel Corporation. All rights
> > > reserved.<BR> #
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent #
> > > +
> > > +##
> > > +# Import Modules
> > > +#
> > > +import Common.LongFilePathOs as os
> > > +import sys
> > > +import struct
> > > +import time
> > > +import datetime
> > > +import argparse
> > > +try:
> > > +    from configparser import ConfigParser
> > > +except:
> > > +    from ConfigParser import ConfigParser from
> > > Common.BuildToolError
> > > +import * from Common.Misc import * from Common.DataType
> > > import * from
> > > +Common.BuildVersion import gBUILD_VERSION import
> > > Common.EdkLogger as
> > > +EdkLogger from Common.LongFilePathSupport import
> > > OpenLongFilePath as
> > > +open
> > > +
> > > +_BIOS_Signature = "$IBIOSI$"
> > > +_SectionKeyName = '__name__'
> > > +_SectionName = 'config'
> > > +
> > > +__prog__ = 'GenBiosld'
> > > +__description__ = 'Trim files preprocessed by compiler'
> > > +__copyright__ = 'Copyright (c) 2019, Intel Corporation.
> > > All rights reserved.<BR> '
> > > +__version__ = '%s Version %s' % (__prog__, '0.1 ')
> > > +
> > > +_ConfigItem = {
> > > +    "BOARD_ID"   : {'Value' : '', 'Length' : 7},
> > > +    "BOARD_REV"  : {'Value' : '', 'Length' : 1},
> > > +    "BOARD_EXT"  : { 'Value' : '', 'Length' : 3},
> > > +    "BUILD_TYPE" : {'Value' : '', 'Length' :1},
> > > +    "VERSION_MAJOR" : {'Value' : '0000', 'Length' : 4},
> > > +    "VERSION_MINOR" : {'Value' : '00', 'Length' : 2},
> > > +
> > > +}
> > > +
> > > +
> > > +_Usage = "Usage: GenBiosId -i Configfile -o OutputFile
> > > [-ob OutputBatchFile]"
> > > +_ConfigSectionNotDefine = "Not support the config file
> > > format, need config section"
> > > +_ConfigLenInvalid = "Config item %s length is invalid"
> > > +_ConfigItemInvalid = "Item %s is invalid"
> > > +
> > > +def Main():
> > > +    parser = argparse.ArgumentParser(prog=__prog__,
> > > +
> > > description=__description__ + __copyright__ + _Usage,
> > > +
> > > conflict_handler='resolve')
> > > +    parser.add_argument('-v', '--version',
> > > action='version', version=__version__,
> > > +                        help="show program's version
> > > number and exit")
> > > +    parser.add_argument('-i', '--int',
> > > metavar='FILENAME', dest='InputFile', help="Input Config
> > > file")
> > > +    parser.add_argument('-o', '--out',
> > > metavar='FILENAME', dest='OutputFile', help="Output
> > > file")
> > > +    parser.add_argument('-ob', '--batch',
> > > metavar='FILENAME', dest='OutputBatchFile',
> > > help="OutputBatch file")
> > > +    Options = parser.parse_args()
> > > +    try:
> > > +        EdkLogger.Initialize()
> > > +        if len(sys.argv) !=5 and not (len(sys.argv) == 7
> > > and Options.OutputBatchFile):
> > > +            EdkLogger.error("GenBiosId", OPTION_MISSING,
> > > ExtraData=_Usage)
> > > +        elif not Options.InputFile or not
> > > Options.OutputFile:
> > > +            EdkLogger.error("GenBiosId", OPTION_MISSING,
> > > ExtraData=_Usage)
> > > +    except FatalError as X:
> > > +        return 1
> > > +    InputFile = Options.InputFile
> > > +    OutputFile = Options.OutputFile
> > > +    OutputBatchFile = Options.OutputBatchFile
> > > +    if not os.path.exists(InputFile):
> > > +        EdkLogger.error("GenBiosId", FILE_NOT_FOUND,
> > > ExtraData="Input file not found")
> > > +    cf = ConfigParser()
> > > +    cf.optionxform = str
> > > +    cf.read(InputFile)
> > > +    if _SectionName not in cf._sections:
> > > +        EdkLogger.error("GenBiosId",
> > > FORMAT_NOT_SUPPORTED, ExtraData=_ConfigSectionNotDefine)
> > > +    for Item in cf._sections[_SectionName]:
> > > +        if Item == _SectionKeyName:
> > > +            continue
> > > +        if Item not in _ConfigItem:
> > > +            EdkLogger.error("GenBiosId", FORMAT_INVALID,
> > > ExtraData=_ConfigItemInvalid % Item)
> > > +        _ConfigItem[Item]['Value'] =
> > > cf._sections[_SectionName][Item]
> > > +        if len(_ConfigItem[Item]['Value']) !=
> > > _ConfigItem[Item]['Length']:
> > > +            EdkLogger.error("GenBiosId", FORMAT_INVALID,
> > > ExtraData=_ConfigLenInvalid % Item)
> > > +    for Item in _ConfigItem:
> > > +        if not _ConfigItem[Item]['Value']:
> > > +            EdkLogger.error("GenBiosId",
> > > FORMAT_UNKNOWN_ERROR, ExtraData="Item %s is missing" %
> > > Item)
> > > +    utcnow = datetime.datetime.utcnow()
> > > +    TimeStamp = time.strftime("%y%m%d%H%M",
> > > utcnow.timetuple())
> > > +
> > > +    Id_Str = _ConfigItem['BOARD_ID']['Value'] +
> > > _ConfigItem['BOARD_REV']['Value'] + '.' +
> > > _ConfigItem['BOARD_EXT']['Value'] + '.' +
> > > _ConfigItem['VERSION_MAJOR']['Value'] + \
> > > +             '.' + _ConfigItem["BUILD_TYPE"]['Value'] +
> > > _ConfigItem['VERSION_MINOR']['Value'] + '.' + TimeStamp
> > > +    with open(OutputFile, 'wb') as FdOut:
> > > +        for i in _BIOS_Signature:
> > > +            FdOut.write(struct.pack('B', ord(i)))
> > > +
> > > +        for i in Id_Str:
> > > +            FdOut.write(struct.pack('H', ord(i)))
> > > +
> > > +        FdOut.write(struct.pack('H', 0x00))
> > > +    if OutputBatchFile:
> > > +        with open(OutputBatchFile, 'w') as FdOut:
> > > +            if sys.platform.startswith('win'):
> > > +                Id_Str = 'SET BIOS_ID=' + Id_Str
> > > +            else:
> > > +                Id_Str = 'export BIOS_ID=' + Id_Str
> > > +            FdOut.write(Id_Str)
> > > +    return 0
> > > +
> > > +if __name__ == '__main__':
> > > +    r = Main()
> > > +    ## 0-127 is a safe return range, and 1 is a standard
> > > default error
> > > +    if r < 0 or r > 127: r = 1
> > > +    sys.exit(r)
> > > --
> > > 2.14.1.windows.1
> 
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId
  2019-06-18 13:26   ` Liming Gao
@ 2019-06-18 21:03     ` rebecca
  0 siblings, 0 replies; 8+ messages in thread
From: rebecca @ 2019-06-18 21:03 UTC (permalink / raw)
  To: devel, liming.gao, Fan, ZhijuX
  Cc: Feng, Bob C, Zhang, Shenglei, Kinney, Michael D

On 2019-06-18 07:26, Liming Gao wrote:
> Rebecca:
>   Thanks for your report. Now, pylama is not the requirement for BaseTools python script. Patch contributor can base on the report to fix the critical issue. 


I know it's not a required tool, but many of these are common Python
style issues that might be nice to fix.


-- 

Rebecca Cran


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-06-18 21:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-17 10:54 [PATCH V2] BaseTools/GenBiosId: Add a new tool GenBiosId Fan, ZhijuX
2019-06-17 17:18 ` Michael D Kinney
2019-06-18 13:14   ` Liming Gao
     [not found]   ` <15A94D43BBFC7A00.29221@groups.io>
2019-06-18 14:00     ` [edk2-devel] " Liming Gao
2019-06-17 19:05 ` rebecca
2019-06-18 13:26   ` Liming Gao
2019-06-18 21:03     ` rebecca
2019-06-17 19:11 ` rebecca

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