Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: Validate all defconfig files before running any builds #14317

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lupyuen
Copy link
Member

@lupyuen lupyuen commented Oct 15, 2024

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: #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...

@github-actions github-actions bot added Area: CI Size: S The size of the change in this PR is small labels Oct 15, 2024
@lupyuen lupyuen linked an issue Oct 15, 2024 that may be closed by this pull request
1 task
@nuttxpr
Copy link

nuttxpr commented Oct 15, 2024

[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 [FEATURE] Revising the Arm32 Targets for CI Build #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.

@lupyuen lupyuen force-pushed the label14 branch 3 times, most recently from e36038a to 2eba9f7 Compare October 15, 2024 19:03
@lupyuen lupyuen marked this pull request as ready for review October 15, 2024 19:22
@lupyuen lupyuen marked this pull request as draft October 15, 2024 19:22
@lupyuen lupyuen force-pushed the label14 branch 2 times, most recently from 41f0146 to ce10548 Compare October 15, 2024 22:23
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: apache#14259
@lupyuen lupyuen marked this pull request as ready for review October 16, 2024 08:49
@lupyuen lupyuen marked this pull request as draft October 16, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CI Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Revising the Arm32 Targets for CI Build
2 participants