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 <[email protected]>
> Cc: Yonghong Zhu <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jaben Carsey <[email protected]>
> ---
>  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
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to