Skip to content

Commit 1e50484

Browse files
author
Release Manager
committed
gh-36686: `.ci/merge-fixes.sh`: Obtain patches via URL, make customizable by repository variable <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Instead of using `gh pr checkout`, we obtain the CI fixes via their patch URLs. - This is faster because typically we do not have to unshallow the repo to apply the patches (seconds instead of ~2 minutes) - Fewer surprises when applied to a PR based on an older release - Conjecturally uses fewer API queries, helping avoid #36685 When a repository variable `SAGE_CI_FIXES_FROM_REPOSITORIES` is set in a fork, it is used instead of the hardcoded sagemath/sage as the source(s) of the CI fixes; this gives better control in decentralized development. When set to "none", this also makes it possible to see the "ground truth", addressing a concern raised in #36349 (comment). See comments at the top of `.ci/merge-fixes.sh` for details. Also improving the display of details of the merged PRs, as requested by @tornaria in #36696 (comment) Example runs: - https://github.com/sagemath/sage/actions/runs/6854931832/job/186389018 59?pr=36686 (here it recovers gracefully from failed merges due to the 10.2.rc1/10.2.rc2 tagging confusions) - https://github.com/mkoeppe/sage/actions/runs/6855410957/job/1864038039 8#step:3:8 (I have set `SAGE_CI_FIXES_FROM_REPOSITORIES=none` in my fork -- for testing this PR) <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes #12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - #12345: short description why this is a dependency - #34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #36686 Reported by: Matthias Köppe Reviewer(s): Gonzalo Tornaría, Kwankyu Lee, Matthias Köppe
2 parents b8daad8 + 49b7f74 commit 1e50484

File tree

8 files changed

+87
-33
lines changed

8 files changed

+87
-33
lines changed

.ci/merge-fixes.sh

Lines changed: 79 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,82 @@
11
#!/bin/sh
2-
# Merge open PRs from sagemath/sage labeled "blocker".
3-
REPO="sagemath/sage"
4-
GH="gh -R $REPO"
5-
PRs="$($GH pr list --label "p: blocker / 1" --json number --jq '.[].number')"
6-
if [ -z "$PRs" ]; then
7-
echo 'Nothing to do: Found no open PRs with "blocker" status.'
8-
else
9-
echo "Found PRs: " $PRs
10-
export GIT_AUTHOR_NAME="ci-sage workflow"
11-
export GIT_AUTHOR_EMAIL="[email protected]"
12-
export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME"
13-
export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"
14-
git tag -f test_base
15-
git commit -q -m "Uncommitted changes" --no-allow-empty -a
16-
for a in $PRs; do
17-
git fetch --unshallow --all > /dev/null 2>&1 && echo "Unshallowed."
18-
echo "::group::Merging PR https://github.com/$REPO/pull/$a"
19-
git tag -f test_head
20-
$GH pr checkout -b pr-$a $a
21-
git checkout -q test_head
22-
if git merge --no-edit --squash -q pr-$a; then
23-
echo "::endgroup::"
24-
if git commit -q -m "Merge https://github.com/$REPO/pull/$a" -a --no-allow-empty; then
25-
echo "Merged #$a"
2+
# Apply open PRs labeled "blocker" from sagemath/sage as patches.
3+
# This script is invoked by various workflows in .github/workflows
4+
#
5+
# The repository variable SAGE_CI_FIXES_FROM_REPOS can be set
6+
# to customize this for CI runs in forks:
7+
#
8+
# - If set to a whitespace-separated list of repositories, use them instead of sagemath/sage.
9+
# - If set to "none", do not apply any PRs.
10+
#
11+
# https://docs.github.com/en/actions/learn-github-actions/variables#creating-configuration-variables-for-a-repository
12+
export GIT_AUTHOR_NAME="ci-sage workflow"
13+
export GIT_AUTHOR_EMAIL="[email protected]"
14+
export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME"
15+
export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"
16+
mkdir -p upstream
17+
for REPO in ${SAGE_CI_FIXES_FROM_REPOSITORIES:-sagemath/sage}; do
18+
case $REPO in
19+
none)
20+
echo "Nothing to do for 'none' in SAGE_CI_FIXES_FROM_REPOSITORIES"
21+
;;
22+
*/*)
23+
echo "Getting open PRs with 'blocker' status from https://github.com/$REPO/pulls?q=is%3Aopen+label%3A%22p%3A+blocker+%2F+1%22"
24+
GH="gh -R $REPO"
25+
REPO_FILE="upstream/ci-fixes-${REPO%%/*}-${REPO##*/}"
26+
PRs="$($GH pr list --label "p: blocker / 1" --json number --jq '.[].number' | tee $REPO_FILE)"
27+
date -u +"%Y-%m-%dT%H:%M:%SZ" > $REPO_FILE.date # Record the date, for future reference
28+
if [ -z "$PRs" ]; then
29+
echo "Nothing to do: Found no open PRs with 'blocker' status in $REPO."
2630
else
27-
echo "Empty, skipped"
31+
echo "Found open PRs with 'blocker' status in $REPO: $(echo $PRs)"
32+
git tag -f test_base
33+
git commit -q -m "Uncommitted changes" --no-allow-empty -a
34+
for a in $PRs; do
35+
# We used to pull the branch and merge it (after unshallowing), but when run on PRs that are
36+
# based on older releases, it has the side effect of updating to this release,
37+
# which may be confusing.
38+
#
39+
# Here, we obtain the "git am"-able patch of the PR branch.
40+
# This also makes it unnecessary to unshallow the repository.
41+
#
42+
# Considered alternative: Use https://github.com/$REPO/pull/$a.diff,
43+
# which squashes everything into one diff without commit metadata.
44+
PULL_URL="https://github.com/$REPO/pull/$a"
45+
PULL_FILE="$REPO_FILE-$a"
46+
PATH=build/bin:$PATH build/bin/sage-download-file --quiet "$PULL_URL.patch" $PULL_FILE.patch
47+
date -u +"%Y-%m-%dT%H:%M:%SZ" > $PULL_FILE.date # Record the date, for future reference
48+
LAST_SHA=$(sed -n -E '/^From [0-9a-f]{40}/s/^From ([0-9a-f]{40}).*/\1/p' $PULL_FILE.patch | tail -n 1)
49+
COMMITS_URL="https://github.com/$REPO/commits/$LAST_SHA"
50+
echo "::group::Applying PR $PULL_URL @ $COMMITS_URL as a patch"
51+
export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME applying $PULL_URL @ $COMMITS_URL"
52+
if git am --signoff --empty=keep < $PULL_FILE.patch; then
53+
echo "---- Applied patch --------------------------------------------------------------------------------"
54+
cat $PULL_FILE.patch
55+
echo "--------------------------------------------------------------------8<-----------------------------"
56+
echo "::endgroup::"
57+
elif git am --abort \
58+
&& if git fetch --unshallow --all > /dev/null 2>&1; then echo "Unshallowed"; fi \
59+
&& echo "Retrying with 3-way merge" \
60+
&& git am --empty=keep --3way < $PULL_FILE.patch; then
61+
echo "---- Applied patch --------------------------------------------------------------------------------"
62+
cat $PULL_FILE.patch
63+
echo "--------------------------------------------------------------------8<-----------------------------"
64+
echo "::endgroup::"
65+
else
66+
echo "---- Failure applying patch -----------------------------------------------------------------------"
67+
git am --signoff --show-current-patch=diff
68+
echo "--------------------------------------------------------------------8<-----------------------------"
69+
echo "::endgroup::"
70+
echo "Failure applying $PULL_URL as a patch, resetting"
71+
git am --signoff --abort
72+
fi
73+
done
2874
fi
29-
else
30-
echo "::endgroup::"
31-
echo "Failure merging #$a, resetting"
32-
git reset --hard
33-
fi
34-
done
35-
git log test_base..HEAD
36-
fi
75+
;;
76+
*)
77+
echo "Repository variable SAGE_CI_FIXES_FROM_REPOSITORIES, if set, should be a whitespace-separated list of repositories or 'none'"
78+
echo "Got: $SAGE_CI_FIXES_FROM_REPOSITORIES"
79+
exit 1
80+
;;
81+
esac
82+
done

.github/workflows/build.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ jobs:
3838
.ci/merge-fixes.sh
3939
env:
4040
GH_TOKEN: ${{ github.token }}
41+
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}
4142
- name: Store CI fixes in upstream artifact
4243
run: |
4344
mkdir -p upstream

.github/workflows/ci-conda.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ jobs:
3838
.ci/merge-fixes.sh
3939
env:
4040
GH_TOKEN: ${{ github.token }}
41+
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}
4142

4243
- name: Create conda environment files
4344
run: ./bootstrap-conda

.github/workflows/doc-build-pdf.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ jobs:
3232
.ci/merge-fixes.sh
3333
env:
3434
GH_TOKEN: ${{ github.token }}
35+
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}
3536
- name: Store CI fixes in upstream artifact
3637
run: |
3738
mkdir -p upstream

.github/workflows/doc-build.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ jobs:
2727
.ci/merge-fixes.sh
2828
env:
2929
GH_TOKEN: ${{ github.token }}
30+
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}
3031
- name: Store CI fixes in upstream artifact
3132
run: |
3233
mkdir -p upstream

.github/workflows/docker.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,8 @@ jobs:
222222
.ci/merge-fixes.sh
223223
env:
224224
GH_TOKEN: ${{ github.token }}
225+
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}
226+
225227
- name: Show disk space
226228
run: |
227229
df -h

.github/workflows/lint.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ jobs:
2626
.ci/merge-fixes.sh
2727
env:
2828
GH_TOKEN: ${{ github.token }}
29+
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}
2930

3031
- name: Set up Python
3132
uses: actions/setup-python@v4

.github/workflows/macos.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ jobs:
9494
.ci/merge-fixes.sh
9595
env:
9696
GH_TOKEN: ${{ github.token }}
97+
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}
9798

9899
- uses: actions/download-artifact@v3
99100
with:

0 commit comments

Comments
 (0)