[PATCH v2 2/2] tools: binman: ti_board_cfg: Check for linting problems

[Ryan Eatmon]

On 1/22/2024 6:59 AM, Max Krummenacher wrote:
> Hi
> 
> On Fri, Jan 05, 2024 at 05:09:17PM +0530, Neha Malcom Francis wrote:
>> Use yamllint for checking whether YAML configuration files are adhering
>> to default yamllint rules.
> 
> If I understand this correctly this patch now runs checks to verify
> that yaml files which are part of the U-Boot source tree are correct.
> 
> Shouldn't this be done when one commits a yaml file, i.e. in U-Boot CI
> rather than repeating the process on every build and thus having an
> additional dependency to build U-Boot and additional build time?
> 
> Note that in openembedded there is currently no python yamllint
> recipe providing the used python module in the regularly used layers.
> How do you plan to integrate the change in the OE U-Boot recipe?
> At least our OE CI of latest master now fails (for a TI AM62 based SoM)
> as the python module is missing.

I have submitted a patch to oe-core for this missing recipe.

https://lists.openembedded.org/g/openembedded-core/message/194192

Hopefully they accept it.  Otherwise, we are going to "temporarily" 
carrying the same recipe in meta-ti-extras until they accept it in oe-core.


> Regards
> Max
> 
>>
>> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
>> Suggested-by: Nishanth Menon <nm at ti.com>
>> ---
>> Changes since v1:
>> 	- add yamllint to requirements.txt (Nishanth)
>>
>>   tools/binman/etype/ti_board_config.py        |  5 +++++
>>   tools/binman/ftest.py                        |  6 ++++++
>>   tools/binman/test/323_ti_board_cfg_phony.dts | 14 ++++++++++++++
>>   tools/binman/test/yaml/config.yaml           |  4 ++--
>>   tools/binman/test/yaml/config_phony.yaml     | 18 ++++++++++++++++++
>>   tools/buildman/requirements.txt              |  1 +
>>   6 files changed, 46 insertions(+), 2 deletions(-)
>>   create mode 100644 tools/binman/test/323_ti_board_cfg_phony.dts
>>   create mode 100644 tools/binman/test/yaml/config_phony.yaml
>>
>> diff --git a/tools/binman/etype/ti_board_config.py b/tools/binman/etype/ti_board_config.py
>> index 94f894c281..2c3bb8f7b5 100644
>> --- a/tools/binman/etype/ti_board_config.py
>> +++ b/tools/binman/etype/ti_board_config.py
>> @@ -9,6 +9,7 @@
>>   import os
>>   import struct
>>   import yaml
>> +import yamllint
>>   
>>   from collections import OrderedDict
>>   from jsonschema import validate
>> @@ -18,6 +19,7 @@ from binman.entry import Entry
>>   from binman.etype.section import Entry_section
>>   from dtoc import fdt_util
>>   from u_boot_pylib import tools
>> +from yamllint import config
>>   
>>   BOARDCFG = 0xB
>>   BOARDCFG_SEC = 0xD
>> @@ -244,6 +246,9 @@ class Entry_ti_board_config(Entry_section):
>>               with open(self._schema_file, 'r') as sch:
>>                   self.schema_yaml = yaml.safe_load(sch)
>>   
>> +            yaml_config = config.YamlLintConfig("extends: default")
>> +            for p in yamllint.linter.run(open(self._config_file, "r"), yaml_config):
>> +                self.Raise(f"Yamllint error: {p.line}: {p.rule}")
>>               try:
>>                   validate(self.file_yaml, self.schema_yaml)
>>               except Exception as e:
>> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
>> index a4ac520cbb..1fbb0fef1a 100644
>> --- a/tools/binman/ftest.py
>> +++ b/tools/binman/ftest.py
>> @@ -7030,6 +7030,12 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
>>           data = self._DoReadFile('293_ti_board_cfg.dts')
>>           self.assertEqual(TI_BOARD_CONFIG_DATA, data)
>>   
>> +    def testTIBoardConfigLint(self):
>> +        """Test that an incorrectly linted config file would generate error"""
>> +        with self.assertRaises(ValueError) as e:
>> +            data = self._DoReadFile('323_ti_board_cfg_phony.dts')
>> +        self.assertIn("Yamllint error", str(e.exception))
>> +
>>       def testTIBoardConfigCombined(self):
>>           """Test that a schema validated combined board config file can be generated"""
>>           data = self._DoReadFile('294_ti_board_cfg_combined.dts')
>> diff --git a/tools/binman/test/323_ti_board_cfg_phony.dts b/tools/binman/test/323_ti_board_cfg_phony.dts
>> new file mode 100644
>> index 0000000000..441296de4f
>> --- /dev/null
>> +++ b/tools/binman/test/323_ti_board_cfg_phony.dts
>> @@ -0,0 +1,14 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/dts-v1/;
>> +
>> +/ {
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +
>> +	binman {
>> +		ti-board-config {
>> +			config = "yaml/config_phony.yaml";
>> +			schema = "yaml/schema.yaml";
>> +		};
>> +	};
>> +};
>> diff --git a/tools/binman/test/yaml/config.yaml b/tools/binman/test/yaml/config.yaml
>> index 5f799a6e3a..c2be32128b 100644
>> --- a/tools/binman/test/yaml/config.yaml
>> +++ b/tools/binman/test/yaml/config.yaml
>> @@ -10,9 +10,9 @@ main-branch:
>>       b: 0
>>     arr: [0, 0, 0, 0]
>>     another-arr:
>> -    - #1
>> +    -  # 1
>>         c: 0
>>         d: 0
>> -    - #2
>> +    -  # 2
>>         c: 0
>>         d: 0
>> diff --git a/tools/binman/test/yaml/config_phony.yaml b/tools/binman/test/yaml/config_phony.yaml
>> new file mode 100644
>> index 0000000000..d76fcb3b82
>> --- /dev/null
>> +++ b/tools/binman/test/yaml/config_phony.yaml
>> @@ -0,0 +1,18 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +# Test config
>> +#
>> +---
>> +
>> +main-branch :
>> +  obj :
>> +    a : 0x0
>> +    b: 0
>> +  arr: [0, 0, 0, 0]
>> +  another-arr:
>> +    -  # 1
>> +      c: 0
>> +      d: 0
>> +    -  # 2
>> +      c: 0
>> +      d: 0
>> diff --git a/tools/buildman/requirements.txt b/tools/buildman/requirements.txt
>> index a1efcb9d4b..4a31e69e4c 100644
>> --- a/tools/buildman/requirements.txt
>> +++ b/tools/buildman/requirements.txt
>> @@ -1,2 +1,3 @@
>>   jsonschema==4.17.3
>>   pyyaml==6.0
>> +yamllint==1.26.3
>> -- 
>> 2.34.1
>>

-- 
Ryan Eatmon                reatmon at ti.com
-----------------------------------------
Texas Instruments, Inc.  -  LCPD  -  MGTS