Skip to content

setup-environment: fix standard-mode conda caching + validation (#33) - #78

Merged
mmcky merged 2 commits into
mainfrom
fix/setup-env-standard-cache
Jun 16, 2026
Merged

mmcky merged 2 commits into
mainfrom
fix/setup-env-standard-cache

Conversation

@mmcky

@mmcky mmcky commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Closes #33 (C3, L19, L23) — makes the non-container (standard-mode) Conda cache actually save time, plus two smaller hardening/doc fixes.

C3 — the cache was a no-op

Before, standard mode restored the Conda cache and then ran setup-miniconda with environment-file: set, which recreates the env every run — so the cache never helped (the README's "~3-4 min cached" was aspirational).

Restructured to the documented setup-miniconda caching recipe:

  1. setup-miniconda runs without environment-file (installs conda + the named env shell, doesn't rebuild from the file);
  2. actions/cache restores the env at ${CONDA}/envs;
  3. a new step runs conda env update only on a cache miss (cache-hit != 'true', which also covers prefix-matched restores when the env file changed).

Also:

  • dropped the deprecated use-only-tar-bz2;
  • dropped miniconda-version: latest so the env lives at a stable ${CONDA}/envs path (the runner's pre-installed conda, kept current by auto-update-conda) — this is what makes the cache path reliable.

L19 — fail loudly on missing LaTeX requirements

install-latex: true with a missing requirements file now errors and exits instead of warning + skipping (which produced an env that failed ~an hour later in the build).

L23 — cache-version doc

Documented that cache-version only affects the standard-mode Conda cache (no effect in container mode, which has no Conda cache).

⚠️ Validation note

Standard mode is a fallback (the templates are container-first), and conda env caching is finicky and can't be verified locally. Worth confirming on a real CI run: the second run on an unchanged environment.yml should log a cache hit and skip the "Create/update environment" step. If the ${CONDA}/envs path turns out wrong on the hosted runner, the fix is a one-line path tweak.

Closes #33

🤖 Generated with Claude Code

- C3: in standard (non-container) mode the conda cache was restored and then
  setup-miniconda recreated the env unconditionally (environment-file set), so
  the cache never saved time. Restructured to the documented setup-miniconda
  caching recipe: setup-miniconda runs without environment-file, the env is
  restored from cache (${CONDA}/envs), and `conda env update` runs only on a
  cache miss. Dropped the deprecated use-only-tar-bz2; dropped miniconda-version
  so the env lives at a stable ${CONDA}/envs path (kept current via
  auto-update-conda).
- L19: install-latex with a missing requirements file now errors and exits
  instead of warning and silently producing a broken env.
- L23: cache-version input documents it is standard-mode only (no effect in
  container mode).

Standard mode is a fallback (templates are container-first); the cache hit
should be confirmed on a real CI run (second run should log a hit and skip
env creation).

Closes #33

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 16, 2026 07:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the setup-environment composite action to make standard (non-container) Conda caching effective, hardens LaTeX requirements handling, and documents the cache-version input’s scope.

Changes:

  • Reworks standard-mode Conda setup to restore ${CONDA}/envs from cache and only update/create the env when the primary cache key doesn’t hit.
  • Fails fast when install-latex: true but the LaTeX requirements file is missing.
  • Documents that cache-version only affects the standard-mode Conda cache (no effect in container mode) and records the behavior in the changelog.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
setup-environment/action.yml Reorders standard-mode Conda setup and caching; adds cache-miss update step; makes missing LaTeX requirements an error; clarifies cache-version input description.
CHANGELOG.md Adds an Unreleased entry describing the setup-environment caching/validation/doc changes.

Comment thread setup-environment/action.yml
Comment thread setup-environment/action.yml Outdated
Comment thread CHANGELOG.md Outdated
- Cache key now includes environment-name + python-version (changing either
  no longer reuses a cache built for a different env/Python); restore-keys
  tightened to the same name+python (dropped the too-broad conda-{os}- fallback).
- Validate environment.yml exists in standard mode (fail fast) so a missing
  file can't be masked by a cache hit.
- Renamed the update step ('no exact cache hit' — it also runs on a restore-keys
  partial match, not only a true miss) and added --prune so deps dropped from
  environment.yml don't linger in an updated cached env.
- CHANGELOG wording corrected to match (no exact cache hit, not 'cache miss').

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mmcky
mmcky merged commit 2997df3 into main Jun 16, 2026
@mmcky
mmcky deleted the fix/setup-env-standard-cache branch June 16, 2026 09:31
mmcky added a commit that referenced this pull request Jun 16, 2026
Pin third-party actions to full commit SHAs (with a `# vN` comment) so a
hijacked upstream tag can't inject code into our workflows (cf. the
tj-actions/changed-files compromise):
- docker/login-action, docker/setup-buildx-action, docker/metadata-action,
  docker/build-push-action  (build-containers.yml, test-container.yml)
- softprops/action-gh-release  (publish-gh-pages)
- conda-incubator/setup-miniconda  (setup-environment)

First-party actions/* are left on major tags (GitHub-maintained, per GitHub's
guidance). Dependabot's github-actions ecosystem keeps the SHA pins current.

Note: conda-incubator/setup-miniconda is in setup-environment/action.yml, which
open PR #78 (#33) also edits — whichever merges second needs a trivial rebase.

Closes #39

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Jun 16, 2026
Pin third-party actions to full commit SHAs (with a `# vN` comment) so a
hijacked upstream tag can't inject code into our workflows (cf. the
tj-actions/changed-files compromise):
- docker/login-action, docker/setup-buildx-action, docker/metadata-action,
  docker/build-push-action  (build-containers.yml, test-container.yml)
- softprops/action-gh-release  (publish-gh-pages)
- conda-incubator/setup-miniconda  (setup-environment)

First-party actions/* are left on major tags (GitHub-maintained, per GitHub's
guidance). Dependabot's github-actions ecosystem keeps the SHA pins current.

Note: conda-incubator/setup-miniconda is in setup-environment/action.yml, which
open PR #78 (#33) also edits — whichever merges second needs a trivial rebase.

Closes #39

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Jul 30, 2026
* ci: add action-level PR harness for cache and environment actions

First stage of #100. The action logic shipped with no automated tests
— restore-jupyter-cache's save mode was dead on arrival in v0.8.0
(#104) and survived eight months because nothing in CI exercised it.

Adds .github/workflows/test-actions.yml, which runs the actions via ./
local paths (the PR's code, not a released ref) against per-run salted
fixtures, asserting on outputs and the filesystem:

- restore-jupyter-cache: both cache types x both modes, fail-on-miss
  firing and staying quiet, payload round-trips
- setup-environment: standard-mode conda cache two-run miss->hit chain
  (PLAN item 9 — the #33/#78 path CI had never confirmed)
- build-lectures: real executed HTML build on the cache-restored env
- build-jupyter-cache: smoke build, then a full round-trip into
  restore-jupyter-cache for both cache types

The committed .github/fixtures/mini-lectures fixture executes a real
code cell so _build/.jupyter_cache is genuinely populated — the
container test book builds with execution off and cannot.

Not covered, by construction: build-jupyter-cache's internal @v0
sibling calls (GitHub forbids expressions in uses:) and the
publish/preview actions (need real deploy targets). Both belong to the
post-release canary proposed in #100.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(setup-environment): cache the env dir, not the root-owned envs parent

Caught by the harness on its first run. The standard-mode conda cache
was rooted at ${CONDA}/envs, whose parent is root-owned on hosted
runners: the save works, but every restore dies in tar (Cannot
utime/chmod on envs) and actions/cache reports a miss — so the cache
has never restored, every run pays the full solve, and a partial
extraction can leave a mangled env behind for conda env update to
patch. Root the cache at the runner-owned envs/<environment-name>
instead, which round-trips cleanly.

Old-path caches become unreachable (path is part of the cache
version); the first run after this re-saves under the new path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: TEMP diagnostic job for jb silent exit 1 on runner

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: TEMP instrument build-lectures exit path and composite context

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: TEMP probe SHLVL/logout in both step contexts

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(build-lectures): stop the exit builtin from tripping bash_logout

Second defect caught by the harness. The build step runs in a login
shell (bash -l, needed for conda activation) and ended with
exit $BUILD_EXIT_CODE. On hosted runners the exit builtin triggers
~/.bash_logout processing at SHLVL=1, where Ubuntu's clear_console
fails on a headless runner and its status overrides the one passed to
exit — a successful jb build printed its banner and the step still
returned 1. Verified by instrumentation: "jb exit code: 0",
"reached exit, code: 0", then "Process completed with exit code 1".

Return the status by ending the script instead of calling exit.
Containers are unaffected (root has no ~/.bash_logout), which is why
production lecture builds never surfaced this.

The harness's bjc-smoke job removes ~/.bash_logout as a temporary
workaround because build-jupyter-cache's internal build-lectures@v0
still carries the old exit pattern until the v0 tag moves.

Also removes the temporary diagnostic job and instrumentation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(contributing): add a release step to drop obsolete @v0 workarounds

The harness's rm -f ~/.bash_logout step in bjc-smoke only exists because
build-jupyter-cache's internal build-lectures@v0 still carries the exit
builtin pattern, and that copy cannot pick up the fix until the floating
v0 tag moves. Nothing but a code comment recorded that it must then go,
so put the reminder where the tag actually moves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(harness): assert a failing build fails the step

env-hit-build covers only the success direction of build-lectures. The
status-propagation fix in this branch replaces the exit builtin with a
trailing (exit $BUILD_EXIT_CODE); dropping that line, or appending any
command after it, would discard the build status and turn every broken
consumer build into a silent pass with the whole harness still green.

build-fail-guard derives a broken page from the good fixture (so the
failure comes from the raised AssertionError, not a malformed document),
builds it under continue-on-error, and asserts the step outcome is
failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setup-environment: fix standard-mode conda caching and validation

2 participants