nf-interpreter icon indicating copy to clipboard operation
nf-interpreter copied to clipboard

[CI-CD] Create weekly build for devcontainer targets

Open networkfusion opened this issue 3 months ago β€’ 5 comments

Description

Add weekly builds for selected targets within a devcontainer and using MinSizeRel (and could be expanded to include add ALL targets (including those in contrib repo)).. This is different from smoketests which target debug releases (and should be kept minimal) and only runs weekly.

Motivation and Context

  • Improves testing.
  • Allows forks to build without requiring Azure CI (github workflows allow the easiest point of entry).

How Has This Been Tested?

Screenshots

Types of changes

  • [ ] Improvement (non-breaking change that improves a feature, code or algorithm)
  • [ ] Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • [ ] New feature (non-breaking change which adds functionality to code)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [x] Config and build (change in the configuration and build system, has no impact on code or features)
  • [x] Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • [ ] Dependencies/declarations (update dependencies or assembly declarations and changes associated, has no impact on code or features)
  • [ ] Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • [ ] My code follows the code style of this project (only if there are changes in source code).
  • [ ] My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • [ ] I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • [ ] I have read the CONTRIBUTING document.
  • [ ] I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

Summary by CodeRabbit

  • Chores
    • Expanded CI to cover an additional firmware target and added a weekly scheduled target-build workflow for broader automated build coverage.
    • Improved build orchestration: conditional handling for optional radio-frequency settings and safer container/template selection.
    • Streamlined setup by removing an unnecessary submodule update step.
    • Implemented explicit LittleFS version management (default v2.9.3) and clarified source/version messaging; minor messaging/formatting tweaks.

networkfusion avatar Nov 05 '25 18:11 networkfusion

Walkthrough

Updates CI workflows: modifies devcontainer smoketest matrix and guards RADIO_FREQUENCY substitution; adds a new weekly devcontainer-targets build workflow with a broad matrix and per-target devcontainer builds. Adjusts CMake: changes LITTLEFS package name and adds LITTLEFS version handling and messaging; tweaks FatFS source formatting/URL.

Changes

Cohort / File(s) Summary
Smoketest workflow
.github/workflows/devcontainer-smoketest.yaml
Adds matrix entry { target: ORGPAL_PALTHREE, build-type: Debug, container: ChibiOS }; removes explicit git submodule update step; makes RADIO_FREQUENCY substitution conditional (only runs sed when matrix.radio-freq is non-empty).
Weekly devcontainer-target builds
.github/workflows/devcontainer-targets-weekly-build.yaml
Adds new scheduled workflow ("Weekly [devcontainer] target builds") triggered on workflow-file PRs and weekly Sunday 03:00 UTC; defines a large matrix (targets, build-types, containers, optional radio frequencies), installs submodules, frees disk, updates templates and devcontainer.json (sets Dockerfile.${container}.SRC), conditionally substitutes RADIO_FREQUENCY, and runs devcontainers/ci with CMake presets to build each matrix entry.
CMake package discovery
CMake/binutils.ChibiOS.cmake
Replaces find_package(littlefs ...) with find_package(LITTLEFS ...) in nf_add_platform_packages.
LittleFS versioning & FatFS messaging
targets/ChibiOS/CMakeLists.txt
Adds LITTLEFS_VERSION_TAG / LITTLEFS_VERSION_EMPTY handling (defaults to v2.9.3 when unspecified), updates littlefs messages to include version tag and source formatting, adjusts FatFS message formatting and Git URL to include .git.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant GH as GitHub Actions
  participant Repo as Repository
  participant Runner as Job Runner
  participant CI as devcontainers/ci

  Note over GH,Repo: Trigger = workflow-file PR or weekly schedule
  GH->>Repo: checkout (with submodules)
  GH->>Runner: prepare environment (free disk)
  Runner->>Repo: apply template updates & set `devcontainer.json` Dockerfile.${container}.SRC
  alt matrix.radio-freq non-empty
    Runner->>Repo: substitute RADIO_FREQUENCY in templates (sed)
  else
    Runner->>Repo: skip RADIO_FREQUENCY substitution
  end
  Runner->>CI: invoke devcontainers/ci per matrix entry (CMake preset + build)
  CI-->>GH: report build results

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to:
    • Matrix definitions, variable mappings, and fail-fast behavior in .github/workflows/devcontainer-targets-weekly-build.yaml.
    • Correct conditional handling of matrix.radio-freq and removal of the submodule update in .github/workflows/devcontainer-smoketest.yaml.
    • Template modifications and devcontainer.json Dockerfile.${container}.SRC substitutions.
    • CMake find_package(LITTLEFS ...) usage vs. installed package names/configs and the LITTLEFS version-default logic in targets/ChibiOS/CMakeLists.txt.

Pre-merge checks and finishing touches

βœ… Passed checks (3 passed)
Check name Status Explanation
Title check βœ… Passed The title '[CI-CD] Create weekly build for devcontainer targets' directly and clearly summarizes the main objective of the PR, which is adding a new weekly build workflow for devcontainer targets.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • [ ] πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch dev-container-nightly

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 05 '25 18:11 coderabbitai[bot]

@coderabbitai the nitpick should now be resolved.

networkfusion avatar Nov 07 '25 16:11 networkfusion

[!TIP] For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch all review comments from coderabbitai on PR #3232
gh pr view 3232 --json comments,reviews --jq '.reviews[] | select(.author.login == "coderabbitai[bot]") | .body' | head -50

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Also fetch review thread comments
gh api repos/nanoframework/nf-interpreter/pulls/3232/comments --jq '.[] | select(.user.login == "coderabbitai[bot]") | {body: .body, path: .path, line: .line, created_at: .created_at}' | head -100

Length of output: 14118


🏁 Script executed:

#!/bin/bash
# Get recent commits in the PR to see what was changed
gh pr view 3232 --json commits --jq '.commits[-3:] | .[] | {message: .messageHeadline, date: .committedDate}'

Length of output: 272


🏁 Script executed:

#!/bin/bash
# Verify the conditional check for radio-frequency is now in place
cat .github/workflows/devcontainer-targets-weekly-build.yaml | sed -n '65,85p'

Length of output: 933


@networkfusion, perfect! βœ… I can confirm the nitpick has been resolved. The conditional check is now properly in place:

if [[ -n "${{ matrix.radio-freq }}" ]]; then
  sed -i -- 's|"RADIO_FREQUENCY": "CHANGE_ME_TO_A_VALID_VALUE_868_OR_915"|"RADIO_FREQUENCY": "${{ matrix.radio-freq }}"|g' user-prefs.json
fi

This ensures that the RADIO_FREQUENCY substitution only occurs for TI targets that define the radio-freq matrix field, while non-TI targets preserve the original placeholder. Great fix! πŸŽ‰


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

coderabbitai[bot] avatar Nov 07 '25 16:11 coderabbitai[bot]

@coderabbitai re-review

networkfusion avatar Nov 07 '25 17:11 networkfusion

@networkfusion, I'll review the changes in the PR now.

βœ… Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar Nov 07 '25 17:11 coderabbitai[bot]