From ad4b36d2d23e01c9c7303f4c9c6e1270a93dfe7f Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 12:50:18 +0200 Subject: [PATCH 1/9] ci/check-cherry-picks: move from maintainers/scripts The script is part of CI and changes to it should be reviewed by the CI owners. Thus moving it to ci/ is the most sensible thing to do. --- .github/workflows/check-cherry-picks.yml | 2 +- {maintainers/scripts => ci}/check-cherry-picks.sh | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename {maintainers/scripts => ci}/check-cherry-picks.sh (100%) diff --git a/.github/workflows/check-cherry-picks.yml b/.github/workflows/check-cherry-picks.yml index e85fa59bb6995..5bd93ee4bbf90 100644 --- a/.github/workflows/check-cherry-picks.yml +++ b/.github/workflows/check-cherry-picks.yml @@ -28,4 +28,4 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | - ./trusted/maintainers/scripts/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA" + ./trusted/ci/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA" diff --git a/maintainers/scripts/check-cherry-picks.sh b/ci/check-cherry-picks.sh similarity index 100% rename from maintainers/scripts/check-cherry-picks.sh rename to ci/check-cherry-picks.sh From e2a37921691a3c0131114795d4b756aa7bd5d4a4 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 12:39:32 +0200 Subject: [PATCH 2/9] ci/check-cherry-picks: improve error handling We recently moved the $commits variable out of a "subshell in a herestring", let's do the same for the list of branches, where errors would be silently swallowed as well. Also reformat the expressions slightly, we have enough line-length. --- ci/check-cherry-picks.sh | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index 0e02c709d8f5d..31ff9d3a82a0a 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -14,9 +14,7 @@ cd "$(dirname "${BASH_SOURCE[0]}")" PICKABLE_BRANCHES=${PICKABLE_BRANCHES:-master staging release-??.?? staging-??.??} problem=0 -commits="$(git rev-list \ - -E -i --grep="cherry.*[0-9a-f]{40}" --reverse \ - "$1..$2")" +commits="$(git rev-list -E -i --grep="cherry.*[0-9a-f]{40}" --reverse "$1..$2")" while read new_commit_sha ; do if [ -z "$new_commit_sha" ] ; then @@ -45,6 +43,8 @@ while read new_commit_sha ; do for branch_pattern in $PICKABLE_BRANCHES ; do set +f # re-enable pathname expansion + branches="$(git for-each-ref --format="%(refname)" "refs/remotes/origin/$branch_pattern")" + 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" @@ -79,11 +79,7 @@ while read new_commit_sha ; do # move on to next commit continue 3 fi - done <<< "$( - git for-each-ref \ - --format="%(refname)" \ - "refs/remotes/origin/$branch_pattern" - )" + done <<< "$branches" done if [ "$GITHUB_ACTIONS" = 'true' ] ; then From 6cf5f9e83bad75cb431661e35c7e15afe1fdc1e9 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 12:43:41 +0200 Subject: [PATCH 3/9] ci/check-cherry-picks: run shellcheck --- ci/check-cherry-picks.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index 31ff9d3a82a0a..9e12464526d2c 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -16,7 +16,7 @@ problem=0 commits="$(git rev-list -E -i --grep="cherry.*[0-9a-f]{40}" --reverse "$1..$2")" -while read new_commit_sha ; do +while read -r new_commit_sha ; do if [ -z "$new_commit_sha" ] ; then continue # skip empty lines fi @@ -33,7 +33,7 @@ while read new_commit_sha ; do | grep -Ei -m1 "cherry.*[0-9a-f]{40}" \ | grep -Eoi -m1 '[0-9a-f]{40}' ) - if [ "$?" != "0" ] ; then + if [ -z "$original_commit_sha" ] ; then echo " ? Couldn't locate original commit hash in message" [ "$GITHUB_ACTIONS" = 'true' ] && echo ::endgroup:: continue From 2fea2bbf527ec30a62892f146d1a42305f7aa8f8 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 12:48:30 +0200 Subject: [PATCH 4/9] ci/check-cherry-picks: support different remotes than "origin" The default is to checkout a contributors fork as "origin", thus the NixOS/nixpkgs remote is most likely named differently. But not everybody keeps their fork's main branches up-to-date all the time. Thus the script would fail locally. --- ci/check-cherry-picks.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index 9e12464526d2c..44924ab8d8ab7 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -14,6 +14,9 @@ cd "$(dirname "${BASH_SOURCE[0]}")" PICKABLE_BRANCHES=${PICKABLE_BRANCHES:-master staging release-??.?? staging-??.??} problem=0 +# Not everyone calls their remote "origin" +remote="$(git remote -v | grep -i 'NixOS/nixpkgs' | head -n1 | cut -f1 || true)" + commits="$(git rev-list -E -i --grep="cherry.*[0-9a-f]{40}" --reverse "$1..$2")" while read -r new_commit_sha ; do @@ -40,10 +43,10 @@ while read -r new_commit_sha ; do fi set -f # prevent pathname expansion of patterns - for branch_pattern in $PICKABLE_BRANCHES ; do + for pattern in $PICKABLE_BRANCHES ; do set +f # re-enable pathname expansion - branches="$(git for-each-ref --format="%(refname)" "refs/remotes/origin/$branch_pattern")" + branches="$(git for-each-ref --format="%(refname)" "refs/remotes/${remote:-origin}/$pattern")" while read -r picked_branch ; do if git merge-base --is-ancestor "$original_commit_sha" "$picked_branch" ; then From 245b1c1c4859d4e5636dd2d3bccb3e4d3c44558e Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 14:56:08 +0200 Subject: [PATCH 5/9] ci/check-cherry-picks: never use a pager In a small terminal window this would just stop running after each commit until you exit the pager. That's not what we want when running it locally. --- ci/check-cherry-picks.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index 44924ab8d8ab7..dd8e9bb0ca191 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -52,7 +52,7 @@ while read -r new_commit_sha ; 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 range-diff + range_diff_common='git --no-pager range-diff --no-notes --creation-factor=100 '"$original_commit_sha~..$original_commit_sha"' From e575364ae613c5e208862350c48a60e6234b3086 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 13:12:19 +0200 Subject: [PATCH 6/9] workflows/check-cherry-picks: reduce checkout time Using a `tree:0` filter instead of `blob:none` reduces the checkout time from over 3 minutes to about 45 seconds. The required trees/blobs will then be fetched on-demand. This on-demand fetching creates additional output for `git range-diff` on stderr, so we hide that. This only happens the first time it's run, so we don't need to adjust the other calls - which will still return any real errors, should they happen. --- .github/workflows/check-cherry-picks.yml | 2 +- ci/check-cherry-picks.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-cherry-picks.yml b/.github/workflows/check-cherry-picks.yml index 5bd93ee4bbf90..3c6ef1d189206 100644 --- a/.github/workflows/check-cherry-picks.yml +++ b/.github/workflows/check-cherry-picks.yml @@ -20,7 +20,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: fetch-depth: 0 - filter: blob:none + filter: tree:0 path: trusted - name: Check cherry-picks diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index dd8e9bb0ca191..580d6da6a38ec 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -59,7 +59,7 @@ while read -r new_commit_sha ; do '"$new_commit_sha~..$new_commit_sha"' ' - if $range_diff_common --no-color | grep -E '^ {4}[+-]{2}' > /dev/null ; then + 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 ::" From ea636d1728f25328511401c4919eec96e7426de0 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 15:29:50 +0200 Subject: [PATCH 7/9] ci/check-cherry-picks: allow cherry-picking from haskell-updates and python-updates Those are protected branches, which can't be force pushed to - so the commits will remain. Thus, we can also backport from them. --- ci/check-cherry-picks.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index 580d6da6a38ec..2d1e6eb7c03c0 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -11,7 +11,7 @@ fi # Make sure we are inside the nixpkgs repo, even when called from outside cd "$(dirname "${BASH_SOURCE[0]}")" -PICKABLE_BRANCHES=${PICKABLE_BRANCHES:-master staging release-??.?? staging-??.??} +PICKABLE_BRANCHES="master release-??.?? staging-??.?? haskell-updates python-updates" problem=0 # Not everyone calls their remote "origin" From a9b718b79640e9c0ad4366391654fd4138248187 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 15:31:14 +0200 Subject: [PATCH 8/9] ci/check-cherry-picks: never check older stable branches This makes the job significantly faster when the commit can't be found on master or staging directly. Before this change, the script would have had to iterate through 20+ release branches before finding the latest one. With lazy fetching for git enabled, this would take a few minutes. --- ci/check-cherry-picks.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index 2d1e6eb7c03c0..0ef8ff904f4ab 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -46,7 +46,14 @@ while read -r new_commit_sha ; do for pattern in $PICKABLE_BRANCHES ; do set +f # re-enable pathname expansion - branches="$(git for-each-ref --format="%(refname)" "refs/remotes/${remote:-origin}/$pattern")" + # Reverse sorting by refname and taking one match only means we can only backport + # from unstable and the latest stable. That makes sense, because even right after + # branch-off, when we have two supported stable branches, we only ever want to cherry-pick + # **to** the older one, but never **from** it. + # This makes the job significantly faster in the case when commits can't be found, + # because it doesn't need to iterate through 20+ branches, which all need to be fetched. + branches="$(git for-each-ref --sort=-refname --format="%(refname)" \ + "refs/remotes/${remote:-origin}/$pattern" | head -n1)" while read -r picked_branch ; do if git merge-base --is-ancestor "$original_commit_sha" "$picked_branch" ; then From dfaefc053531399e9359f4d41054be4f7b597e45 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 28 May 2025 13:05:17 +0200 Subject: [PATCH 9/9] ci/check-cherry-picks: fail without proper cherry-pick When cherry-picking without -x or not cherry-picking at all, the check-cherry-picks job would usually remain green. This is annoying to deal with for reviewers, because "all green" still needs attention - have all commits been cherry-picked properly? If a commit was not cherry-picked correctly, either without -x or not at all, because it's a genuine commit to begin with, the reviewers attention is required anyway. Thus we can also let the job fail in this case. --- ci/check-cherry-picks.sh | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index 0ef8ff904f4ab..612174925f575 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -17,7 +17,7 @@ problem=0 # Not everyone calls their remote "origin" remote="$(git remote -v | grep -i 'NixOS/nixpkgs' | head -n1 | cut -f1 || true)" -commits="$(git rev-list -E -i --grep="cherry.*[0-9a-f]{40}" --reverse "$1..$2")" +commits="$(git rev-list --reverse "$1..$2")" while read -r new_commit_sha ; do if [ -z "$new_commit_sha" ] ; then @@ -34,11 +34,19 @@ while read -r new_commit_sha ; do original_commit_sha=$( git rev-list --max-count=1 --format=format:%B "$new_commit_sha" \ | grep -Ei -m1 "cherry.*[0-9a-f]{40}" \ - | grep -Eoi -m1 '[0-9a-f]{40}' + | grep -Eoi -m1 '[0-9a-f]{40}' || true ) if [ -z "$original_commit_sha" ] ; then - echo " ? Couldn't locate original commit hash in message" - [ "$GITHUB_ACTIONS" = 'true' ] && echo ::endgroup:: + 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'" + problem=1 continue fi