-
-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CI macOS: Fix failure with macos-13-homebrew, add tests on M1 runners, add timeouts #37237
Changes from all commits
f4e2aff
15193b3
6b7c9a6
7b1d6c9
9d924c8
c885e54
50d1eaf
52654ea
700a939
c8ee8e7
49fdc95
5f4e0fe
1f2d9f9
628c219
d26937b
d47f100
2165bef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,17 +19,21 @@ on: | |
type: string | ||
# System configuration | ||
osversion_xcodeversion_toxenv_tuples: | ||
# As of 2024-02, "runs-on: macos-latest" is macos-12. | ||
# and "runs-on: macos-14" selects the new M1 runners. | ||
# https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories | ||
description: 'Stringified JSON object' | ||
default: >- | ||
[["latest", "", "homebrew-macos-usrlocal-minimal"], | ||
["latest", "", "homebrew-macos-usrlocal-standard"], | ||
["11", "xcode_13.2.1", "homebrew-macos-usrlocal-minimal"], | ||
[["11", "xcode_13.2.1", "homebrew-macos-usrlocal-minimal"], | ||
["12", "", "homebrew-macos-usrlocal-minimal"], | ||
["12", "", "homebrew-macos-usrlocal-standard"], | ||
["12", "", "homebrew-macos-usrlocal-python3_xcode-standard"], | ||
["12", "", "homebrew-macos-usrlocal-maximal"], | ||
["13", "xcode_15.0", "homebrew-macos-usrlocal-standard"], | ||
["latest", "", "homebrew-macos-usrlocal-maximal"], | ||
["latest", "", "homebrew-macos-usrlocal-python3_xcode-standard"], | ||
["14", "", "homebrew-macos-opthomebrew-standard"], | ||
["latest", "", "conda-forge-macos-minimal"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fails, can we disable it until someone finds the time to fix it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it's useful as a reminder what needs fixing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An issue would serve that purpose better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. Source: I'm the person who monitors the portability CI. |
||
["latest", "", "conda-forge-macos-standard"]] | ||
["latest", "", "conda-forge-macos-standard"], | ||
["14", "", "conda-forge-macos-standard"]] | ||
type: string | ||
extra_sage_packages: | ||
description: 'Extra Sage packages to install as system packages' | ||
|
@@ -41,6 +45,10 @@ on: | |
free_disk_space: | ||
default: false | ||
type: boolean | ||
timeout: | ||
description: 'Elapsed time (seconds) at which to kill the build' | ||
default: 20000 | ||
type: number | ||
# | ||
# For use in upstream CIs. | ||
# | ||
|
@@ -74,10 +82,16 @@ jobs: | |
repository: ${{ inputs.sage_repo }} | ||
ref: ${{ inputs.sage_ref }} | ||
fetch-depth: 10000 | ||
|
||
- uses: actions/setup-python@v5 | ||
# As of 2024-02-03, the macOS M1 runners do not have preinstalled python or pipx. | ||
# Installing pipx follows the approach of https://github.com/pypa/cibuildwheel/pull/1743 | ||
id: python | ||
with: | ||
python-version: "3.8 - 3.12" | ||
update-environment: false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you set update-environment to false? Otherwise you could just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because i want to use this Python only for running tox, not for influencing the Sage build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it would have a preinstalled python, it would also influence the sage build. |
||
- name: Install test prerequisites | ||
run: | | ||
brew install tox | ||
"${{ steps.python.outputs.python-path }}" -m pip install pipx | ||
- name: Download upstream artifact | ||
uses: actions/download-artifact@v3 | ||
with: | ||
|
@@ -129,7 +143,8 @@ jobs: | |
*) export TARGETS_PRE="${{ inputs.targets_pre }}" TARGETS="${{ inputs.targets }} TARGETS_OPTIONAL="${{ inputs.targets_optional }} | ||
;; | ||
esac | ||
MAKE="make -j12" tox -e $TOX_ENV -- SAGE_NUM_THREADS=4 $TARGETS | ||
(sleep ${{ inputs.timeout }}; pkill make) & | ||
MAKE="make -j12" EXTRA_SAGE_PACKAGES="${{ inputs.extra_sage_packages }}" "${{ steps.python.outputs.python-path }}" -m pipx run tox -e $TOX_ENV -- SAGE_NUM_THREADS=6 $TARGETS | ||
- name: Prepare logs artifact | ||
run: | | ||
mkdir -p "artifacts/$LOGS_ARTIFACT_NAME"; cp -r .tox/*/log "artifacts/$LOGS_ARTIFACT_NAME" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the overall execution time constant (to not increase the priority of the portability ci runs over the PR runs even more), please remove macos-11 (it's past its end of life anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not a relevant criterion. These platforms are tested because we want them tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it's a very relevant criterion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll not be negotiating with you how I run the portability CI.
For reference: #36726 (comment), #36726 (comment)