From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=UX+yIjb/; spf=pass (domain: linaro.org, ip: 209.85.221.47, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by groups.io with SMTP; Fri, 21 Jun 2019 03:26:39 -0700 Received: by mail-wr1-f47.google.com with SMTP id r16so6007573wrl.11 for ; Fri, 21 Jun 2019 03:26:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=uvEJlkTyEU4mcPdlrxKlT7+FoY9qC3jazFZhuez31j8=; b=UX+yIjb/vIVqEj7kMPxc9lF2tszjWxOG+u+WWZzpFJFnG0A3s/Ap7X78cIXzC/hNiq Bv7rjhdM6u2pEa6i1PBXLMKirHMRtntoCGGxAQwhKRjnzUWsx8/nq2SBuzMgrtLdir+1 vvCpdwTX4nMu+HA5CCeR6rCcJeW9Pg5/7l8ZJ/xDkM8p1ZMiE8ZMPQ8mHjI5LrGpchEW gUIOEZvOL2HtEpaeDAGlWsmkAFAxPIDMS2xpsr4MuKxkX+KyG8CgfxDzP0Atqr1wph1w sTNNN9IXtgMHgGSY1RLYp5yT/v2M8Vs38eaPDVjYoTXYhFmWu9kj2oIBY6qg1c3S/ahF r+Mg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=uvEJlkTyEU4mcPdlrxKlT7+FoY9qC3jazFZhuez31j8=; b=Itu5OhZdVE+azcnxQFDkzW6m9/mikjrJF4v8eZTMENsE5c2a+IspjgRifHIJnRD1pN G+HcM3+qut+EcoTRm1/OcnBYV3pL21EYpyYNANTAb0E5AxLX83SI7QPN+/NBsJVHom+A yDxZ3yxSxh8a+a/PWr6M7/nA2bUftMPelySfEgH71SzT8rQqlfzlOwa44bLmsS4AXB4E gZYQDvtViPMUqjwYQavx+D5wGWE8+PG0sTg2+9BVO3y0ZYSwq2tjbz1AO28qzCTxbB5H Rnow7rjx86i14QYKO6WvBqUb1pLH69kJ1eTRvdyEGOTL3NGS78+ZjMjotyvJvyBS9emw 19Ww== X-Gm-Message-State: APjAAAVEnj47l8/XqIgOxjThX/z5KbHaSppnlIm18D/l9s4iwS2xuieM /PRK6mqhUTGoSghIMRa619bvKw== X-Google-Smtp-Source: APXvYqxvaJ9rFo2nIIOhfaGkmuUvbjSSlnh55/qCpg+QbAj+AiEWWPRAjAgIqGXXfdC9tCknK3ibcA== X-Received: by 2002:adf:db81:: with SMTP id u1mr93374704wri.296.1561112797556; Fri, 21 Jun 2019 03:26:37 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id a2sm3158330wmj.9.2019.06.21.03.26.36 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 21 Jun 2019 03:26:36 -0700 (PDT) Date: Fri, 21 Jun 2019 11:26:34 +0100 From: "Leif Lindholm" To: "Fan, ZhijuX" Cc: "devel@edk2.groups.io" , "Gao, Liming" , "Feng, Bob C" , Ard Biesheuvel , "Kinney, Michael D" Subject: Re: [edk2-devel] [edk2-platform patch 1/2] Platform/Intel:Add GenBiosId into edk2-platforms/Platform/Intel/Tools Message-ID: <20190621102634.q4kyphtbvod5qg67@bivouac.eciton.net> References: <20190621090050.px3t4a7cdldorpnf@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > > Cc: devel@edk2.groups.io; Gao, Liming ; Feng, Bob C > > ; Ard Biesheuvel ; > > Kinney, Michael D > > 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 > > > Cc: Bob Feng > > > Cc: Ard Biesheuvel > > > Cc: Leif Lindholm > > > Cc: Michael D Kinney > > > Signed-off-by: Zhiju.Fan > > > --- > > > 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.
# > > > +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.
# 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.
' > > > +__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