Unverified Commit 515b174c authored by Wolfgang Walther's avatar Wolfgang Walther
Browse files

workflows/check-cherry-picks: post review comments 

Instead of failing the job, the workflow will now post review comments
as "Request Changes". This makes the feedback more readily visible and
avoids having to merge despite a failing CI job. It is also a
pre-requisite to enable required status checks / required workflows in
the future.

Committers are asked to confirm the differences by explicitly dismissing
the generated review. After dismissal, the related review comment will
automatically be marked as "resolved".

The comments only report warnings and errors. Reviews are automatically
dismissed when they have been addressed by the author and no problems
remain. If problems remain, existing, still pending, review comments
will be updated. If the same problems had already been dismissed
earlier, no new review comment will be created either.
parent 3dff9c34

.github/workflows/check-cherry-picks.yml

+100 −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,105 @@ 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 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

              )

            )



            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 })

              })

            )

.github/workflows/dismissed-review.yml

0 → 100644
+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 })

ci/check-cherry-picks.md

0 → 100644
+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.

ci/check-cherry-picks.sh

+28 −9
Original line number Diff line number Diff line
@@ -3,11 +3,14 @@ 


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]}")"
@@ -34,6 +37,18 @@ log() { 
  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

}



endgroup() {
@@ -58,9 +73,7 @@ while read -r new_commit_sha ; do 
  )

  if [ -z "$original_commit_sha" ] ; then

    endgroup

    log error "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'"

    log warning "Couldn't locate original commit hash in message of $new_commit_sha."

    problem=1

    continue

  fi
@@ -90,13 +103,19 @@ while read -r new_commit_sha ; do 
        if $range_diff_common --no-color 2> /dev/null | grep -E '^ {4}[+-]{2}' > /dev/null ; then

          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:"

          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 "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'"

          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.

          $range_diff_common | tail -n +2 | sed -Ee 's/^ {4}/> /g' >> $markdown_file

          echo '> ```' >> $markdown_file

          echo "> </details>" >> $markdown_file



          problem=1

        else

          log success "$original_commit_sha present in branch $picked_branch"
@@ -112,7 +131,7 @@ while read -r new_commit_sha ; do 
  done



  endgroup

  log error "$original_commit_sha not found in any pickable branch"

  log error "$original_commit_sha given in $new_commit_sha not found in any pickable branch."



  problem=1

done <<< "$commits"