Skip to content

ci: Support pull request dependencies via Depends-On. - #3690

Open
zhangning21 wants to merge 1 commit into
apache:masterfrom
zhangning21:feature/pull-request-dependencies
Open

ci: Support pull request dependencies via Depends-On.#3690
zhangning21 wants to merge 1 commit into
apache:masterfrom
zhangning21:feature/pull-request-dependencies

Conversation

@zhangning21

Copy link
Copy Markdown
Contributor

Summary

Add the nuttx-apps side of the pull-request dependency support already merged in apache/nuttx#19075. This keeps the two repositories' Build workflows consistent and lets an apps PR test exact same- or cross-repository companion PR heads before either change is merged.

Why this is needed

NuttX and nuttx-apps are built together. For a normal PR, Fetch-Source checks out the target branch from the changed repository and the corresponding branch from the other repository. A feature that must change both repositories can therefore fail in both PRs because neither target branch contains the companion change yet. This feature lets CI test the exact combined changes before either PR is merged. It also supports a PR depending on another PR in the same repository.

Implementation

  • Add the same standard-library depends_on.py parser and 40 regression tests used by NuttX.
  • Gate heavy CI on dependency-changing PR-description edits.
  • Fetch and cherry-pick ordered dependency PR commits once in Fetch-Source, before the existing matrix.
  • Upload a structured report from the read-only Build workflow.
  • Validate and render that untrusted report in a default-branch workflow_run with narrowly scoped actions: read and pull-requests: write permissions.

The trusted depends-on-comment.yml workflow becomes active after it is merged into the repository's default branch, because GitHub only triggers a newly added workflow_run workflow when that workflow file exists on the default branch.

The follow-up comment reports one of three outcomes:

  • ok: each dependency PR and its fetched head SHA, abbreviated in the comment;
  • invalid: a marker was found but no valid dependency could be parsed, so no dependency is applied and CI continues with normal source selection;
  • failed: a valid dependency could not be applied, with a fixed diagnostic reason. This causes Fetch-Source to fail.

The accepted declaration forms, result comments, release behavior, and limitations are already documented in NuttX's central CI documentation by apache/nuttx#19075.

Usage

A PR targeting master may declare one dependency per line:

Depends-On: https://github.com/apache/nuttx/pull/1234
Depends-On: https://github.com/apache/nuttx-apps/pull/5678

or one inline list:

Depends-On: [apache/nuttx/pull/1234 apache/nuttx-apps/pull/5678]

The marker is case-insensitive. Duplicate references are applied once in first-seen order. Invalid declarations do not apply a dependency and do not fail Build; a valid dependency that cannot be fetched or cherry-picked fails Fetch-Source.

Impact and security

  • PRs without Depends-On: retain the existing source selection and build-matrix behavior; every PR gains only the short Changes gate, and Build additionally listens for description edits.
  • Release/backport PRs ignore declarations and retain matching-release-branch selection.
  • Build remains contents: read; it does not receive write permission or secrets.
  • The parser does not execute PR text and uses no eval, shell parsing pipeline, or third-party package.
  • The write-capable workflow_run does not check out or execute fork code. Before commenting it validates the report schema, enforces the repository allow-list and dependency-number constraints, rejects duplicate entries, requires full dependency head SHAs for ok results, and verifies run/current-head binding.
  • Repository policy confirmation: The trusted workflow requests pull-requests: write only to post the validated result comment, matching the permission model already merged and exercised in apache/nuttx. Please confirm that the same repository policy is enabled for apache/nuttx-apps; no other write permission is requested.
  • No build target is added or duplicated. Parsing and cherry-picking occur once in Fetch-Source.
  • An unrelated description edit runs only the short Changes gate and skips Fetch-Source and the matrix.
  • No runtime, hardware, ABI, target, or application behavior changes.

Testing

Local validation:

  • Black 24.8.0 and Flake8 7.1.1: passed.
  • py_compile: passed.
  • Parser unit tests: 40/40 passed.
  • Parser CLI, workflow YAML/permissions/repository assertions, and git diff --check: passed.
  • Parser and tests match the merged NuttX versions byte-for-byte.
  • GitHub Lint passed at the pure feature commit (run 30734995071) and the E2E commit (run 30735668914).

Real GitHub Actions E2E was run in the public zhn-test/nuttx-apps mirror with fresh dependency PRs and a trusted default-branch comment workflow. The staging workflow maps apache/nuttx, apache/nuttx-apps, and master to zhn-test/nuttx, zhn-test/nuttx-apps, and a master-equivalent test base; the feature logic is otherwise the same. The four E2E-only matrix gates and all test repository/base substitutions are excluded from the upstream commit:

Case Result Evidence
Single cross-repository dependency Applied NuttX PR and reported exact SHA PR #8 · run · comment
Ordered cross- and same-repository dependencies Both applied in declaration order with exact SHAs PR #9 · run · comment
Invalid non-GitHub host invalid; Build continued and warning was posted PR #10 · run · comment
Non-existent dependency Fetch-Source failed and a fixed-reason comment was posted PR #11 · run · comment
No dependency Apply/report skipped, no comment, selected x86_64-01 build passed final single-commit PR #17 · successful run
Prose, prefixed marker, fenced code Ignored; no report/comment PR #12 · run
Unsupported bullet continuation invalid; warning posted PR #13 · run · comment
Release/backport base Declaration ignored; no report/comment PR #15 · run
Dependency-changing body edit Gate reran Fetch/apply and posted result PR #14 · run · comment
Unrelated body edit Only Changes ran; Fetch and downstream jobs skipped PR #8 · run
Per-Build comment history An empty commit created a new initiating head; new comment created and prior comment preserved PR #9 · new run · new comment
Edited gate with trusted base parser Base already contained depends_on.py; log confirms the base parser was used before Fetch/apply/comment succeeded PR #16 · run · Changes job · comment

The staging scenario branches skipped the heavy matrix after Fetch-Source to avoid duplicating runner cost. Before that E2E-only gate was added, feature run 30734995149 at the pure feature commit verified Changes, Fetch-Source, and all four architecture-selection jobs successfully. Its overall result is cancelled because it was superseded after those jobs passed to stop the remaining heavy matrix; it is not presented as a complete matrix pass. The upstream PR's normal CI remains responsible for the full apps build matrix.

Intentional limitations

  • Dependency PR state and target branch are not queried through the GitHub API.
  • Updating a dependency PR does not automatically retrigger the initiating PR.
  • A result comment reflects the read-only Build report and does not independently prove the dependency PR's own checks passed.
  • This does not automate synchronized merging.
  • Cherry-pick conflicts fail Build; authors must update the dependency rather than rely on CI conflict resolution.

Allow pull requests targeting master to declare same- and
cross-repository dependencies. Parse declarations with a tested Python
helper, apply exact dependency commits before the existing build matrix,
and rerun heavy CI only when an edited description changes the dependency
state.

Keep fork builds read-only and use a trusted workflow_run to validate
artifacts and post per-build dependency results. Keep the apps workflow
consistent with the implementation already merged in apache/nuttx.

Assisted-by: Kiro:gpt-5.6-sol
Signed-off-by: zhangning21 <zhangning21@xiaomi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants