On Fri, Jun 21, 2019 at 09:47:37AM +0000, Fan, ZhijuX wrote:
> Hi:
>
> Thank you for your comments. The output 1 was designed as a standard error.
> Function EdkLogger () sys.exit(1) "1" missing while writing the script
>
>
>
>
> Any question, please let me know. Thanks.
>
> Best Regards
> Fan Zhiju
>
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Leif Lindholm
> > Sent: Friday, June 21, 2019 5:01 PM
> > To: Fan, ZhijuX <zhijux....@intel.com>
> > Cc: devel@edk2.groups.io; Gao, Liming <liming....@intel.com>; Feng, Bob C
> > <bob.c.f...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>;
> > Kinney, Michael D <michael.d.kin...@intel.com>
> > Subject: Re: [edk2-devel] [edk2-platform patch 1/2] Platform/Intel:Add
> > GenBiosId into edk2-platforms/Platform/Intel/Tools
> >
> > On Fri, Jun 21, 2019 at 01:58:17AM +0000, Fan, ZhijuX wrote:
> > > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1855
> > >
> > > GenBiosId is a tool to generate the BIOS ID binary file which uses the
> > > data from the configuration file.
> > > This tool can be run in both Py2 and Py3.
> > >
> > > This patch is going to add GenBiosId to Platform/Intel/Tools
> > >
> > > Cc: Liming Gao <liming....@intel.com>
> > > Cc: Bob Feng <bob.c.f...@intel.com>
> > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> > > Cc: Leif Lindholm <leif.lindh...@linaro.org>
> > > Cc: Michael D Kinney <michael.d.kin...@intel.com>
> > > Signed-off-by: Zhiju.Fan <zhijux....@intel.com>
> > > ---
> > > Platform/Intel/Tools/GenBiosId/BiosId.env | 27 +++++
> > > Platform/Intel/Tools/GenBiosId/GenBiosId.py | 180
> > > ++++++++++++++++++++++++++++
> > > 2 files changed, 207 insertions(+)
> > > create mode 100644 Platform/Intel/Tools/GenBiosId/BiosId.env
> > > create mode 100644 Platform/Intel/Tools/GenBiosId/GenBiosId.py
> > >
> > > diff --git a/Platform/Intel/Tools/GenBiosId/BiosId.env
> > > b/Platform/Intel/Tools/GenBiosId/BiosId.env
> > > new file mode 100644
> > > index 0000000000..e1e913da76
> > > --- /dev/null
> > > +++ b/Platform/Intel/Tools/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_T
> > YPE)$(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 = KBLRVP3
> > > +BOARD_REV = 1
> > > +BOARD_EXT = 000
> > > +BUILD_TYPE = D
> > > +VERSION_MAJOR = 0001
> > > +VERSION_MINOR = 01
> > > diff --git a/Platform/Intel/Tools/GenBiosId/GenBiosId.py
> > > b/Platform/Intel/Tools/GenBiosId/GenBiosId.py
> > > new file mode 100644
> > > index 0000000000..20fb7592b4
> > > --- /dev/null
> > > +++ b/Platform/Intel/Tools/GenBiosId/GenBiosId.py
> > > @@ -0,0 +1,180 @@
> > > +## @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 os
> > > +import sys
> > > +import time
> > > +import logging
> > > +import struct
> > > +import datetime
> > > +import argparse
> > > +import platform
> > > +
> > > +try:
> > > + from configparser import ConfigParser
> > > +except:
> > > + from ConfigParser import ConfigParser
> > > +
> > > +# Config message
> > > +_BIOS_Signature = "$IBIOSI$"
> > > +_SectionKeyName = '__name__'
> > > +_SectionName = 'config'
> > > +
> > > +_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}, }
> > > +
> > > +# Version message
> > > +__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 ')
> > > +
> > > +# ExtraData message
> > > +_Usage = "Usage: GenBiosId -i Configfile -o OutputFile [-ot
> > OutputTextFile]"
> > > +_ConfigSectionNotDefine = "Not support the config file format, need
> > config section"
> > > +_ErrorMessageTemplate = '\n\n%(tool)s...\n :
> > error: %(msg)s\n\t%(extra)s'
> > > +_ErrorLogger = logging.getLogger("tool_error") _ErrorFormatter =
> > > +logging.Formatter("%(message)s") _ConfigLenInvalid = "Config item %s
> > > +length is invalid"
> > > +_ConfigItemInvalid = "Item %s is invalid"
> > > +
> > > +# Error message
> > > +INFO = 20
> > > +ERRORCODE = 50
> > > +OPTION_MISSING = 'Missing option'
> > > +FORMAT_INVALID = 'Invalid syntax/format'
> > > +FILE_NOT_FOUND = 'File/directory not found in workspace'
> > > +FORMAT_UNKNOWN_ERROR = 'Unknown error in syntax/format'
> > > +FORMAT_NOT_SUPPORTED = 'Not supported syntax/format'
> > > +
> > > +
> > > +def SetEdkLogger():
> > > + _ErrorLogger.setLevel(INFO)
> > > + _ErrorCh = logging.StreamHandler(sys.stderr)
> > > + _ErrorCh.setFormatter(_ErrorFormatter)
> > > + _ErrorLogger.addHandler(_ErrorCh)
> > > + return _ErrorLogger
> > > +
> > > +
> > > +# Output the error message and exit the tool def EdkLogger(ToolName,
> > > +Message, ExtraData):
> > > + _ErrorLogger = SetEdkLogger()
> > > + TemplateDict = {"tool": ToolName, "msg": Message, "extra": ExtraData}
> > > + LogText = _ErrorMessageTemplate % TemplateDict
> > > + _ErrorLogger.log(ERRORCODE, LogText)
> > > + sys.exit()
> >
> > Should the above two functions be in a common module in edk2?
> >
> > > +
> > > +
> > > +# Open the file in the correct way
> > > +def FileOpen(FileName, Mode, Buffer=-1):
> > > + def LongFilePath(FileName):
> > > + FileName = os.path.normpath(FileName)
> > > + if platform.system() == 'Windows':
> > > + if FileName.startswith('\\\\?\\'):
> > > + return FileName
> > > + if FileName.startswith('\\\\'):
> > > + return '\\\\?\\UNC\\' + FileName[2:]
> > > + if os.path.isabs(FileName):
> > > + return '\\\\?\\' + FileName
> > > + return FileName
> > > +
> > > + return open(LongFilePath(FileName), Mode, Buffer)
> > > +
> > > +
> > > +# Parse command line options
> > > +def MyOptionParser():
> > > + 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('-ot', '--text', metavar='FILENAME',
> > > +dest='OutputTextFile', help="Output Text file")
> >
> > '-ot' is long for a short argument. Could it just be '-t', which also
> > matches '--
> > text' better?
> >
> > > + Options = parser.parse_args()
> > > + return Options
> > > +
> > > +
> > > +# Check the Tool for missing variables def CheckOptions(Options):
> > > + if len(sys.argv) != 5 and not (len(sys.argv) == 7 and
> > Options.OutputTextFile):
> > > + EdkLogger("GenBiosId", OPTION_MISSING, ExtraData=_Usage)
> > > + elif not Options.InputFile or not Options.OutputFile:
> > > + EdkLogger("GenBiosId", OPTION_MISSING, ExtraData=_Usage)
> >
> > Can we use argparse to validate the arguments instead - that's what it's
> > for?
> >
> > > + InputFile = Options.InputFile
> > > + OutputFile = Options.OutputFile
> > > + OutputTextFile = Options.OutputTextFile
> > > + if not os.path.exists(InputFile):
> > > + EdkLogger("GenBiosId", FILE_NOT_FOUND, ExtraData="Input file
> > > + not found")
> >
> > https://stackoverflow.com/questions/37471636/python-argument-parsing-
> > validation-best-practices
> > gives an example on how to make argparse verify that files exist.
> >
> > > + return InputFile, OutputFile, OutputTextFile
> > > +
> > > +
> > > +# Parse the input file and extract the information def
> > > +ParserInputFile(InputFile):
> > > + cf = ConfigParser()
> > > + cf.optionxform = str
> > > + cf.read(InputFile)
> > > + if _SectionName not in cf._sections:
> > > + EdkLogger("GenBiosId", FORMAT_NOT_SUPPORTED,
> > ExtraData=_ConfigSectionNotDefine)
> > > + for Item in cf._sections[_SectionName]:
> > > + if Item == _SectionKeyName:
> > > + continue
> > > + if Item not in _ConfigItem:
> > > + EdkLogger("GenBiosId", FORMAT_INVALID,
> > ExtraData=_ConfigItemInvalid % Item)
> > > + _ConfigItem[Item]['Value'] = cf._sections[_SectionName][Item]
> > > + if len(_ConfigItem[Item]['Value']) !=
> > > _ConfigItem[Item]['Length']:
> > > + EdkLogger("GenBiosId", FORMAT_INVALID,
> > ExtraData=_ConfigLenInvalid % Item)
> > > + for Item in _ConfigItem:
> > > + if not _ConfigItem[Item]['Value']:
> > > + EdkLogger("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
> > > + return Id_Str
> > > +
> > > +
> > > +# Output information to a file
> > > +def PrintOutputFile(OutputFile, OutputTextFile, Id_Str):
> > > + with FileOpen(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 OutputTextFile:
> > > + with FileOpen(OutputTextFile, 'w') as FdOut:
> > > + FdOut.write(Id_Str)
> > > +
> > > +
> > > +# Tool entrance method
> > > +def Main():
> > > + Options = MyOptionParser()
> > > + InputFile, OutputFile, OutputTextFile = CheckOptions(Options)
> > > + Id_Str = ParserInputFile(InputFile)
> > > + PrintOutputFile(OutputFile, OutputTextFile, 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
> >
> > What is the point of this dance? Main always returns 0 (which does not sound
> > ideal).
> >
> > > + sys.exit(r)
> >
> > Implications for asynchronous i/o?
> >
> > /
> > Leif
> >
> > > --
> > > 2.14.1.windows.1
> > >
> >
> >
> >
> >
>
Can you please respond inline, next to the points being raised.
We are already communicating in what is not either of our first
language, about a non-human language. We do not need to increase the
difficulty level.
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#42697): https://edk2.groups.io/g/devel/message/42697
Mute This Topic: https://groups.io/mt/32154571/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
