From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 405B22033D1C0 for ; Fri, 4 May 2018 04:14:43 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7316C81A88A8; Fri, 4 May 2018 11:14:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-129.rdu2.redhat.com [10.10.120.129]) by smtp.corp.redhat.com (Postfix) with ESMTP id AC7D52024CA1; Fri, 4 May 2018 11:14:41 +0000 (UTC) To: Jaben Carsey Cc: edk2-devel@lists.01.org, Liming Gao References: From: Laszlo Ersek Message-ID: <814bebab-3b06-ce21-4fbb-326cc0a49121@redhat.com> Date: Fri, 4 May 2018 13:14:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 04 May 2018 11:14:42 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 04 May 2018 11:14:42 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH v1 17/27] BaseTools: DataType - cleanup list constants X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 May 2018 11:14:44 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi, On 04/20/18 17:51, Jaben Carsey wrote: > remove unused ones > convert lists used for membership testing to sets > use shared ones not local ones > > Cc: Liming Gao > Cc: Yonghong Zhu > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jaben Carsey > --- > BaseTools/Source/Python/AutoGen/AutoGen.py | 32 ++++++++--------- > BaseTools/Source/Python/AutoGen/GenC.py | 36 ++++++++------------ > BaseTools/Source/Python/AutoGen/GenPcdDb.py | 4 +-- > BaseTools/Source/Python/Common/DataType.py | 29 ++++++---------- > BaseTools/Source/Python/Ecc/Configuration.py | 2 +- > BaseTools/Source/Python/Ecc/c.py | 2 +- > BaseTools/Source/Python/GenFds/FdfParser.py | 2 +- > BaseTools/Source/Python/GenFds/Ffs.py | 20 +---------- > BaseTools/Source/Python/GenFds/OptRomInfStatement.py | 1 - > BaseTools/Source/Python/Workspace/InfBuildData.py | 2 +- > BaseTools/Source/Python/Workspace/MetaFileParser.py | 4 +-- > BaseTools/Source/Python/build/build.py | 4 +-- > 12 files changed, 53 insertions(+), 85 deletions(-) I think this patch (commit eece4292acc80) broke the Ecc tool: > diff --git a/BaseTools/Source/Python/Ecc/Configuration.py b/BaseTools/Source/Python/Ecc/Configuration.py > index b523858e1b1f..b5b583be8c4a 100644 > --- a/BaseTools/Source/Python/Ecc/Configuration.py > +++ b/BaseTools/Source/Python/Ecc/Configuration.py > @@ -53,7 +53,7 @@ class Configuration(object): > > # List customized Modifer here, split with ',' > # Defaultly use the definition in class DataType > - self.ModifierList = MODIFIER_LIST > + self.ModifierSet = MODIFIER_SET > > ## General Checking > self.GeneralCheckAll = 0 When I run Ecc, it prints: BaseTools/Source/Python/Ecc/config.ini(44): error F004: Invalid configuration option 'ModifierList' was found The error comes from "BaseTools/Source/Python/Ecc/Configuration.py": LineNo = 0 for Line in open(Filepath, 'r'): LineNo = LineNo + 1 Line = CleanString(Line) if Line != '': List = GetSplitValueList(Line, TAB_EQUAL_SPLIT) if List[0] not in self.__dict__: ErrorMsg = "Invalid configuration option '%s' was found" % List[0] EdkLogger.error("Ecc", EdkLogger.ECC_ERROR, ErrorMsg, File = Filepath, Line = LineNo) if List[0] == 'ModifierList': List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT) if List[0] == 'MetaDataFileCheckPathOfGenerateFileList' and List[1] == "": continue if List[0] == 'SkipDirList': List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT) if List[0] == 'SkipFileList': List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT) if List[0] == 'BinaryExtList': List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT) if List[0] == 'Copyright': List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT) self.__dict__[List[0]] = List[1] Basically, the condition List[0] not in self.__dict__ expects that option names in the "config.ini" file match the attributes / members of the Python object directly. Thus, if we rename a member, such as from "ModifierList" to "ModifierSet", then all the config files that used to contain "ModifierList" now have to rename their "ModifierList" options to "ModifierSet" as well. This looks brittle to me; option names in config files are public, while object member names are an implementation detail. They shouldn't be coupled. Thanks Laszlo