Autobuild: Refactor & use actions/cache #2284

hoffie · 2022-01-26T09:49:29Z

Short description of changes

This PR introduces an actions/cache step for Android, Mac and Windows builds. For Linux, this makes little sense as all dependencies are installed via apt, which should be almost locally anyway (Azure apt mirror).

For the caching to be effective and correct, all cached versions must be pinned. This is permits the caching logic to invalidate caches automatically based on a key (script content + invocation paramters). Pinning has partly been implemented already (Qt, NSIS, ASIOSDK, Android Command Line Tools, Android NDK, Android SDK). In addition, this PR requires #2345 which introduces version pinning for JACK and aqtinstaller.

The following environments are cached:

Qt installation directory (Windows, Mac, Android)
pip cache (Windows, Mac)
choco cache (Windows)
Android SDK & NDK (Android)

The cache is used if the workflow-provided calls (which include Qt versions) are unchanged and if certain files (most notably autobuild.yml, prepare scripts and prepare script dependencies) are unchanged. If anything changes, the cache is not used. Instead, everything starts clean and the result is cached for further reuse again.

Goals of this PR:

Increase resilience against temporary external outages (e.g. Qt mirror problems)
Make builds more deterministic
Speed up builds by avoiding repeated downloads and some extractions

Risks:

If we get our cache key wrong we might end up building in an unexpected build environment state. I took care to ensure that we fail in such cases, but there is no guarantee. We should check the cache keys extensively. Cache invalidation is hard (on the other hand: The actions/cache seems to be wildly used).
Github Actions caches are limited to 10GB. If we reach this limit, old cache keys are evicted automatically and we no longer profit from caching. A full autobuild run corrently takes about 2300 MB (with Android taking the vast amount of it). Unless there are lots of pending PRs which touch autobuild-related code, this should cover our use case most of the time. See Build: Use actions/cache #2277 (comment) for further details.

Several refactorings in the build scripts have been necessary to be able to introduce those features. Therefore, they are part of this PR. In order to keep history clean and make review easier, they have been extracted as individual commits:

Define versions and paths as variables
Android: Avoid cleanups, condense calls
Clean up Android Docker leftovers
Use bash consistently

Context: Fixes an issue?

Fixes #2277

Does this change need documentation? What needs to be documented and how?

No.

Status of this Pull Request

Not ready for merge. Should wait for 3.9.0.

What is missing until this pull request can be merged?

Windows Build: Fail workflow early on errors #2276 should be merged first to avoid caching failed builds
Thorough review of the Cache paths and key logic
All officially supported release artifacts should be validated
- Windows
- Windows JACK (?)
- Mac
- Mac Legacy
- Debian/Ubuntu

Checklist

I've verified that this Pull Request follows the general code principles
I tested my code and it does what I want
~~My code follows the style guide~~
I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
I've filled all the content above

hoffie · 2022-01-26T09:55:39Z

.github/workflows/autobuild.yml

+      # Enable caching of downloaded dependencies
+      - name:                       "Cache Mac dependencies"
+        if:                         ${{ matrix.config.target_os == 'macos' }}
+        uses:                       actions/cache@v2
+        with:
+          path: |
+            /usr/local/opt/qt
+            ~/Library/Caches/pip
+          key:                      ${{ matrix.config.target_os }}-${{ hashFiles('.github/workflows/autobuild.yml', 'autobuild/mac/artifacts/autobuild_mac_1_prepare.sh', 'autobuild/mac/codeQL/autobuild_mac_1_prepare.sh') }}-${{ matrix.config.cmd1_prebuild }}
+
+      - name:                       "Cache Windows dependencies"
+        if:                         ${{ matrix.config.target_os == 'windows' }}
+        uses:                       actions/cache@v2
+        with:
+          path: |
+            C:\Qt
+            C:\ChocoCache
+            ~\AppData\Local\pip\Cache
+            ~\windows\NSIS
+            ~\windows\ASIOSDK2
+          key:                      ${{ matrix.config.target_os }}-${{ hashFiles('.github/workflows/autobuild.yml', 'autobuild/windows/autobuild_windowsinstaller_1_prepare.ps1', 'windows/deploy_windows.ps1') }}-${{ matrix.config.cmd1_prebuild }}
+
+      - name:                       "Cache Android dependencies"
+        if:                         ${{ matrix.config.target_os == 'android' }}
+        uses:                       actions/cache@v2
+        with:
+          path: |
+            /opt/Qt
+            /opt/android/android-sdk
+            /opt/android/android-ndk
+          key:                      ${{ matrix.config.target_os }}-${{ hashFiles('.github/workflows/autobuild.yml', 'autobuild/android/autobuild_apk_1_prepare.sh', 'autobuild/android/install-qt.sh') }}-${{ matrix.config.cmd1_prebuild }}
+


I've considered making this a single step and providing the platform-specific differences (path, key) via the matrix config, but that would imply that we would have to duplicate all that per-instance (e.g. once for regular Windows builds, once for JACK; once for regular Mac builds, once for legacy). Therefore I decided to go this route.

pljones

Looks better and has lots green builds so far.

ann0see

Ready to merge? Probably not yet since the artefacts haven’t been tested.

softins

This is looking good

hoffie · 2022-02-04T23:06:51Z

Thanks for the reviews.
While the refactoring stuff is quite safe, getting the cache keys even slightly wrong might result in unexpected build output. Therefore I think it might be better to target that for 3.9.0 and avoid having build issues during 3.8.2 release time?
Let me know if you think otherwise.

hoffie · 2022-02-05T11:06:45Z

What I would feel safe with would be a merge of the refactoring-/pinning-only changes for 3.8.2. In fact, doing so would make future planned refactorings simpler so I'd like to go this path and will open a copy of this PR with just the refactoring parts.
Edit: Did that as #2345.

This commit introduces an actions/cache step for Android, Mac and Windows builds. For Linux, this makes little sense as all dependencies are installed via apt, which should be almost locally anyway (Azure apt mirror). We rely on pinned versions for the relevant external dependencies. The following environments are cached: - Qt installation directory (Windows, Mac, Android) - pip cache (Windows, Mac) - choco cache (Windows) - Android SDK & NDK (Android) The cache is used if the workflow-provided calls (which include Qt versions) are unchanged and if certain files (most notably the files that this commit touches) are unchanged. If anything changes, the cache is not used. Instead, everything starts clean and the result is cached for further reuse again. This is supposed to: - Make builds more deterministic - Speed up builds by avoiding repeated downloads and some extractions - Increase resilience against temporary external outages (e.g. Qt mirror problems)

ann0see · 2022-02-20T09:28:52Z

Should be merged after upcoming release (not squash merged)

ann0see · 2022-02-20T18:35:39Z

@hoffie are you ok with a merge (and test during our development cycle) or is anything blocking (i.e. caching of create-dmg, the PR I merged now)

hoffie · 2022-02-20T20:49:29Z

@hoffie are you ok with a merge (and test during our development cycle)

Yes, I think that'd be best.

is anything blocking (i.e. caching of create-dmg, the PR I merged now)

Should not be blocking. Brew is not explicitly part of the cache in this PR, but that's not an issue. We can extend it in a follow-up. I neither want to sneak this into this already approved PR nor go another round of approvals here. Will open a follow-up.

See #2412 for tracking.

hoffie commented Jan 26, 2022

View reviewed changes

hoffie force-pushed the cache-autobuild-deps branch 2 times, most recently from f12c5b5 to 91d47e2 Compare January 30, 2022 14:05

hoffie force-pushed the cache-autobuild-deps branch from 91d47e2 to e8c05a4 Compare February 2, 2022 20:29

pljones approved these changes Feb 2, 2022

View reviewed changes

ann0see approved these changes Feb 3, 2022

View reviewed changes

ann0see marked this pull request as draft February 3, 2022 06:50

softins approved these changes Feb 3, 2022

View reviewed changes

hoffie added this to the Release 3.9.0 milestone Feb 4, 2022

hoffie mentioned this pull request Feb 5, 2022

Autobuild: Refactor and add dependency pinning #2345

Merged

8 tasks

hoffie force-pushed the cache-autobuild-deps branch from e8c05a4 to 82761fc Compare February 5, 2022 18:28

ann0see marked this pull request as ready for review February 20, 2022 09:28

hoffie merged commit c6f63b9 into jamulussoftware:master Feb 20, 2022

hoffie mentioned this pull request Feb 20, 2022

Build: Cache Mac's brew install (create-dmg) #2412

Closed

2 tasks

hoffie deleted the cache-autobuild-deps branch February 20, 2022 20:59

pljones mentioned this pull request Feb 20, 2022

Build: Use actions/cache #2277

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autobuild: Refactor & use actions/cache #2284

Autobuild: Refactor & use actions/cache #2284

hoffie commented Jan 26, 2022 •

edited

Loading

hoffie Jan 26, 2022 •

edited

Loading

pljones left a comment

ann0see left a comment •

edited

Loading

softins left a comment

hoffie commented Feb 4, 2022

hoffie commented Feb 5, 2022 •

edited

Loading

ann0see commented Feb 20, 2022

ann0see commented Feb 20, 2022 •

edited

Loading

hoffie commented Feb 20, 2022 •

edited

Loading

Autobuild: Refactor & use actions/cache #2284

Autobuild: Refactor & use actions/cache #2284

Conversation

hoffie commented Jan 26, 2022 • edited Loading

Checklist

hoffie Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

pljones left a comment

Choose a reason for hiding this comment

ann0see left a comment • edited Loading

Choose a reason for hiding this comment

softins left a comment

Choose a reason for hiding this comment

hoffie commented Feb 4, 2022

hoffie commented Feb 5, 2022 • edited Loading

ann0see commented Feb 20, 2022

ann0see commented Feb 20, 2022 • edited Loading

hoffie commented Feb 20, 2022 • edited Loading

hoffie commented Jan 26, 2022 •

edited

Loading

hoffie Jan 26, 2022 •

edited

Loading

ann0see left a comment •

edited

Loading

hoffie commented Feb 5, 2022 •

edited

Loading

ann0see commented Feb 20, 2022 •

edited

Loading

hoffie commented Feb 20, 2022 •

edited

Loading