Skip to content

reset mkdocs-material before applying patch - #275

Open
JacobBarthelmeh wants to merge 1 commit into
wolfSSL:masterfrom
JacobBarthelmeh:ja
Open

reset mkdocs-material before applying patch#275
JacobBarthelmeh wants to merge 1 commit into
wolfSSL:masterfrom
JacobBarthelmeh:ja

Conversation

@JacobBarthelmeh

Copy link
Copy Markdown
Contributor

No description provided.

@JacobBarthelmeh JacobBarthelmeh self-assigned this Aug 3, 2026
Copilot AI review requested due to automatic review settings August 3, 2026 17:28

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 ensures the mkdocs-material submodule’s theme files are reset to a clean state before re-applying the language-specific patch, preventing cross-language builds (EN vs JA) from leaving incorrect patched navigation links behind.

Changes:

  • Add a git checkout -- step in html-setup to revert patched theme files prior to applying the next patch.
  • Document why the reset is needed (EN/JA patches touch the same header lines).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yosuke-wolfssl yosuke-wolfssl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, it looks good to me but I commented one suggestion below.
Besides that, I found wolfHSM/Makefile:78 needs the same change. wolfHSM overrides html-setup and duplicates these lines (there's a NOTE above it asking for manual sync), so it isn't covered by the common.am fix:

-	$(Q)git submodule update --init
+	$(Q)git submodule --quiet update --init --force

Verified with the real wolfHSM docs repo: with only common.am fixed, a JA build after an EN build still produces wolfssl.com nav in the Japanese manual. With both fixed, alternating DOC_LANG=JA make html-setup / make html-setup gives the right nav every time. It's the only override in the repo.

Longer term the duplication itself is worth removing — html-setup differs from common.am only in $(HOMEPAGE) vs $(word 1,$(SOURCES)), and pdf-setup only in the perl pattern, so both could become ?= defaults with wolfHSM keeping just html-setup pdf-setup: wolfhsm-update. Fine as a follow-up.

Comment thread common/common.am
# Revert any theme patch from a previous build. The EN and JA patches touch the
# same lines of header.html, so without this an EN build in a tree last used for
# a JA build (or vice versa) keeps the wrong language's nav links.
$(Q)cd ../mkdocs-material; git checkout -- material/base.html material/partials/header.html

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I confirmed the bug reproduces and this fixes it. Thanks.
I have one suggestion on it: --force instead of the explicit file list**

git submodule update --force runs checkout --force in the submodule, which reverts the patch for any file it touched, so the two-line change collapses into a modification of the existing line:

	$(Q)git submodule --quiet update --init --force

The hardcoded base.html / header.html list is correct today, but it has to be kept in sync with the patches by hand — add a third file to a future theme patch and this silently goes stale. --quiet keeps the build output as it is now.

Checked against the real submodule: no fetch needed when the pinned SHA is already local, first-time init unchanged, and it's slightly faster than the checkout -- form on a warm tree. Only behavioral difference is that it also reverts unrelated hand-edits inside mkdocs-material, which the patch files are the sanctioned alternative to anyway.

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.

4 participants