nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

CI: Validate all defconfig files before running any builds

Open lupyuen opened this issue 1 year ago • 1 comments

TODO: This adds 15 mins to Every CI Job, might exceed our quota of GitHub Runner Minutes. Maybe we should trigger this in github.com/nuttxpr, whenever a PR is created / modified?

Summary

Currently, CI Build Jobs will validate the defconfig file just before compiling the NuttX Target (like rv-virt:nsh). This means that the Build Job might run for a while, before hitting a defconfig error and failing much later.

This PR updates the CI Workflow build.yml to validate all defconfig files before running any builds. This means that errors in the defconfig files will be flagged earlier. And the Build Job will terminate (with an error) before any build begins.

This behaviour is helpful for resolving CI Build Issues quickly. The code is derived from tools/testbuild.sh. The enhancement was suggested here: https://github.com/apache/nuttx/issues/14259

Impact

The CI Workflow build.yml will validate all defconfig files before running any builds. This might take up to 15 mins for bigger jobs, like arm-05.

If any errors are found in defconfig files: The Build Job will terminate (with an error) after all defconfig files have been validated, before any build begins.

The Updated CI Workflow shall be synced to nuttx-apps repo in the next PR.

Testing

We tested by creating an intentional error in a defconfig file:

If defconfig validation fails: The Build Job terminates (with an error) after all defconfig files have been validated, before any build begins https://github.com/lupyuen5/label-nuttx/actions/runs/11344293823/job/31548821624

Validating targets in tools/ci/testlist/x86_64-01.dat
./tools/refresh.sh --silent qemu-intel64/nsh
  Normalize qemu-intel64/nsh
8d7
< CONFIG_AAA=y
Saving the new configuration file
Error: qemu-intel64/nsh:1:1: error: qemu-intel64/nsh is configured incorrectly. To fix it, run "tools/refresh.sh qemu-intel64/nsh"
...
Error: tools/ci/testlist/x86_64-01.dat:1:1: error: Quitting, defconfig validation failed for tools/ci/testlist/x86_64-01.dat

Errors are specially formatted to display correctly in the GitHub Actions Job Summary:

Screenshot 2024-10-15 at 6 43 47 PM

If defconfig validation is successful: The Build Job continues to build the targets https://github.com/lupyuen5/label-nuttx/actions/runs/11361897141/job/31604515998

Validating targets in tools/ci/testlist/x86_64-01.dat
./tools/refresh.sh --silent qemu-intel64/nsh
  Normalize qemu-intel64/nsh
...
Configuration/Tool: qemu-intel64/nsh
  Building NuttX...

lupyuen avatar Oct 15 '24 10:10 lupyuen

[Experimental Bot, please feedback here]

NuttX PR Requirements Review

This PR appears to meet the NuttX requirements. Here's a breakdown:

Summary:

  • Clear need: Addresses slow CI builds by catching defconfig errors early (Issue #14259).
  • Functionality: Modifies CI workflow (build.yml) to validate defconfig files before building.
  • Implementation: Leverages tools/testbuild.sh for validation.

Impact:

  • New Feature: Yes, early defconfig validation in CI.
  • User Impact: No, CI-only change.
  • Build Impact: Yes, CI builds terminate early on defconfig errors.
  • Hardware Impact: No.
  • Documentation Impact: Potentially, if CI usage guidelines exist.
  • Security Impact: No.
  • Compatibility Impact: No.
  • Other: nuttx-apps repo needs syncing.

Testing:

  • Comprehensive: Tests both success and failure scenarios.
  • Platforms: Limited platform info provided, assuming sufficient coverage based on CI scope.
  • Logs: Clear logs demonstrating expected behavior.

Overall:

Well-structured PR addressing a valid issue. Minor suggestions:

  • Briefly mention platforms used for testing (Build Host/Target).
  • Consider adding a note to relevant documentation about the new CI behavior.

nuttxpr avatar Oct 15 '24 10:10 nuttxpr