Skip to content

ci: lint all OS targets on one Linux builder - #1526

Merged
zackees merged 3 commits into
mainfrom
fix/dylint-linux-targets-final
Sep 27, 2026
Merged

zackees merged 3 commits into
mainfrom
fix/dylint-linux-targets-final

Conversation

@zackees

@zackees zackees commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Change

Keep ordinary PR Dylint on one Linux runner, and make the existing ci-full Dylint call and manual dispatch check Linux, Windows MSVC, and macOS targets from that same Linux builder. The full run prepares the catalogued prebuilt driver once, installs the pinned nightly's cross-target rust-std, runs all 27 custom lints over the Linux workspace, then checks the five crates named by the platform-boundary baseline for Windows/macOS. Each target's actual Dylint observations are compared with the source ledger. UI fixtures run when dylints/ changes, without rebuilding their separate test-profile trees on unrelated PRs.

The compiled lint-library tree uses an actions/cache key based on dylints/** and the pinned nightly, so workflow-only and application-only commits can reuse it across source SHAs. A short/qualified nightly alias lets Soldr and the cache share one directory. The large target-specific workspace tree is outside this cache.

This keeps the routine gate fast after #1524 made it Linux-host only, while restoring an explicit full-platform Dylint path. zackees/soldr#3426 tracks automatic target rust-std materialization; fbuild#1527 tracks removal of the temporary workflow install step after that Soldr fix ships.

Evidence

  • Full all-workspace target run: all three targets passed; target step 24m43s cold. setup-soldr skipped Dylint output save because its path was short-nightly while Soldr's was host-qualified.
  • Full baseline-scoped target run: all three targets passed; target step 20m12s cold. The content-keyed lint-library cache saved successfully under fbuild-dylint-libraries-v1-linux-nightly-2026-05-28-....
  • Local Docker targeted ban_manual_slash_normalize Dylint checks passed for Windows MSVC and macOS; full soldr cargo check --workspace --all-targets passed for Windows GNU, Windows MSVC, and macOS. Workflow contract tests: 11 passed.
  • The latest PR run is checking cross-SHA cache restore and the routine host-only path.

Summary by CodeRabbit

  • Chores
    • Automated checks now cover Linux on pull requests, with full validation also covering Windows and macOS for designated runs.
    • Full checks now validate platform-specific lint results and boundary records, and report failures when either check does not pass. Unchanged lint libraries are skipped during pull request checks.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: FastLED/fbuild/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 24dc5702-ab99-44a9-91b3-d760331d53a9

📥 Commits

Reviewing files that changed from the base of the PR and between 06c1566 and cd10fbf.

📒 Files selected for processing (2)
  • .github/workflows/dylint.yml
  • ci/test_fractional_workflows.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Dylint workflow runs Linux checks and adds Windows and macOS checks for full runs. It configures compiled-library caching and skips library UI tests when Dylint sources are unchanged. CI tests assert the workflow settings.

Changes

Dylint workflow

Layer / File(s) Summary
Workflow setup and compiled-library cache
.github/workflows/dylint.yml
The workflow configures cache settings, conditionally installs cross-target standard libraries, skips library UI tests when dylints is unchanged, reconciles library paths, and saves compiled libraries.
Cross-target lint and boundary validation
.github/workflows/dylint.yml, ci/test_fractional_workflows.py
The workflow runs Linux lint checks and adds Windows and macOS checks for full runs. It validates each target’s boundary ledger and normalizes the success marker. The CI test checks target, cache, and source-change settings.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant BoundaryBaseline
  participant Dylint
  participant SuccessMarker
  GitHubActions->>BoundaryBaseline: Read cross-target package selections
  GitHubActions->>Dylint: Run workspace lint for enabled targets
  Dylint-->>GitHubActions: Return lint results
  GitHubActions->>BoundaryBaseline: Validate each target's observed ledger
  GitHubActions->>SuccessMarker: Verify and normalize after successful checks
Loading

Merge Risk: ⚪ Minimal · up to cd10f

No actionable issue is established for this change; it is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to cd10f

The workflow adds useful Windows and macOS lint coverage while retaining a read-only repository token and a fail-closed gate. The cross-target checks cover a selected set of crates rather than every crate, and compiled lint libraries are reused across commits, so the limits of those controls merit review. No introduced security vulnerability was established.

Retained concerns

  • Low · security · inferred: The new cross-target Dylint pass selects packages from existing platform-boundary baseline rows. Target-gated code in a crate with no such row is not directly included in that pass, so the pass cannot by itself guarantee cross-target coverage for all custom lints.
Security review details

Security Blast Radius

  • inferred — The new cache carries executable lint libraries between CI commits, while the limited cross-target selection affects the assurance provided for code outside the baseline-selected crates. The evidence does not show that either path grants repository write access.

Trust Boundaries and Controls

  • observed — The job checks out the requested ref, uses a read-only repository token, and requires successful lint and ledger comparison before normalizing its success marker. The available source does not establish the external cache service’s access rules or the tool’s restored-binary validation.

Resilience and Maintainability Implications

  • observed — Per-target failures accumulate rather than silently skipping subsequent targets, and the final gate fails unless both policy and lint jobs succeed.

Hardening Proposals

  • proposed — If full cross-target custom-lint coverage is the intended guarantee, select packages from target-gated source coverage rather than existing platform-boundary findings, or explicitly state the narrower guarantee.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: running lint checks for all supported OS targets on one Linux builder.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@zackees
zackees force-pushed the fix/dylint-linux-targets-final branch from 4da8dc1 to d2fe3f6 Compare September 27, 2026 21:18
@zackees
zackees merged commit ef10ced into main Sep 27, 2026
17 checks passed
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant