Re: [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-fran...@ti.com>
Suggested-by: Nishanth Menon <n...@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                reat...@ti.com
-----------------------------------------
Texas Instruments, Inc.  -  LCPD  -  MGTS