Closed
Replace vendored sphinx-tabs in envoy.docs.sphinx_runner with upstream PyPI dependency#4433
sphinx-tabs in envoy.docs.sphinx_runner with upstream PyPI dependency#4433Conversation
✅ Deploy Preview for nifty-bassi-e26446 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Agent-Logs-Url: https://github.com/envoyproxy/toolshed/sessions/1e2878b3-5697-4d6c-b57c-4d53e48a4c76 Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Remove vendored sphinx-tabs and update to v3.5.0
Replace vendored May 11, 2026
sphinx-tabs in envoy.docs.sphinx_runner with upstream PyPI dependency
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
envoy.docs.sphinx_runnerwas still shipping a vendoredsphinx-tabs(v3.4.1) that existed to work around an upstreamdocutilspin. That upstream issue has long been resolved, so this change removes the vendored copy and switches the package to upstreamsphinx-tabs(>=3.5.0).Dependency wiring (
sphinx_runner)/py/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/BUILD:./sphinx_tabs//py/deps:reqs#sphinx-tabs(grouped with other Sphinx deps)Remove vendored extension
/py/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/sphinx_tabs/entirely:Promote to canonical Python deps
sphinx-tabs>=3.5.0to/py/deps/requirements.inReference cleanup
envoy.docs.sphinx_runner.sphinx_tabs,from .sphinx_tabs,./sphinx_tabs) and removed remaining usage by switching dependency ownership to the PyPI packageOriginal prompt
Background
The
envoy.docs.sphinx_runnerpackage contains a vendored copy ofsphinx-tabsat version3.4.1under:The vendored README explicitly documents why and when it can be removed:
Issue executablebooks/sphinx-tabs#171 (a strict
docutils~=0.18.0pin that conflicted with sphinx / sphinx_rtd_theme / doc8 etc.) was closed as completed on 2023-12-26 and the fix shipped in sphinx-tabs v3.4.5 (Jan 2024) via PR #186 "FIX: unpin docutils".The latest upstream release at the time of writing is v3.5.0 (March 2026), which also includes Sphinx 8.1 / Docutils 0.22 compatibility fixes and a fix to drop usage of internal Sphinx API (PR #198 in v3.4.6). The vendored copy at 3.4.1 is missing ~4 years of upstream improvements.
Task
Remove the vendored
sphinx-tabsand switch to the upstream PyPI package.Changes required
Delete the vendored directory (entire tree):
py/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/sphinx_tabs/__init__.pypy/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/sphinx_tabs/tabs.pypy/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/sphinx_tabs/README.mdpy/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/sphinx_tabs/BUILDpy/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/sphinx_tabs/static/tabs.csspy/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/sphinx_tabs/static/tabs.jsUpdate
py/envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/BUILD:"./sphinx_tabs"dependency."//py/deps:reqs#sphinx-tabs"in its place (alphabetically sorted alongside the othersphinx*reqs).Current relevant snippet:
Add
sphinx-tabsto the Python requirements inpy/deps/(or wherever the canonicalrequirements.in/requirements.txtlives for this workspace — match the pattern used bysphinx-copybutton,sphinx-rtd-theme, etc.). Use a sensible lower bound such as>=3.5.0(this is the version that contains both the docutils fix and the Sphinx 8.1 / Docutils 0.22 compatibility fixes). Then regenerate any lock files using the standard process used in this repo.Search the codebase for any internal references to
envoy.docs.sphinx_runner.sphinx_tabs(e.g. as a Sphinx extension entry, imports inrunner.py,cmd.py,ext/*.py, or in tests underpy/envoy.docs.sphinx_runner/) and update them to refer to the upstreamsphinx_tabsextension instead. The upstream extension is registered assphinx_tabs.tabs.Verify nothing else in the repo imports from the vendored path. A grep for
sphinx_runner.sphinx_tabs,from .sphinx_tabs, andimport sphinx_tabsshould be clean (or only reference the upstream package after the switch).Verification
bazel/Pants test invocation used by CI forpy/envoy.docs.sphinx_runner).sphinx-tabsresolves in the dependency lockfile alongside the existingsphinx,sphinx-rtd-theme,docutils, etc., with no version conflicts.Notes
v3.4.1(the BUILD file disables flake8/mypy/docformatter, consistent with "do not touch vendored code"). No Envoy-specific patches need to be preserved.envoyproxy/envoydocs/conf.py) may reference the extension by its current import path. That is out of scope for this PR — any required follow-up will be done in the consumer repo. Just make sure this PR cleanly publishes the package without the vendored module.The following is the prior conversation context from the user's chat exploration (may be truncated):
User: its been brought to my attention that the py package envoy.docs.sphinx_runner package has a vendored sphinx-tabs
pretty sure there was a reason why this was vendored originally - not sure at all there is any reason to keep - other bot said they could not see one - but want to make sure
Assistant: Fo...
This pull request was created from Copilot chat.