Skip to content
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

GH Actions (macOS): Run a job for "make build-local" first, cache image for job "make build" #32703

Closed
mkoeppe opened this issue Oct 16, 2021 · 79 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Oct 16, 2021

We revise the GH Actions workflows to use a 2-stage build:
In the first stage, run make build-local, and store SAGE_LOCAL as a build artifact. In the second stage, download the build artifact and run more building and testing.

(+) On top of the artifact containing the full SAGE_LOCAL, we can test several ways to build the Python parts

(+) Tests of optional and experimental packages can be streamlined, as we avoid rebuilding their dependencies that are standard packages.

(+) Splitting the job into two would also help with the configurations for which we scrape at the 6 hour time limit

(-) Unfortunately, because because "needs" cannot depend on "matrix", the jobs for building/testing Python packages would not start before all jobs building SAGE_LOCAL for all platforms are completed

In this ticket, we only change all existing macos workflows to a 2-stage workflow, integrating the separate workflows for optional and experimental packages.

As of this ticket, we rely on the bottleneck of the available parallel jobs on GH Actions to ensure that the 2nd stages of a configuration are run after the 1st stage of that configuration. Experience with this workflow will show whether this suffices.

We also update the macOS/Xcode versions according to what's available on GH Actions and switch the homebrew builds to faster homebrew-usrlocal variants, which can use bottles for all available packages.

Depends on #32113
Depends on #32947

CC: @tobiasdiez @kliem @orlitzky @isuruf

Component: porting

Author: Matthias Koeppe

Branch/Commit: edb4364

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/32703

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 16, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 16, 2021

Dependencies: #31535

@mkoeppe mkoeppe changed the title GH Actions: In tox-docker, run a job for "make build-local" first, cache image for job "make build" GH Actions: Run a job for "make build-local" first, cache image for job "make build" Oct 17, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 17, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 17, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 17, 2021

Commit: d08452c

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 17, 2021

Last 10 new commits:

2101b8cbuild/pkgs/python3/spkg-build.in: Make sure that python finds sqlite3 when determining which extension modules to build
3bbc5d8build/pkgs/python3/spkg-build.in: Set rpath
124b605Merge #32698
bca2141pkgs/sagemath-standard/tox.ini: Use SAGE_VENV or venv symlink to find wheels
bdbab3abuild/make/Makefile.in: Remove SPKG-tox, SPKG-sdist targets
63e47ffMerge #31535
1c40d1c.github/workflows/extract-sage-local.sh: Make script usable on non-cygwin
dee3f41github/workflows/tox.yml: Multi-stage local-macos
bd9fad1.github/workflows/: Remove workflows for testing
d08452cUpdate systems

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 17, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

0799c5f.github/workflows/tox.yml: Fixup for macOS tar

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Changed commit from d08452c to 0799c5f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

e031622.github/workflows/tox.yml: Fix up upload path

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Changed commit from 0799c5f to e031622

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

a40d706.github/workflows/tox.yml: Do not use /tmp for artifacts to avoid https://github.com/actions/upload-artifact/issues/92

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Changed commit from e031622 to a40d706

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 18, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

dd2bc15.github/workflows/tox.yml: Use env.GITHUB_WORKSPACE
2b46f02.github/workflows/tox.yml: Use sage-local-artifact/ as path

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 18, 2021

Changed commit from a40d706 to 2b46f02

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 19, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 19, 2021

comment:11

working well now

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 19, 2021

Changed commit from 2b46f02 to 4dcd768

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 19, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

4dcd768.github/workflows/tox.yml: mkdir

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 19, 2021

Changed commit from 4dcd768 to 828463d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 19, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

828463dgithub/workflows/tox.yml: Multi-stage local-macos

@tobiasdiez
Copy link
Contributor

comment:14

If I understand the changes (and the output) correctly, then currently stage 1 and 2 run parallel. For example, some stage 2 runs (e.g https://github.com/mkoeppe/sage/runs/3935018795?check_suite_focus=true) fail since they try to download a non-existing artifact. Maybe something like https://github.com/marketplace/actions/wait-for-check helps.

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 19, 2021

comment:15

Replying to @tobiasdiez:

If I understand the changes (and the output) correctly, then currently stage 1 and 2 run parallel.

Yes, in theory. But in practice we only get 5 parallel jobs on macOS and we launch many more jobs...

For example, some stage 2 runs (e.g https://github.com/mkoeppe/sage/runs/3935018795?check_suite_focus=true) fail since they try to download a non-existing artifact.

Well, they failed because the previous stage failed.

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 19, 2021

comment:16

Replying to @tobiasdiez:

Maybe something like https://github.com/marketplace/actions/wait-for-check helps.

Thanks for the pointer, we can look into using something like this

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 19, 2021

comment:17

Testing with integrated builds for the optional packages now at https://github.com/mkoeppe/sage/actions/runs/1360303527

@tobiasdiez
Copy link
Contributor

comment:18

Replying to @mkoeppe:

Replying to @tobiasdiez:

If I understand the changes (and the output) correctly, then currently stage 1 and 2 run parallel.

Yes, in theory. But in practice we only get 5 parallel jobs on macOS and we launch many more jobs...

Okay, but isn't this a very fragile design? Imagine that you have exactly 5 matrix cases. So all of the stage 1 tasks start at the same time. Now if the last case exists slightly before the other 4, then the stage 2 for the other 4 may be invoked and now fail as their stage 1 is not yet finished. Also this would limit everything to mac, as on linux you have way more runners.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 21, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

43d481bbump to 0.1.0
f66821bcysignals are needed
ae37b4edeprecate sage.interfaces.primecount, not just remove
d3ae5f4primecount is on conda, too
3a8c7feprimesieve is on conda, too
74b3845allow float inputs for prime_pi
591be34Merge #32894
049a5f6tox.ini: Do not set environment variable CONDARC
f3116a1tox.ini (conda): Force use of conda's python3
6418579Merge #32113

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 21, 2021

Changed commit from 6418579 to 933134c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 21, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

933134cbuild/pkgs/gfortran/distros/homebrew.txt: Use gfortran

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 21, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 21, 2021

Changed dependencies from #32113 to #32113, #32947

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 21, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

ab4fb99tox.ini: Add ubuntu-jammy, debian-bookworm, linuxmint-20.3, fedora-36
e51225a.github/workflows/tox*.yml: Remove debian-jessie, add ubuntu-jammy, debian-bookworm, linuxmint-20.3, fedora-36
ac47e7d.github/workflows/tox-gcc_spkg.yml: Remove
1aa9328Merge #32947
e99a09esed -i.bak 's/ubunty/ubuntu/g' .github/workflows/*.yml

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 21, 2021

Changed commit from 933134c to e99a09e

@tobiasdiez
Copy link
Contributor

comment:48

I know we already touch upon this point, but can you please expand on

Unfortunately, because ​because "needs" cannot depend on "matrix", the jobs for building/testing Python packages would not start before all jobs building SAGE_LOCAL for all platforms are completed

That is, why do you prefer the "custom stage build" over having a "local-macos-stage2" job that depends via "needs" on "local-macos-stage1", where each of these jobs defines an appropriate matrix. In particular, I don't understand the "all platforms" part, as ubuntu and macos are in different jobs, right? It appears to me that with the current design also stage 1 builds are executed before any stage 2 is executed, so the only difference is that you can already have 4 or so stage 2 builds running while the last stage 1 build finishes. I guess for the overall build time this shouldn't make much of a difference anyway, as these 4 runners would be busy with other workflows then (or are free to execute builds of other stuff in the sagemath org).

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 21, 2021

comment:49

Replying to @tobiasdiez:

That is, why do you prefer the "custom stage build" over having a "local-macos-stage2" job that depends via "needs" on "local-macos-stage1", where each of these jobs defines an appropriate matrix.

To avoid copy-paste

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 21, 2021

comment:50

Replying to @tobiasdiez:

I don't understand the "all platforms" part, as ubuntu and macos are in different jobs, right?

Yes, that's right, within each job; and in this ticket, only for macos.

@tobiasdiez
Copy link
Contributor

comment:51

Replying to @mkoeppe:

Replying to @tobiasdiez:

That is, why do you prefer the "custom stage build" over having a "local-macos-stage2" job that depends via "needs" on "local-macos-stage1", where each of these jobs defines an appropriate matrix.

To avoid copy-paste

Github recently made it possible to reuse workflows: https://docs.github.com/en/actions/learn-github-actions/reusing-workflows
Thus you could extract the current local-macos job into a new workflow and pass the right "tox" command as an argument (determined by the matrix + step).

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 21, 2021

comment:52

Thanks for the pointer!! I've added this to #29060 for welcome future refactoring - but this won't make it into Sage 9.5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 21, 2021

Changed commit from e99a09e to edb4364

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 21, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

edb4364.github/workflows/tox.yml: Replace homebrew-macos-python3_xcode-standard by homebrew-macos-usrlocal-python3_xcode-standard

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title GH Actions: Run a job for "make build-local" first, cache image for job "make build" GH Actions (macOS): Run a job for "make build-local" first, cache image for job "make build" Dec 21, 2021
@dimpase
Copy link
Member

dimpase commented Dec 23, 2021

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/1605145773 to Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Dec 23, 2021

comment:55

OK,macOS runs on GH look good.

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 23, 2021

comment:56

Thanks!

@vbraun
Copy link
Member

vbraun commented Jan 1, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants