Unverified Commit 80e3aa39 authored by Wolfgang Walther's avatar Wolfgang Walther Committed by GitHub
Browse files

workflows/check-cherry-picks: post review comments (#412068)

parents 21bfb6cd 856792f9
Loading
Loading
Loading
Loading
+103 −2
Original line number Diff line number Diff line
@@ -10,7 +10,8 @@ on:
      - 'staging-**'
      - '!staging-next'

permissions: {}
permissions:
  pull-requests: write

jobs:
  check:
@@ -24,8 +25,108 @@ jobs:
          path: trusted

      - name: Check cherry-picks
        id: check
        continue-on-error: true
        env:
          BASE_SHA: ${{ github.event.pull_request.base.sha }}
          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
        run: |
          ./trusted/ci/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA"
          ./trusted/ci/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA" checked-cherry-picks.md

      - name: Prepare review
        if: steps.check.outcome == 'failure'
        uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
        with:
          script: |
            const { readFile, writeFile } = require('node:fs/promises')

            const job_url = (await github.rest.actions.listJobsForWorkflowRun({
              owner: context.repo.owner,
              repo: context.repo.repo,
              run_id: context.runId
            })).data.jobs[0].html_url + '?pr=' + context.payload.pull_request.number

            const header = await readFile('trusted/ci/check-cherry-picks.md')
            const body = await readFile('checked-cherry-picks.md')
            const footer =
              `\n_Hint: The full diffs are also available in the [runner logs](${job_url}) with slightly better highlighting._`

            const review = header + body + footer
            await writeFile('review.md', review)
            core.summary.addRaw(review)
            core.summary.write()

      - name: Request changes
        if: ${{ github.event_name == 'pull_request_target' && steps.check.outcome == 'failure' }}
        uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
        with:
          script: |
            const { readFile } = require('node:fs/promises')
            const body = await readFile('review.md', 'utf-8')

            const pendingReview = (await github.paginate(github.rest.pulls.listReviews, {
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: context.payload.pull_request.number
            })).find(review =>
              review.user.login == 'github-actions[bot]' && (
                // If a review is still pending, we can just update this instead
                // of posting a new one.
                review.state == 'CHANGES_REQUESTED' ||
                // No need to post a new review, if an older one with the exact
                // same content had already been dismissed.
                review.body == body
              )
            )

            // Either of those two requests could fail for very long comments. This can only happen
            // with multiple commits all hitting the truncation limit for the diff. If you ever hit
            // this case, consider just splitting up those commits into multiple PRs.
            if (pendingReview) {
              await github.rest.pulls.updateReview({
                owner: context.repo.owner,
                repo: context.repo.repo,
                pull_number: context.payload.pull_request.number,
                review_id: pendingReview.id,
                body
              })
            } else {
              await github.rest.pulls.createReview({
                owner: context.repo.owner,
                repo: context.repo.repo,
                pull_number: context.payload.pull_request.number,
                event: 'REQUEST_CHANGES',
                body
              })
            }

      - name: Dismiss old reviews
        if: ${{ github.event_name == 'pull_request_target' && steps.check.outcome == 'success' }}
        uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
        with:
          script: |
            await Promise.all(
              (await github.paginate(github.rest.pulls.listReviews, {
                owner: context.repo.owner,
                repo: context.repo.repo,
                pull_number: context.payload.pull_request.number
              })).filter(review =>
                review.user.login == 'github-actions[bot]' &&
                review.state == 'CHANGES_REQUESTED'
              ).map(async (review) => {
                await github.rest.pulls.dismissReview({
                  owner: context.repo.owner,
                  repo: context.repo.repo,
                  pull_number: context.payload.pull_request.number,
                  review_id: review.id,
                  message: 'All cherry-picks are good now, thank you!'
                })
                await github.graphql(`mutation($node_id:ID!) {
                  minimizeComment(input: {
                    classifier: RESOLVED,
                    subjectId: $node_id
                  })
                  { clientMutationId }
                }`, { node_id: review.node_id })
              })
            )
+30 −0
Original line number Diff line number Diff line
name: Dismissed Review

on:
  pull_request_review:
    types: [dismissed]

permissions:
  pull-requests: write

jobs:
  # The check-cherry-picks workflow creates review comments,
  # that should sometimes be manually dismissed.
  # When a CI-generated review is dismissed, this job automatically
  # minimizes it, to prevent it from cluttering the PR.
  minimize:
    name: Minimize as resolved
    if: github.event.review.user.login == 'github-actions[bot]'
    runs-on: ubuntu-24.04-arm
    steps:
      - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
        with:
          script: |
            await github.graphql(`mutation($node_id:ID!) {
              minimizeComment(input: {
                classifier: RESOLVED,
                subjectId: $node_id
              })
              { clientMutationId }
            }`, { node_id: context.payload.review.node_id })
+7 −0
Original line number Diff line number Diff line
This report is automatically generated by the `check-cherry-picks` CI workflow.

Some of the commits in this PR have not been cherry-picked exactly and require the author's and reviewer's attention.

Please make sure to follow the [backporting guidelines](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#how-to-backport-pull-requests) and cherry-pick with the `-x` flag. This requires changes to go to the unstable branches (`master` / `staging`) first, before backporting them.

Occasionally, it is not possible to cherry-pick exactly the same patch. This most frequently happens when resolving merge conflicts while cherry-picking or when updating minor versions of packages which have already advanced to the next major on unstable. If you need to merge this PR despite the warnings, please [dismiss](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/dismissing-a-pull-request-review) this review.
+73 −37
Original line number Diff line number Diff line
#!/usr/bin/env bash
# Find alleged cherry-picks

set -eo pipefail
set -euo pipefail

if [ $# != "2" ] ; then
  echo "usage: check-cherry-picks.sh base_rev head_rev"
if [[ $# != "2" && $# != "3" ]] ; then
  echo "usage: check-cherry-picks.sh base_rev head_rev [markdown_file]"
  exit 2
fi

markdown_file="$(realpath ${3:-/dev/null})"
[ -v 3 ] && rm -f "$markdown_file"

# Make sure we are inside the nixpkgs repo, even when called from outside
cd "$(dirname "${BASH_SOURCE[0]}")"

@@ -19,11 +22,43 @@ remote="$(git remote -v | grep -i 'NixOS/nixpkgs' | head -n1 | cut -f1 || true)"

commits="$(git rev-list --reverse "$1..$2")"

while read -r new_commit_sha ; do
  if [ -z "$new_commit_sha" ] ; then
    continue  # skip empty lines
log() {
  type="$1"
  shift 1

  local -A prefix
  prefix[success]="  ✔ "
  if [ -v GITHUB_ACTIONS ]; then
    prefix[warning]="::warning::"
    prefix[error]="::error::"
  else
    prefix[warning]="  ⚠ "
    prefix[error]="  ✘ "
  fi

  echo "${prefix[$type]}$@"

  # Only logging errors and warnings, which allows comparing the markdown file
  # between pushes to the PR. Even if a new, proper cherry-pick, commit is added
  # it won't change the markdown file's content and thus not trigger another comment.
  if [ "$type" != "success" ]; then
    local -A alert
    alert[warning]="WARNING"
    alert[error]="CAUTION"
    echo >> $markdown_file
    echo "> [!${alert[$type]}]" >> $markdown_file
    echo "> $@" >> $markdown_file
  fi
  if [ "$GITHUB_ACTIONS" = 'true' ] ; then
}

endgroup() {
  if [ -v GITHUB_ACTIONS ] ; then
    echo ::endgroup::
  fi
}

while read -r new_commit_sha ; do
  if [ -v GITHUB_ACTIONS ] ; then
    echo "::group::Commit $new_commit_sha"
  else
    echo "================================================="
@@ -37,15 +72,8 @@ while read -r new_commit_sha ; do
    | grep -Eoi -m1 '[0-9a-f]{40}' || true
  )
  if [ -z "$original_commit_sha" ] ; then
    if [ "$GITHUB_ACTIONS" = 'true' ] ; then
      echo ::endgroup::
      echo -n "::error ::"
    else
      echo -n "  ✘ "
    fi
    echo "Couldn't locate original commit hash in message"
    echo "Note this should not necessarily be treated as a hard fail, but a reviewer's attention should" \
      "be drawn to it and github actions have no way of doing that but to raise a 'failure'"
    endgroup
    log warning "Couldn't locate original commit hash in message of $new_commit_sha."
    problem=1
    continue
  fi
@@ -65,8 +93,6 @@ while read -r new_commit_sha ; do

    while read -r picked_branch ; do
      if git merge-base --is-ancestor "$original_commit_sha" "$picked_branch" ; then
        echo "  ✔ $original_commit_sha present in branch $picked_branch"

        range_diff_common='git --no-pager range-diff
          --no-notes
          --creation-factor=100
@@ -75,23 +101,38 @@ while read -r new_commit_sha ; do
        '

        if $range_diff_common --no-color 2> /dev/null | grep -E '^ {4}[+-]{2}' > /dev/null ; then
          if [ "$GITHUB_ACTIONS" = 'true' ] ; then
            echo ::endgroup::
            echo -n "::warning ::"
          else
            echo -n "  ⚠ "
          log success "$original_commit_sha present in branch $picked_branch"
          endgroup
          log warning "Difference between $new_commit_sha and original $original_commit_sha may warrant inspection."

          # First line contains commit SHAs, which we already printed.
          $range_diff_common --color | tail -n +2

          echo -e "> <details><summary>Show diff</summary>\n>" >> $markdown_file
          echo '> ```diff' >> $markdown_file
          # The output of `git range-diff` is indented with 4 spaces, which we need to match with the
          # code blocks indent to get proper syntax highlighting on GitHub.
          diff="$($range_diff_common | tail -n +2 | sed -Ee 's/^ {4}/> /g')"
          # Also limit the output to 10k bytes (and remove the last, potentially incomplete line), because
          # GitHub comments are limited in length. The value of 10k is arbitrary with the assumption, that
          # after the range-diff becomes a certain size, a reviewer is better off reviewing the regular diff
          # in GitHub's UI anyway, thus treating the commit as "new" and not cherry-picked.
          # Note: This could still lead to a too lengthy comment with multiple commits touching the limit. We
          # consider this too unlikely to happen, to deal with explicitly.
          max_length=10000
          if [ "${#diff}" -gt $max_length ]; then
            printf -v diff "%s\n\n[...truncated...]" "$(echo "$diff" | head -c $max_length | head -n-1)"
          fi
          echo "Difference between $new_commit_sha and original $original_commit_sha may warrant inspection:"
          echo "$diff" >> $markdown_file
          echo '> ```' >> $markdown_file
          echo "> </details>" >> $markdown_file

          $range_diff_common --color

          echo "Note this should not necessarily be treated as a hard fail, but a reviewer's attention should" \
            "be drawn to it and github actions have no way of doing that but to raise a 'failure'"
          problem=1
        else
          echo "  ✔ $original_commit_sha highly similar to $new_commit_sha"
          log success "$original_commit_sha present in branch $picked_branch"
          log success "$original_commit_sha highly similar to $new_commit_sha"
          $range_diff_common --color
          [ "$GITHUB_ACTIONS" = 'true' ] && echo ::endgroup::
          endgroup
        fi

        # move on to next commit
@@ -100,13 +141,8 @@ while read -r new_commit_sha ; do
    done <<< "$branches"
  done

  if [ "$GITHUB_ACTIONS" = 'true' ] ; then
    echo ::endgroup::
    echo -n "::error ::"
  else
    echo -n "  ✘ "
  fi
  echo "$original_commit_sha not found in any pickable branch"
  endgroup
  log error "$original_commit_sha given in $new_commit_sha not found in any pickable branch."

  problem=1
done <<< "$commits"