Circular dependencies between Sphinx and sphinxcontrib-*

[bruno-digitbio]

Describe the bug

I'm not sure if this is something that the Sphinx team even is interested in explicitly supporting, but the latest release of the sphinxcontrib tooling cannot be used with rules_python (i.e. by users that use Bazel to install Sphinx).

We build our monorepo's docs at work with Sphinx, and today when running some automated tooling to bump package versions, I encountered issues like the following:

$ bazel build //...
ERROR: /home/bruno/.cache/bazel/_bazel_bruno/8f2b6fc37214e29f3e0fada1717a9254/external/pip_sphinx/BUILD.bazel:22:11: in py_library rule @pip_sphinx//:pkg: cycle in dependency graph:
    //building/python/sphinx:build_wrapper (ebf05eacb2537ac9057c50f99af724bc4fbe8ed0ce57ceeb546c8f679dec84e5)
.-> @pip_sphinx//:pkg (ebf05eacb2537ac9057c50f99af724bc4fbe8ed0ce57ceeb546c8f679dec84e5)
|   @pip_sphinxcontrib_devhelp//:pkg (ebf05eacb2537ac9057c50f99af724bc4fbe8ed0ce57ceeb546c8f679dec84e5)
`-- @pip_sphinx//:pkg (ebf05eacb2537ac9057c50f99af724bc4fbe8ed0ce57ceeb546c8f679dec84e5)

These errors can easily be solved by pinning the older versions at one patch number below the current one:

sphinxcontrib-applehelp==1.0.4
sphinxcontrib-devhelp==1.0.2
sphinxcontrib-htmlhelp==2.0.1
sphinxcontrib-qthelp==1.0.3
sphinxcontrib-serializinghtml==1.1.5

The error is caused by the fact that at some point since the last release of these packages, Sphinx>5 was added as an explicit dependency of these packages, creating the dependency loops demonstrated above. For example, compare the setup.py of serializainghtml at 1.1.5 to its new pyproject.toml. This particular breakage appears to originate here.

I recognize that most Python users are not also Bazel users, so I also created a minimal Git repo that reproduces the issue.

• <link to minimal repro repo>

The requirements_lock.txt in this repo was produced by:

$ conda create -n sphinx_repro python=3.10
$ conda activate sphinx_repro
$ pip install sphinx
$ pip freeze > requirements_lock.txt

Please let me know if there is interest in resolving this issue here, and if so if I can help in any way!

How to Reproduce

$ git clone git@github.com:brunobeltran/minimal-sphinx-repro.git
$ cd minimal-sphinx-repro
$ bazel build //...

Environment Information

Platform:              linux; (Linux-6.1.0-10-amd64-x86_64-with-glibc2.36)
Python version:        3.10.12 (main, Jul  5 2023, 18:54:27) [GCC 11.2.0])
Python implementation: CPython
Sphinx version:        7.1.2
Docutils version:      0.20.1
Jinja2 version:        3.1.2
Pygments version:      2.16.1

Sphinx extensions

No response

Additional context

• Bazel.

[bruno-digitbio]

After reading up on the conversation starting here, it seems like the cleanest workaround is the one described here, which would allow one to manually remove the back-dependencies of sphinxcontrib-* on Sphinx manually for each package.

This should work 100% cleanly for all Bazel users who know that they will only be using sphinxcontrib-* as a dependency of Sphinx (and not ever as a stand-alone package).

I'll leave it up to discussion here whether having the sphinxcontrib-* be independently installable is worth creating these circular dependencies at the wheel-level, but after reading some recent Python mailing list discussion, it seems like there is some consensus that this is just how Python packaging works.

[AA-Turner]

Hi Bruno,

Thank you for your detailed issue. I have no experience with Bazel or rules_python, so please suffer me in my ignorance.

In sphinxcontrib-XXX, I want to declare a minimum allowed version of Sphinx through the packaging system, to make things easier for users. I used the project.dependencies table within pyproject.toml to achieve this, as there isn't (to my knowledge) a way to declare "constraints" within a distributed package.

What would be a Bazel-compatible way of achieving this? I'm happy to explore solutions, but as you say, from my perspective "this is just how Python packaging works" -- I never considered that adding this would cause any issues.

Thanks,
Adam

[bruno-digitbio]

@AA-Turner sorry for the delay, I was in the process of vendoring several dozen more Python libraries at work, and wanted to see how pervasive this problem is before proposing any "solutions" here.

Of the ~400 dependencies we (transitively) rely on, only Sphinx, PyTorch, and Airflow have this problem of breaking Bazel's underlying expectations that all dependencies should form a DAG.

AFAICT, the only "Bazel-compliant" way to break this dependency cycle is to not allow sphinxcontrib-XXX to depend on Sphinx. There is no way to do this without breaking users' abilities to do

$ pip install sphinxcontrib-XXX

and get a completely-working package.

One workaround that would work would be to define

[project.optional-dependencies]
standalone = [
  "sphinx > 5",
]

then users that really wanted to just install a contrib package directly could manually do

$ pip install sphinxcontrib-XXX[standalone]

this would need to be paired with a runtime import check that prints a friendly error message if sphinx fails to import from sphinxcontrib (prompting the user to use [standalone] if needed).

That said, this is a large amount of faff, when I think the Bazel team has basically decided that an upcoming version of Bazel will allow users to explicitly annotate groups of pip packages with dependency cycles so that Bazel can "ignore" these errors the same way pip does---there is even a production-ready PR already open adding this feature.

Given these considerations, I will close this Issue on the assumption(s) that

• we are not interested in breaking this dependency cycle intrinsically
• other Bazel users will hopefully be able to find this issue via Google and quickly deduce the solution via the links provided
• we can always reopen/revisit if it this also becomes a pain for other people that rely on DAG-like dependencies (e.g. if Debian package maintainers try to package any of the sphinxcontrib-*).

If my first assumption is wrong and you don't mind the solution I propose above, feel free to re-open and I can shoot over a PR that implements that approach. Thanks for your time!

[jrmarino]

It's not just a "Bazel" thing. Any script that traces through dependencies trees will hit this. These commits broke the auto-generation of python packages at Ravenports. The workaround was to sed out the new Sphinx >= 5 lines out of the requirements before tracing the requirements. There are well over 400 python modules in Ravenports and the Sphinx family is the ONLY one that caused a circular dependency on itself.

But yes, if you want to leave Sphinx as a dependency of htmlhelp and friends, then removing those as dependencies of Sphinx will resolve the circular dependency.

[mtelka]

pipdeptree reports this as an issue too: #11614

[mtelka]

In sphinxcontrib-XXX, I want to declare a minimum allowed version of Sphinx through the packaging system, to make things easier for users. I used the project.dependencies table within pyproject.toml to achieve this, as there isn't (to my knowledge) a way to declare "constraints" within a distributed package.

Isn't possible to check the low Sphinx version boundary during runtime? For example directly in sphinxcontrib/applehelp/__init__.py. This won't be so convenient for users, but I believe it will solve issues like this one. Thank you.

[AA-Turner]

It seems this causes more consternation than expected. @bruno-digitbio would you still be willing to implement the [standalone] workaround in each of the 6 sphinxcontrib packages? Ideally we'd also set app.require_sphinx('5.0').

A

[phlax]

fwiw, we are also hitting this (using bazel)

[phlax]

this now appears to be totally broken (ie pinning deps no longer works) for 7.2.3+ as it depends upon a problem version of sphinxcontrib-serializinghtml

[AA-Turner]

@phlax as mentioned, I'm willing to adopt the [standalone] approach Bruno mentioned, if someone creates the PRs -- would you consider doing so, please?

A

[phlax]

• Remove Sphinx as a required dependency sphinxcontrib-serializinghtml#10
• Remove Sphinx as a required dependency sphinxcontrib-devhelp#11
• Remove Sphinx as a required dependency sphinxcontrib-qthelp#15
• Remove Sphinx as a required dependency sphinxcontrib-htmlhelp#18
• Remove Sphinx as a required dependency sphinxcontrib-applehelp#15

[phlax]

@AA-Turner would be good to get reviews on ^^

[AA-Turner]

Oh, sorry -- I had meant to reply. Please may you add the requires_sphinx calls mentioned above, and an entry to CHANGES (can be identical, just mentioning the dependency change).

A

[AA-Turner]

Also, I believe .[test,standalone] should work, which only performs one installation.

[phlax]

requires_sphinx calls mentioned above

im not seeing the ref

an entry to CHANGES

sure

I believe .[test,standalone]

sgtm

[phlax]

ah ok - i see

app.require_sphinx('5.0')

[AA-Turner]

Thank you!

[phlax]

app.require_sphinx('5.0')

looking further - i had assumed there was some existing machinery that you wanted me to tap into but im not seeing any

can you be more specific on what you are expecting here ?

[AA-Turner]

Each package should have a def setup(app: Sphinx): line or similar -- the app.require_sphinx('5.0') call should go immediately under that.

See: https://www.sphinx-doc.org/en/master/extdev/appapi.html#sphinx.application.Sphinx.require_sphinx

A

[phlax]

got it - not sure why my require_sphinx searches didnt work - ill update shortly ...

[phlax]

ive put date as Pending in CHANGES files - should i put todays date?

[AA-Turner]

I tend to use "unreleased" or "in development".

[phlax]

k - if they are not being released today - then ill let you follow up to correct that

[phlax]

@AA-Turner any chance of resolving this - we have to pin a load of deps in various build to workaround, and we cant upgrade

[betaboon]

@AA-Turner friendly bump.

[BoleynSu]

@AA-Turner friendly ping

[AA-Turner]

Thanks to @phlax for various pull requests, releases just out for:

• https://pypi.org/project/sphinxcontrib-applehelp/1.0.8/
• https://pypi.org/project/sphinxcontrib-devhelp/1.0.6/
• https://pypi.org/project/sphinxcontrib-htmlhelp/2.0.5/
• https://pypi.org/project/sphinxcontrib-qthelp/1.0.7/
• https://pypi.org/project/sphinxcontrib-serializinghtml/1.1.10/

The above packages no longer contain a dependency on Sphinx.

A

[jayaddison]

I'm still learning more of the details but am a bit puzzled by the approach to a fix here.

My understanding is that Sphinx lists dependencies on various popular sphinxcontrib extensions because they're useful in a wide variety of situations, and so it's a good idea for them to be available with a default installation of sphinx.

However, the dependency specifications from the sphinxcontrib extensions are also valid: those extensions genuinely do not work except with specified versions of Sphinx (in many cases >=5 was a requirement until these changes). See #11890 and gitpython-developers/GitPython#1802 for examples.

Does anyone have context on why this problem had not been reported related to bazel at an earlier date? Both Sphinx and Bazel seem like they're popular and widely-used, so I'm wondering whether there were some circumstances that precipitated this -- it could be useful to know that because the Bazel rules_python fix PR (bazelbuild/rules_python#1166) mentioned by Bruno has been merged in verson 0.28 of that component (on 2024-01-08 GMT).

[phlax]

fwiw bazel now has a workaround for this which we are currently using (pending me removing it)

regardless i think bazel is just flagging a genuine problem - ie circular dependencies - my understanding is that the landed solution is the best way to express this relationship

not sure about historical without tracking through repos/versions but it started happening relatively recently

[jayaddison]

regardless i think bazel is just flagging a genuine problem - ie circular dependencies

Circular dependencies can be a problem sometimes, but I'm not yet convinced that they have been a problem for Sphinx.

Sphinx lists a few sphinxcontrib packages - in most cases without version filters - as dependencies.

In turn, some of those extensions depend on filtered versions of Sphinx, because they need features from the core of Sphinx at runtime to build documentation projects.

An unresolvable circular dependency could be a problem, I agree - but this doesn't seem to have been the case here.

not sure about historical without tracking through repos/versions but it started happening relatively recently

Can you provide any pointers for us (or me) to get started on that? Without knowing a specific reason for a disruptive change like this, I'd suggest we might want to revert or at least reconsider.

Edit: attempt to clarify some wording, because the term 'build' can refer to building a Python package or using the installed packages to build a documentation project.

[jayaddison]

I have to admit to not following this reference until a few moments ago:

The error is caused by the fact that at some point since the last release of these packages, Sphinx>5 was added as an explicit dependency of these packages, creating the dependency loops demonstrated above. For example, compare the setup.py of serializainghtml at 1.1.5 to its new pyproject.toml. This particular breakage appears to originate here.

So in fact there was a change in August 2023 to introduce the Sphinx dependency -- and that's likely what initiated the chain of events leading to Bazel's rules_python encountering problems (initially that seems to match timing-wise).

I'll do a bit more reading/investigation to confirm that -- and I do still think it's valid in theory for extensions to specify minimum supported Sphinx versions -- but clearly I hadn't understood enough of the details.

[darkvertex]

Hi, just chiming into the pile to say that another package manager that was affected was Rez (a VFX-studio-oriented package and environment manager) which freaked out about having circular dependencies when at my employer we tried to package an internal Python project as a Rez package that was sourcing Sphinx.