Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Fix dependency conflict #14264

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Dec 3, 2020

Pip 20.3 introduced a new dependency resolver[0] which silently
reinterprets our current requirements file in a different way to
resolve the dependencies, which results the build being broken.

On non-aarch64 systems, there are two requirements that satisfy
the sphinx-rtd-theme package:

  • One provided by our own theme repository[1] which has nice consistent
    theming for the website, and
  • One provided by the upstream sphinx-rtd-theme package.

Prior to pip 20.3, the default resolver was able to resolve this
conflict to favour our custom theme, which is the one we intend to use
in most cases. Unfortunately, with the new resolver, this conflict is
resolved the other way. As far as I can tell, there is no "strict" mode
to prescribe that pip should resolve conflicts first and fail out if the
requirements are ambiguous. Instead, pip silently resolves this conflict
and we do not find out that there was ambiguity until later in the
process.

In addition, on readthedocs.org, new versions of pip are automatically
pulled upon each new docs build, which meant that from one day to the
next, a previously successful build began to consistently fail with
weird errors that imply a problem with the dependency but don't explain
why the problem was introduced without changes to code in our build
system:

  Theme error:
  no theme named 'sphinx_rtd_theme_cilium' found (missing theme.conf?)
  make[1]: *** [Makefile:48: html] Error 2
  make: *** [Makefile:552: test-docs] Error 2

Fix the issue by disambiguating the theme dependency.

Fixes: #14252

Pip 20.3 introduced a new dependency resolver[[0]] which silently
reinterprets our current requirements file in a different way to
resolve the dependencies, which results the build being broken.

On non-aarch64 systems, there are two requirements that satisfy
the sphinx-rtd-theme package:
* One provided by our own theme repository[[1]] which has nice consistent
  theming for the website, and
* One provided by the upstream sphinx-rtd-theme package.

Prior to pip 20.3, the default resolver was able to resolve this
conflict to favour our custom theme, which is the one we intend to use
in most cases. Unfortunately, with the new resolver, this conflict is
resolved the other way. As far as I can tell, there is no "strict" mode
to prescribe that pip should resolve conflicts first and fail out if the
requirements are ambiguous. Instead, pip silently resolves this conflict
and we do not find out that there was ambiguity until later in the
process.

In addition, on readthedocs.org, new versions of pip are automatically
pulled upon each new docs build, which meant that from one day to the
next, a previously successful build began to consistently fail with
weird errors that imply a problem with the dependency but don't explain
why the problem was introduced without changes to code in our build
system:

  Theme error:
  no theme named 'sphinx_rtd_theme_cilium' found (missing theme.conf?)
  make[1]: *** [Makefile:48: html] Error 2
  make: *** [Makefile:552: test-docs] Error 2

Fix the issue by disambiguating the theme dependency.

[0]: http://pyfound.blogspot.com/2020/11/pip-20-3-new-resolver.html
[1]: https://github.com/cilium/sphinx_rtd_theme

Fixes: cilium#14252

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer requested a review from a team as a code owner December 3, 2020 19:01
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.1 Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.6 Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.12 Dec 3, 2020
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 3, 2020
@joestringer joestringer added this to Needs backport from master in 1.7.13 Dec 4, 2020
@joestringer joestringer removed this from Needs backport from master in 1.7.12 Dec 4, 2020
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks for the fix!

@qmonnet qmonnet removed their assignment Dec 4, 2020
@qmonnet qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 4, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.12 Dec 4, 2020
@aanm aanm removed this from Needs backport from master in 1.8.6 Dec 4, 2020
@aanm aanm added this to Needs backport from master in 1.8.7 Dec 4, 2020
@aanm aanm removed this from Needs backport from master in 1.7.12 Dec 4, 2020
@aanm aanm added this to Needs backport from master in 1.8.6 Dec 4, 2020
@aanm aanm removed this from Needs backport from master in 1.8.6 Dec 4, 2020
@aanm aanm added this to Needs backport from master in 1.8.7 Dec 4, 2020
@aanm aanm added this to Needs backport from master in 1.9.2 Dec 4, 2020
@aanm aanm removed this from Needs backport from master in 1.9.1 Dec 4, 2020
@kkourt kkourt merged commit f15af5e into cilium:master Dec 7, 2020
@joestringer joestringer deleted the submit/fix-docs-build branch December 8, 2020 00:51
@jibi jibi mentioned this pull request Dec 17, 2020
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.9 in 1.9.2 Jan 18, 2021
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.8 in 1.8.7 Jan 18, 2021
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.7 in 1.7.13 Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.7.13
Backport done to v1.7
1.8.7
Backport done to v1.8
1.9.2
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

Cannot build documentation: no theme named 'sphinx_rtd_theme_cilium' found (missing theme.conf?)
5 participants