cmake: shields: skip validating SHIELD if its not defined

[gudnimg]

When SHIELD is not set, no shields are applied but list_shields.py still runs which takes around 140ms. Then additional 100ms are spent on string operations in CMake.

This reduces the build configuration time by ~250ms on my machine.

---

This is the same idea as #103709

It would be nice to avoid parsing all shields if possible when the build doesn't use any shields.

---

Before (271ms):

After (4.5ms):

[nordicjm]

is a good idea, however is not acceptable in the current form. You would need to add a test which is always executed on every CI run that includes a shield so that all shields in-tree (or e.g. added by a PR) are checked to be valid, because this PR removes that check

[gudnimg]

is a good idea, however is not acceptable in the current form. You would need to add a test which is always executed on every CI run that includes a shield so that all shields in-tree (or e.g. added by a PR) are checked to be valid, because this PR removes that check

I can try writing a test for this. But I am not sure how to enforce the test is always run. Is there a specific tag I should use in the testcase YAML file?

[nordicjm]

is a good idea, however is not acceptable in the current form. You would need to add a test which is always executed on every CI run that includes a shield so that all shields in-tree (or e.g. added by a PR) are checked to be valid, because this PR removes that check

I can try writing a test for this. But I am not sure how to enforce the test is always run. Is there a specific tag I should use in the testcase YAML file?

@PerMac @gchwier any thoughts?

[PerMac]

The scope for testing is automatically determined by test_plan.py script based on changes in a PR. On a case, where only a single test was changed, only such test will be in the scope. This would skip the "dedicated shield test". But I guess it's not an issue, since in such PR nothing is touched with shield application mechanism, hence no need to run it. I believe that such a test will be excluded in other cases as long as it doesn't use any tags which are excluded in the optimization process https://github.com/zephyrproject-rtos/zephyr/blob/main/scripts/ci/tags.yaml

[tejlmand]

Nice improvement, only a minor nit observed.

I think this could be merged for benefit of all, and then work on a better solution for checking new shields.

afaik, shields in-tree are not checked to be valid on every PR, the ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/list_shields.py code only lists available shields, but not their content or whether they are valid.

@nordicjm do you have any pointers to where all the shields are actually tested to be valid that are influenced by this PR ?

[nordicjm]

afaik, shields in-tree are not checked to be valid on every PR, the ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/list_shields.py code only lists available shields, but not their content or whether they are valid.

they are checked, see this demonstration:

｢13 /tmp/aa/zephyr/samples/hello_world/_AA (xippartitionpr)｣$ git diff
diff --git a/boards/shields/mikroe_adc_click/shield.yml b/boards/shields/mikroe_adc_click/shield.yml
index 90109123009..5fbb830107f 100644
--- a/boards/shields/mikroe_adc_click/shield.yml
+++ b/boards/shields/mikroe_adc_click/shield.yml
@@ -1,3 +1,4 @@
+blah
 shield:
   name: mikroe_adc_click
   full_name: ADC Click
｢14 /tmp/aa/zephyr/samples/hello_world/_AA (xippartitionpr)｣$ cmake -GNinja -DBOARD=nrf52840dk/nrf52840 ..
Loading Zephyr default modules (Zephyr repository).
-- Application: /tmp/aa/zephyr/samples/hello_world
-- CMake version: 4.2.1
-- Found Python3: /usr/bin/python (found suitable version "3.13.11", minimum required is "3.10") found components: Interpreter
-- Cache files will be written to: /home/jamie/.cache/zephyr
-- Zephyr version: 4.3.99 (/tmp/aa/zephyr)
-- Found west (found suitable version "1.3.0", minimum required is "0.14.0")
-- Board: nrf52840dk, qualifiers: nrf52840
CMake Error at /tmp/aa/zephyr/cmake/modules/shields.cmake:70 (message):
  Error finding shields

  Error message: Traceback (most recent call last):

    File "/tmp/aa/zephyr/scripts/list_shields.py", line 136, in <module>
      dump_shields(find_shields(args))
                   ~~~~~~~~~~~~^^^^^^
    File "/tmp/aa/zephyr/scripts/list_shields.py", line 61, in find_shields
      for shields in find_shields_in(root):
                     ~~~~~~~~~~~~~~~^^^^^^
    File "/tmp/aa/zephyr/scripts/list_shields.py", line 81, in find_shields_in
      shield_data = yaml.load(f.read(), Loader=SafeLoader)
    File "/usr/lib/python3.13/site-packages/yaml/__init__.py", line 81, in load
      return loader.get_single_data()
             ~~~~~~~~~~~~~~~~~~~~~~^^
    File "/usr/lib/python3.13/site-packages/yaml/constructor.py", line 49, in get_single_data
      node = self.get_single_node()
    File "yaml/_yaml.pyx", line 674, in yaml._yaml.CParser.get_single_node
    File "yaml/_yaml.pyx", line 689, in yaml._yaml.CParser._compose_document
    File "yaml/_yaml.pyx", line 861, in yaml._yaml.CParser._parse_next_event

  yaml.scanner.ScannerError: mapping values are not allowed in this context

    in "<unicode string>", line 2, column 7

Call Stack (most recent call first):
  /tmp/aa/zephyr/cmake/modules/zephyr_default.cmake:129 (include)
  /tmp/aa/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
  /tmp/aa/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:117 (include_boilerplate)
  /home/jamie/LORA-NG/zephyr/share/zephyr-package/cmake/zephyr_package_search.cmake:118 (include)
  /home/jamie/LORA-NG/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:152 (check_zephyr_package)
  CMakeLists.txt:5 (find_package)

[tejlmand]

afaik, shields in-tree are not checked to be valid on every PR, the ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/list_shields.py code only lists available shields, but not their content or whether they are valid.

they are checked, see this demonstration:

but that only checks the shield.yml itself, not the content of the shield.
The shield itself, such as the dt overlay or Kconfig fragment could still be bogus.

That's what I mean by checking whether the shield itself is valid.

[nordicjm]

afaik, shields in-tree are not checked to be valid on every PR, the ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/list_shields.py code only lists available shields, but not their content or whether they are valid.

they are checked, see this demonstration:

but that only checks the shield.yml itself, not the content of the shield. The shield itself, such as the dt overlay or Kconfig fragment could still be bogus.

That's what I mean by checking whether the shield itself is valid.

Correct yes, it checks that the yaml file is valid, likewise with the snippets one, I don't think we should go from having a basic check to no check at all. Would be fine if there is a way to add a test that runs on every PR that tests them all instead, but no clue how to do that

[tejlmand]

I don't think we should go from having a basic check to no check at all.

and that's a fair point.
Personally I think the gain in this PR, plus a future real shield / snippets test-case would would have more benefit, compared to the time period in which the yml check is missing.

Especially as I'm actually more worried of the non-existing shield implementation check we have today, than a malformed yml.
But that's just different view points, which are both valid 🙂

[gudnimg]

I can drop the "draft" status if you want to consider the merge.

I unfortunately haven't found enough time to work on / look into the shield test coverage problem these past two weeks.

[gudnimg]

Resolved two open comments. And also synced the branch with main. (~1300 commits)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[gudnimg]

is a good idea, however is not acceptable in the current form. You would need to add a test which is always executed on every CI run that includes a shield so that all shields in-tree (or e.g. added by a PR) are checked to be valid, because this PR removes that check

@nordicjm I've added a new compliance check to try and start tackling this problem 3bc49c1. If I understand correctly, this does the equivalent validation/testing as already is done in main.

---

but that only checks the shield.yml itself, not the content of the shield.
The shield itself, such as the dt overlay or Kconfig fragment could still be bogus.

That's what I mean by checking whether the shield itself is valid.

@tejlmand One problem I'm running into here is how to guarantee a mapping from a shield to a compatible board. If we can make that work, we can automatically run a build where we can smoke test a shield where the CI runs west build --board <some name> --cmake-only -- -DSHIELD=<some other name>. I don't see any way to do that currently without blowing up the CI time and checking every board to see if one is compatible.

Idea: Force upstream shield.yml files to include at least one board/platform name which can be used to run validation?

I feel at least this problem is outside the scope of this PR... That is, validating more than current main branch.