Skip to content

MDBook download + cache speedup - #14

Merged
cassidyjames merged 2 commits into
mainfrom
mdbook-download-and-cache
Feb 9, 2026
Merged

MDBook download + cache speedup#14
cassidyjames merged 2 commits into
mainfrom
mdbook-download-and-cache

Conversation

@juanmrad

@juanmrad juanmrad commented Feb 8, 2026

Copy link
Copy Markdown
Member

Context & Requests for Reviewers

Fixes #3

This Action changes from building mdbook to downloading the release and caching it. Should speed up the process of our action by looking from cache or downloading the release over building from source.

vinaysrao1
vinaysrao1 requested changes Feb 8, 2026
View reviewed changes
Comment thread .github/workflows/mdbook.yml
curl --proto '=https' --tlsv1.2 https://sh.rustup.rs -sSf -y | sh
rustup update
cargo install --version ${MDBOOK_VERSION} mdbook
ARCH="x86_64-unknown-linux-gnu"

@vinaysrao1 vinaysrao1 Feb 8, 2026

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.

I am on mac - is this too limiting

@juanmrad juanmrad Feb 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This only runs on CI in an ubuntu runner.

@vinaysrao1 vinaysrao1 Feb 9, 2026

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.

OK.

Comment thread .github/workflows/mdbook.yml
URL="https://github.com/rust-lang/mdBook/releases/download/v${MDBOOK_VERSION}/${ARCHIVE}"
mkdir -p mdbook-bin
curl -sSL -o mdbook.tar.gz "$URL"
tar -xzf mdbook.tar.gz -C mdbook-bin --strip-components=1

@vinaysrao1 vinaysrao1 Feb 8, 2026

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.

I just looked up --strip-components command. looks like it is stripping leading directories in the path. making sure that mdbook tarball has leading directory names. (I could be wrong)

@juanmrad juanmrad Feb 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

just doubled checked, and yes the released tarballs have one top-level dir with the binary inside.

@cassidyjames

cassidyjames commented Feb 9, 2026

Copy link
Copy Markdown
Member

At a glance I think this will fail because we're downloading and caching mdbook-bin but then calling mdbook. I'm on mobile at the moment and can more fully review it on a desktop later today.

cassidyjames
cassidyjames approved these changes Feb 9, 2026
View reviewed changes

@cassidyjames cassidyjames left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After checking it out on desktop I saw what was happening: we're caching the mdbook-bin directory itself which has the mdbook binary within. Adding it to the path should have worked as well, but calling the binary directly should also work.

I'm happy to merge this as-is and then we can follow-up to make sure the caching is behaving as expected in GitHub Actions. 👍🏻

@cassidyjames
cassidyjames merged commit ab9c8b0 into main Feb 9, 2026
8 checks passed
@cassidyjames
cassidyjames deleted the mdbook-download-and-cache branch February 9, 2026 21:56
@cassidyjames cassidyjames mentioned this pull request Feb 9, 2026
Closed
@cassidyjames cassidyjames added this to the v0 milestone Feb 9, 2026
taobojlen added a commit to taobojlen/coop that referenced this pull request Jun 8, 2026
@taobojlen @claude
mdbook: drop broken cache step, install binary unconditionally
6cdb7e8 
zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR roostorg#14 / issue roostorg#3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@taobojlen taobojlen mentioned this pull request Jun 8, 2026
Merged
taobojlen added a commit to taobojlen/coop that referenced this pull request Jun 8, 2026
@taobojlen @claude
mdbook: drop broken cache step, install binary unconditionally
c3ed221 
zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR roostorg#14 / issue roostorg#3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
taobojlen added a commit to taobojlen/coop that referenced this pull request Jun 9, 2026
@taobojlen @claude
mdbook: drop broken cache step, install binary unconditionally
3a78911 
zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR roostorg#14 / issue roostorg#3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
taobojlen added a commit to taobojlen/coop that referenced this pull request Jun 9, 2026
@taobojlen @claude
mdbook: drop broken cache step, install binary unconditionally
90ca40d 
zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR roostorg#14 / issue roostorg#3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
taobojlen added a commit to taobojlen/coop that referenced this pull request Jun 12, 2026
@taobojlen @claude
mdbook: drop broken cache step, install binary unconditionally
be5c3e4 
zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR roostorg#14 / issue roostorg#3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
taobojlen added a commit to taobojlen/coop that referenced this pull request Jun 15, 2026
@taobojlen @claude
mdbook: drop broken cache step, install binary unconditionally
9635d84 
zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR roostorg#14 / issue roostorg#3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
taobojlen added a commit that referenced this pull request Jun 15, 2026
@taobojlen @claude
ci: use zizmor to lint for CI security issues (#721)
f396968 
* Harden GitHub Actions workflows per zizmor audit

Fix zizmor findings across the workflows:
- Move `github.base_ref`/`inputs.ref`/step-output expressions out of
  `run:` blocks into `env:` vars to avoid template injection.
- Scope `checks: write` down from the workflow level to only the
  `check_api_server` job in apply_pr_checks.yaml.
- Add `persist-credentials: false` to checkout steps.
- Add a zizmor CI workflow to run the audit on push and PRs.

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

* mdbook: drop broken cache step, install binary unconditionally

zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR #14 / issue #3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

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

* address PR comments

* ignore dependabot cooldowns in zizmor

* add dependency cooldown to dependabot

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cassidyjames pushed a commit that referenced this pull request Jun 24, 2026
@taobojlen @claude @cassidyjames
ci: use zizmor to lint for CI security issues (#721)
decc4b2 
* Harden GitHub Actions workflows per zizmor audit

Fix zizmor findings across the workflows:
- Move `github.base_ref`/`inputs.ref`/step-output expressions out of
  `run:` blocks into `env:` vars to avoid template injection.
- Scope `checks: write` down from the workflow level to only the
  `check_api_server` job in apply_pr_checks.yaml.
- Add `persist-credentials: false` to checkout steps.
- Add a zizmor CI workflow to run the audit on push and PRs.

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

* mdbook: drop broken cache step, install binary unconditionally

zizmor's cache-poisoning auto-fix added `lookup-only: true` to the
mdbook cache step. Because the install step was gated on
`cache-hit != 'true'`, a cache hit then skipped both the download and
the install, leaving mdbook-bin/ empty and breaking the build.

The original win (PR #14 / issue #3) was switching from compiling
mdbook from source to downloading the prebuilt release; the cache was
only a marginal optimization on top. Removing it keeps that win,
fixes the break, and resolves the cache-poisoning finding cleanly.

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

* address PR comments

* ignore dependabot cooldowns in zizmor

* add dependency cooldown to dependabot

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cassidyjames cassidyjames cassidyjames approved these changes
@vinaysrao1 vinaysrao1 vinaysrao1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v0

Development

Successfully merging this pull request may close these issues.

Speed up mdbook action

3 participants

@juanmrad @cassidyjames @vinaysrao1