From 7a795a9f087c3c203f1356b2f23c060098c19012 Mon Sep 17 00:00:00 2001 From: zhangning21 Date: Mon, 31 Aug 2026 20:39:08 +0800 Subject: [PATCH] ci: apply Depends-On dependencies to the memory report The memory report builds this pull request merged into master, together with the nuttx-apps default branch, and nothing else, so a companion or predecessor pull request it declares is absent. For a breaking change the target then fails to build and no report is produced at all, even though the Build workflow already tests the declared sources through Depends-On. Apply the declared dependencies before building, reusing the parser and the fetch/cherry-pick sequence that build.yml uses, and mapping each repository to its checkout exactly as build.yml does. A stacked nuttx dependency is no more optional than an apps one: a pull request that uses an API its predecessor introduces does not build without it. As build.yml does, read the description through the API rather than trusting the event payload, so that a manual re-run after editing a Depends-On line applies the current declaration instead of the one the run was created with. Unlike build.yml, a failed read stops the job rather than falling back to the payload: build.yml resolves this once and hands every target the same tree, while this job runs per target, so a fallback could leave targets on different declarations while their results are filed under one SHA. A declared dependency that cannot be applied fails the job, and so does a missing parser, a parser crash, or a status this step does not recognise: each of those means the declaration was never evaluated, and continuing would measure a combination nobody asked for. build.yml fails Fetch-Source on the same conditions, and no other step in this job carries continue-on-error, so falling back silently would be inconsistent with both. A declaration that parses to nothing valid only warns, again matching build.yml. Forward the parser's warnings too. --print-state prints only the state, so an entry the parser drops -- an unsupported repository, say -- would otherwise leave no trace here at all, although build.yml annotates it, and the source set named below would be silently incomplete. The report is filed under the pull request head SHA rather than the SHA of the tree that was built, so the measurement cannot be reproduced from that SHA alone and cannot be split per dependency. That limits provenance, not the measurement: a combined result is what the declaration asks for, and a regression that only appears in combination is still a regression. Name the whole source set in the step summary so the reader knows which heads went into the number. Note in the parser that the --print-state output is a parsed contract; the edit gate that used to be its only caller is gone. Update the CI documentation to match. Its Pull Request Dependencies section attributes dependency application to build.yml's Fetch-Source job alone, so after this change it would read as if the memory report measured the normal source selection. Cross-reference the two sections rather than restating the rules, which stay shared. Signed-off-by: zhangning21 --- .github/scripts/depends_on.py | 6 +- .github/workflows/membrowse-report.yml | 148 +++++++++++++++++++++++++ Documentation/testing/nuttx-ci.rst | 9 ++ 3 files changed, 162 insertions(+), 1 deletion(-) diff --git a/.github/scripts/depends_on.py b/.github/scripts/depends_on.py index 14a756634f51b..eebb8b4736b9a 100644 --- a/.github/scripts/depends_on.py +++ b/.github/scripts/depends_on.py @@ -23,8 +23,12 @@ CLI: python3 depends_on.py # print result JSON - python3 depends_on.py --print-state # print state used by the edit gate + python3 depends_on.py --print-state # status line, then one ref per line python3 depends_on.py --github-output # write workflow outputs and report + +The --print-state output is a contract rather than a debugging aid: the first +line is the status and every remaining line is a reference in declaration +order. It is parsed by membrowse-report.yml and pinned by test_depends_on.py. """ from __future__ import annotations diff --git a/.github/workflows/membrowse-report.yml b/.github/workflows/membrowse-report.yml index 66b9fdfd75e42..07f35e2adf1f0 100644 --- a/.github/workflows/membrowse-report.yml +++ b/.github/workflows/membrowse-report.yml @@ -7,8 +7,10 @@ on: - master - "releases/*" +# pull-requests read: needed to re-read edited Depends-On declarations on re-runs. permissions: contents: read + pull-requests: read # Per-PR group so superseded PR pushes cancel; per-SHA on push so master # commits never share a group. A shared refs/heads/master group lets a burst @@ -187,6 +189,152 @@ jobs: - name: After CLEAN-UP Disk Space run: df -h + # Apply declared nuttx/apps dependencies before measuring, matching + # build.yml's checkout mapping and cherry-pick order. API or parser + # failures and parsed dependencies that cannot be applied are fatal, so a + # report is never taken from the wrong source set. Keep this after the + # disk cleanup: applying a dependency deepens a checkout. + - name: Apply depends-on PRs + if: ${{ github.event_name == 'pull_request' && github.base_ref == 'master' }} + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_BODY: ${{ github.event.pull_request.body }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -uo pipefail + + # Re-read the current body so a re-run picks up an edited Depends-On + # line. Do not fall back to the event payload: unlike build.yml, which + # resolves this once for every target, each matrix leg resolves + # independently and could otherwise use a different declaration. + if ! PR_BODY="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.body // ""')"; then + echo "::error::Could not read the PR description." + exit 1 + fi + + # A missing parser, a crash, or an unknown status all leave the + # declaration unevaluated. + PARSER=sources/nuttx/.github/scripts/depends_on.py + if [ ! -f "$PARSER" ]; then + echo "::error::${PARSER} not found." + exit 1 + fi + + STATE="${RUNNER_TEMP:-/tmp}/depends-on-state.txt" + if ! python3 "$PARSER" --print-state > "$STATE"; then + echo "::error::Could not parse the depends-on declarations." + exit 1 + fi + + # --print-state omits warnings. Run --github-output again with its + # outputs and report discarded so dropped entries remain visible as + # workflow annotations. + if ! WARNINGS="$(GITHUB_OUTPUT=/dev/null REPORT_PATH= python3 "$PARSER" --github-output)"; then + echo "::error::Could not re-read the depends-on warnings." + exit 1 + fi + printf '%s\n' "$WARNINGS" | grep '^::warning::' || true + + STATUS=$(head -n 1 "$STATE") + case "$STATUS" in + ok) ;; + none) + echo "No Depends-On declaration; building against the default sources." + exit 0 ;; + invalid) + # The parser warning forwarded above already explains this. + echo "No valid dependency parsed; building against the default sources." + exit 0 ;; + *) + echo "::error::Unexpected depends-on parser status: ${STATUS}" + exit 1 ;; + esac + + git config --global user.email "actions@github.com" + git config --global user.name "github-actions" + + # ok must carry at least one reference; an unreadable or empty list + # would skip the loop and report the default sources as though the + # declaration had been applied. Keep the loop in this shell, not a + # pipeline, so a dependency failure exits the step. + if ! tail -n +2 "$STATE" > "${STATE}.refs" || [ ! -s "${STATE}.refs" ]; then + echo "::error::Could not read the parsed dependency list." + exit 1 + fi + # The report is keyed by the pull request head SHA, which names + # neither checkout: nuttx is the pull request merged into its base, + # and apps is an unpinned default branch the event never mentions. + # Record both in the summary, before anything is applied, so the + # source set is visible where the report is read. + NUTTX_CHECKOUT_SHA=$(git -C sources/nuttx rev-parse HEAD) + APPS_CHECKOUT_SHA=$(git -C sources/apps rev-parse HEAD) + { + echo "### Depends-On source set" + echo + echo "Measured under this pull request's head SHA, together with:" + echo "- \`apache/nuttx\` checkout @ \`${NUTTX_CHECKOUT_SHA}\`" + echo "- \`apache/nuttx-apps\` checkout @ \`${APPS_CHECKOUT_SHA}\`" + } >> "${GITHUB_STEP_SUMMARY:-/dev/null}" + + while read -r DEP; do + [ -n "$DEP" ] || continue + DEP_REPO=${DEP%/pull/*} + DEP_NUM=${DEP##*/} + + # Keep the repository mapping aligned with build.yml. + case "$DEP_REPO" in + "apache/nuttx") REPO_PATH=sources/nuttx ;; + "apache/nuttx-apps") REPO_PATH=sources/apps ;; + *) + echo "::error::Unsupported dependency repository: ${DEP_REPO}" + exit 1 ;; + esac + + echo "Applying dependency ${DEP}" + # Deepening is best effort; the common-base and rev-list checks + # below still fail closed. Say it happened, so a missing common + # base is not mistaken for an unrelated dependency. + if [ -f "${REPO_PATH}/.git/shallow" ] \ + && ! git -C "$REPO_PATH" fetch --unshallow origin; then + echo "::warning::Could not deepen ${REPO_PATH}; a missing common base below may follow from that rather than from ${DEP}." + fi + if ! git -C "$REPO_PATH" fetch origin "pull/${DEP_NUM}/head:dep-${DEP_NUM}"; then + echo "::error::Could not fetch ${DEP} (the PR may not exist)." + exit 1 + fi + + # Reject unrelated histories before computing HEAD..dep, which + # would otherwise list every dependency commit. + if [ -z "$(git -C "$REPO_PATH" merge-base "dep-${DEP_NUM}" HEAD || true)" ]; then + echo "::error::Could not find a common base with ${DEP}." + exit 1 + fi + if ! COMMITS=$(git -C "$REPO_PATH" rev-list --reverse "HEAD..dep-${DEP_NUM}"); then + echo "::error::Could not list the commits of ${DEP}." + exit 1 + fi + + DEP_SHA=$(git -C "$REPO_PATH" rev-parse "dep-${DEP_NUM}") + if [ -z "$COMMITS" ]; then + echo "Dependency ${DEP} is already included." + echo "- \`${DEP}\` @ \`${DEP_SHA}\` (already in the checkout)" \ + >> "${GITHUB_STEP_SUMMARY:-/dev/null}" + continue + fi + + # shellcheck disable=SC2086 + if ! git -C "$REPO_PATH" cherry-pick $COMMITS; then + echo "::error::Could not cherry-pick ${DEP}." + echo "::error::If your pull request contains merge commits, rebase instead of merging." + git -C "$REPO_PATH" cherry-pick --abort || true + exit 1 + fi + echo "Applied ${DEP} @ ${DEP_SHA}" + echo "- \`${DEP}\` @ \`${DEP_SHA}\`" \ + >> "${GITHUB_STEP_SUMMARY:-/dev/null}" + done < "${STATE}.refs" + - name: Docker Login uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: diff --git a/Documentation/testing/nuttx-ci.rst b/Documentation/testing/nuttx-ci.rst index 482c7c68f8ced..1fb14a9d29788 100644 --- a/Documentation/testing/nuttx-ci.rst +++ b/Documentation/testing/nuttx-ci.rst @@ -154,6 +154,9 @@ commit list cannot be determined, or it causes a cherry-pick conflict, ``Fetch-Source`` fails instead of silently testing without the requested dependency. +The memory footprint workflow applies the same declarations independently, +using the same parser and apply sequence (see `Memory Footprint Tracking`_). + When a valid dependency report is available, the follow-up comment reports one of three outcomes: @@ -249,3 +252,9 @@ The integration consists of: * the set of tracked targets, configured in ``.github/membrowse-targets.json`` * the ``membrowse-*.yml`` workflows under ``.github/workflows/`` that drive it + +The memory report applies the same ``Depends-On:`` declarations as the Build +workflow (see `Pull Request Dependencies`_), so a pull request that only builds +on top of another one is measured against a tree that compiles. The declaration +rules and the ``master``-only gate are the same. A dependency that cannot be +fetched or applied fails the job in both workflows.