CI: cache microsoft fonts by adamwitmer · Pull Request #35 · lanl/ldrd_neat_ml

adamwitmer · 2026-06-09T21:50:34Z

Fixes BUG: intermittent test-suite failures due to step install fonts #22
add GitHub caching for font installation to reduce intermittent errors in downloading fonts that cause CI failure because of absence of Arial type font for plot generation

AI Disclosure: AI was used to help write the changes in this PR, specifically Claude prototyped the code for me based on the prompt to cache the downloaded fonts and then helped revised the code to avoid the bug that greptile found. I read docs such as https://github.com/marketplace/actions/cache and watched this video (https://www.youtube.com/watch?v=BDQivAobxKA) to better understand how the github actions performs caching

greptile-apps · 2026-06-09T21:53:03Z

_{Reviews (1): Last reviewed commit: "CI: cache microsoft fonts" | Re-trigger Greptile}

* store cached files in user-writable path and copy to root-owned directory * add step to separate font install via download vs. from cache

greptile-apps · 2026-06-09T22:24:49Z

_{Reviews (2): Last reviewed commit: "MAINT, CI: address AI reviewer comment" | Re-trigger Greptile}

tylerjereddy · 2026-06-09T22:36:24Z

If you use AI in your original commit(s), please disclose (i.e., AI/no AI), thanks.

tylerjereddy · 2026-06-09T22:48:49Z

AI Disclosure: AI was used to help write the changes in this PR

Should give a little more detail -- like "Claude wrote most of the code in this PR and I checked it myself by doing X" or something like that.

tylerjereddy · 2026-06-09T22:51:58Z

The point is that you're responsible for understanding the code and not just dumping it on the reviewer, so I'll use those statements to make sure the LLM code isn't just being thrown at me without understanding (otherwise it'll be sent back for revision/closed).

tylerjereddy · 2026-06-09T22:53:23Z

Maybe communicate how you know this is working--did you test it on your fork? It is clear from the log that it is working? Make that clear to the reviewer, otherwise mark it as WIP until it is confidently ready for review.

adamwitmer · 2026-06-09T23:00:23Z

To check that this is working, I allowed the CI to run here for the intial install fonts step to run on linux runners, and then re-ran all the jobs to check that the cache fonts and install fonts from cache steps ran using the cached fonts from the previous CI run, i.e.

cache fonts:

Run actions/cache@v5
Cache hit for: Linux-msttcorefonts-v1
Received 5088242 of 5088242 (100.0%), 31.5 MBs/sec
Cache Size: ~5 MB (5088242 B)
/usr/bin/tar -xf /home/runner/work/_temp/0c3dcb1f-0add-4aab-b5d0-0b742e9476ad/cache.tzst -P -C /home/runner/work/ldrd_neat_ml/ldrd_neat_ml --use-compress-program unzstd
Cache restored successfully
Cache restored from key: Linux-msttcorefonts-v1

install fonts from cache:

sudo mkdir -p /usr/share/fonts/truetype/msttcorefonts/
  sudo cp ~/.cache/msttcorefonts/*.ttf /usr/share/fonts/truetype/msttcorefonts/
  shell: /usr/bin/bash -e {0}
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.14.5/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.14.5/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.14.5/x64
    Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.14.5/x64
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.14.5/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.14.5/x64/lib

tylerjereddy · 2026-06-10T22:59:24Z

+    - name: cache fonts
+      id: cache-fonts
      if: runner.os == 'Linux'
+      uses: actions/cache@v5


It is best practice to use the hash rather than the release tag for security reasons (tags are often mutable), followed by a short comment with the tag/version number.

As an example of how to do this, see pandas CI config here:
https://github.com/pandas-dev/pandas/blob/main/.github/workflows/wheels.yml#L70

The rest of this seems "ok" now.

I modified the cache fonts step to use the github/actions/cache hash instead of the version tag. I cloned the github actions repo and checked out v5.0.5 to find the commit hash associated with the version.

As noted during in person conversation, I didnt check that the version tag and hash number were aligned in the CI file. I have updated the comment to reference the correct tag (v5.0.5).

greptile-apps · 2026-06-11T15:18:09Z

_{Reviews (3): Last reviewed commit: "CI: use github actions cache hash" | Re-trigger Greptile}

* change from incorrect `v5` to correct hash `v5.0.5`

greptile-apps · 2026-06-11T16:09:12Z

_{Reviews (4): Last reviewed commit: "MAINT: fix hash version comment" | Re-trigger Greptile}

greptile-apps · 2026-06-11T16:09:16Z

      run: |
        echo ttf-mscorefonts-installer msttcorefonts/accepted-mscorefonts-eula select true | sudo debconf-set-selections
        sudo apt-get update
        sudo apt-get install -y ttf-mscorefonts-installer || true
-        sudo fc-cache -f -v
+        mkdir -p ~/.cache/msttcorefonts/
+        cp /usr/share/fonts/truetype/msttcorefonts/*.ttf ~/.cache/msttcorefonts/


Empty cache saved on apt install failure creates persistent failure loop

If ttf-mscorefonts-installer fails to download fonts (exactly the intermittent failure this PR is fixing), mkdir -p ~/.cache/msttcorefonts/ still succeeds and creates an empty directory. The subsequent cp then fails, but actions/cache's post-job step runs regardless of job status and saves the empty ~/.cache/msttcorefonts/ directory under the key runner.os-msttcorefonts-v1. On every subsequent run the cache is a hit and the "install fonts from cache" step runs — sudo cp ~/.cache/msttcorefonts/*.ttf ... glob-expands to nothing, the step fails, and tests never run. The only recovery is a manual cache key bump.

A minimal fix is to guard the cp so it either exits cleanly without saving an empty cache (e.g. check that .ttf files exist first) or to avoid caching on failure.

tylerjereddy · 2026-06-11T22:02:22Z

I suppose the LLM comment at #35 (comment) is valid. GitHub evicts unused caches after 7 days (based on official docs: #35 (comment)), so you could get stuck in a failure loop for the Linux jobs if you don't submit PRs for a while.

I wonder how much better your solution is vs. i.e., https://github.com/lanl/ldrd_neat_ml_images approach to reliable asset downloads. This logic is getting a bit complex, but on the other hand perhaps there are licensing issues to public hosting of the Microsoft font assets.

adamwitmer · 2026-06-11T22:20:02Z

I tested the following diff on my forked branch to see if we can gate the cache dir generation on checking that the required fonts (Arial and Arial Bold) are downloaded correctly (I based this solution off of the response here: https://stackoverflow.com/a/75948306):

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 86885da..6492f33 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -38,8 +38,18 @@ jobs:
         echo ttf-mscorefonts-installer msttcorefonts/accepted-mscorefonts-eula select true | sudo debconf-set-selections
         sudo apt-get update
         sudo apt-get install -y ttf-mscorefonts-installer || true
-        mkdir -p ~/.cache/msttcorefonts/
-        cp /usr/share/fonts/truetype/msttcorefonts/*.ttf ~/.cache/msttcorefonts/
+        # before making the user storage directory, check if the font(s) we need (`Arial.ttf` and `Arial_Bold.ttf`)
+        # exist/were successfully downloaded to the root directory to avoid loading an empty directory on a successful
+        # ``cache-hit``
+        if [[ ! -f "/usr/share/fonts/truetype/msttcorefonts/Arial.ttf" || \
+            ! -f "/usr/share/fonts/truetype/msttcorefonts/Arial_Bold.ttf" ]]; then
+            echo "Fonts not downloaded successfully."
+            exit 1
+        else
+            echo "Fonts downloaded successfully. Caching fonts..."
+            mkdir -p ~/.cache/msttcorefonts/
+            cp /usr/share/fonts/truetype/msttcorefonts/*.ttf ~/.cache/msttcorefonts/
+        fi

I performed the following steps to test the CI logic:

First, I commented out the apt-get lines to force a failure (https://github.com/adamwitmer/ldrd_neat_ml/actions/runs/27378614879/job/80909285199).
Then I tested downloading the fonts and storing them in the user cache (https://github.com/adamwitmer/ldrd_neat_ml/actions/runs/27378947386/job/80912048860) which ran successfully.
Then I pushed an empty commit to re-trigger the pipeline and test that the fonts are installed from the cache when available (https://github.com/adamwitmer/ldrd_neat_ml/actions/runs/27379981094/job/80913935208), which performed as expected.

tylerjereddy · 2026-06-11T23:21:59Z

seems sensible

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

tylerjereddy · 2026-06-11T23:51:27Z

ok, this looks fine now, thanks

CI: cache microsoft fonts

8de225b

greptile-apps Bot reviewed Jun 9, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml Outdated

MAINT, CI: address AI reviewer comment

815cfb9

* store cached files in user-writable path and copy to root-owned directory * add step to separate font install via download vs. from cache

tylerjereddy added the CI label Jun 10, 2026

tylerjereddy reviewed Jun 10, 2026

View reviewed changes

CI: use github actions cache hash

864a596

MAINT: fix hash version comment

b5d74b9

* change from incorrect `v5` to correct hash `v5.0.5`

greptile-apps Bot reviewed Jun 11, 2026

View reviewed changes

CI: modify cache logic to check for successful download

189da6d

greptile-apps Bot reviewed Jun 11, 2026

View reviewed changes

tylerjereddy merged commit 62d2ac7 into main Jun 11, 2026
8 checks passed

tylerjereddy deleted the awitmer_issue_22 branch June 11, 2026 23:51

Conversation

adamwitmer commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 9, 2026

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 9, 2026

Uh oh!

tylerjereddy commented Jun 9, 2026

Uh oh!

tylerjereddy commented Jun 9, 2026

Uh oh!

tylerjereddy commented Jun 9, 2026

Uh oh!

tylerjereddy commented Jun 9, 2026

Uh oh!

adamwitmer commented Jun 9, 2026

Uh oh!

tylerjereddy Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

adamwitmer Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

adamwitmer Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Jun 11, 2026

Uh oh!

greptile-apps Bot commented Jun 11, 2026

Uh oh!

greptile-apps Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

tylerjereddy commented Jun 11, 2026

Uh oh!

adamwitmer commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tylerjereddy commented Jun 11, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

tylerjereddy commented Jun 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adamwitmer commented Jun 9, 2026 •

edited

Loading

adamwitmer commented Jun 11, 2026 •

edited

Loading