Opened 2 years ago
Closed 22 months ago
#24935 closed enhancement (fixed)
Upgrade to Sphinx 1.7.5
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  packages: standard  Keywords:  
Cc:  embray, fbissey, dimpase  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Ralf Stephan, François Bissey 
Report Upstream:  Fixed upstream, but not in a stable release.  Work issues:  
Branch:  4e066ba (Commits)  Commit:  4e066baffdce49ccc9bcc402b1ff534022b5f355 
Dependencies:  #25570  Stopgaps: 
Description (last modified by )
Tarballs:
Upstream bugs:
 https://github.com/sphinxdoc/sphinx/issues/4978 (fixed in release 1.7.5)
 https://github.com/sphinxdoc/sphinx/issues/5040 (fixed in master; patch added to Sage)
Change History (62)
comment:1 Changed 2 years ago by
 Cc fbissey added
comment:2 Changed 2 years ago by
 Dependencies set to #24972
comment:3 Changed 2 years ago by
 Description modified (diff)
comment:4 Changed 2 years ago by
 Branch set to u/jdemeyer/upgrade_to_sphinx_1_7_1
comment:5 Changed 2 years ago by
 Commit set to 41f114ec76d1e97701f77e0b197615faab308a10
 Description modified (diff)
comment:6 Changed 2 years ago by
 Commit changed from 41f114ec76d1e97701f77e0b197615faab308a10 to 83682b257e42815d8608db5440f63983ad723088
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
83682b2  Upgrade to Sphinx 1.7.1

comment:7 Changed 2 years ago by
 Commit changed from 83682b257e42815d8608db5440f63983ad723088 to 4fc721aa2203e9c4ff34df1acedcd70f443b96b9
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4fc721a  Upgrade to Sphinx 1.7.1

comment:8 Changed 2 years ago by
 Status changed from new to needs_review
comment:9 followup: ↓ 10 Changed 2 years ago by
Can you please give a link to the sphinxcontrib_websupport package, or is it supposed to be installed by other means? An upgrade attempt failed here because the mirrors didn't have it.
comment:10 in reply to: ↑ 9 Changed 2 years ago by
comment:11 Changed 2 years ago by
 Status changed from needs_review to needs_info
********************************************************************** File "src/sage/misc/sphinxify.py", line 51, in sage.misc.sphinxify.sphinxify Failed example: sphinxify('**Testing**\n`monospace`') Expected: '...<div class="docstring"...<strong>Testing</strong>\n<span class="math"...</p>\n\n\n</div>' Got: '<div class="docstring">\n \n <p><strong>Testing</strong>\n<span class="math notranslate">monospace</span></p>\n\n\n</div>' ********************************************************************** File "src/sage/misc/sphinxify.py", line 53, in sage.misc.sphinxify.sphinxify Failed example: sphinxify('`x=y`') Expected: '...<div class="docstring">\n \n <p><span class="math">x=y</span></p>\n\n\n</div>' Got: '<div class="docstring">\n \n <p><span class="math notranslate">x=y</span></p>\n\n\n</div>' **********************************************************************
comment:12 Changed 2 years ago by
 Status changed from needs_info to needs_work
comment:13 followup: ↓ 16 Changed 2 years ago by
I did not consider that Sphinx might affect doctests too.
comment:14 Changed 2 years ago by
 Commit changed from 4fc721aa2203e9c4ff34df1acedcd70f443b96b9 to 7419e0246230594ebfd5e7a2fe6b80d67abfc98a
Branch pushed to git repo; I updated commit sha1. New commits:
7419e02  Fix sphinxify doctests

comment:15 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:16 in reply to: ↑ 13 Changed 2 years ago by
Replying to jdemeyer:
I did not consider that Sphinx might affect doctests too.
It definitely can :) There are several places in Python 3 where I've had to make sure to ignore some python 3 specific DeprecationWarning
s from Sphinx at import time or else it broke some tests. So I'm not too surprised that a Sphinx upgrade might also affect some docbuildrelated tests.
comment:17 followup: ↓ 18 Changed 2 years ago by
 Reviewers set to Ralf Stephan
 Status changed from needs_review to positive_review
It would be great if patchbot could test "unsafe" tickets but I remember about difficulties.
comment:18 in reply to: ↑ 17 ; followups: ↓ 19 ↓ 21 Changed 2 years ago by
Replying to rws:
It would be great if patchbot could test "unsafe" tickets but I remember about difficulties.
I already have a patch for patchbot that will allow it to test tickets with new packages. It's been running on my Cygwin patchbot for a while now, but it does occasionally run into hiccups requiring me to manually restart the patchbot (after cleaning things up a bit).
I especially am not 100% sure why this happens sometimes, since the patchbot creates a copy of SAGE_LOCAL for "unsafe" tickets before doing much else.
comment:19 in reply to: ↑ 18 Changed 2 years ago by
Speaking of "unsafe" tickets, I'm not convinced that "unsafe" tickets are on average less safe than "safe" tickets. We certainly have had "safe" tickets messing up a patchbot installation.
comment:20 Changed 2 years ago by
Yes, it's a pretty flimsy distinction.
comment:21 in reply to: ↑ 18 Changed 2 years ago by
Replying to embray:
I especially am not 100% sure why this happens sometimes, since the patchbot creates a copy of SAGE_LOCAL for "unsafe" tickets before doing much else.
Please note https://github.com/sagemath/sagepatchbot/issues/34
comment:22 followup: ↓ 28 Changed 2 years ago by
 Status changed from positive_review to needs_work
PDF docs don't build...
comment:23 Changed 23 months ago by
 Description modified (diff)
 Milestone changed from sage8.2 to sage8.3
 Summary changed from Upgrade to Sphinx 1.7.1 to Upgrade to Sphinx 1.7.4
comment:24 Changed 23 months ago by
 Dependencies #24972 deleted
comment:25 Changed 23 months ago by
 Commit changed from 7419e0246230594ebfd5e7a2fe6b80d67abfc98a to e49afe3056470ababad81d34fe0e65c43e0bebec
comment:26 Changed 23 months ago by
 Commit changed from e49afe3056470ababad81d34fe0e65c43e0bebec to 5741161e15e78db9be8ef2e8a6c30af740893088
Branch pushed to git repo; I updated commit sha1. New commits:
5741161  Run TeX for docbuild noninteractively

comment:27 Changed 23 months ago by
 Description modified (diff)
 Report Upstream changed from N/A to Reported upstream. No feedback yet.
comment:28 in reply to: ↑ 22 Changed 23 months ago by
Replying to vbraun:
PDF docs don't build...
Indeed... unfortunately it's a genuine problem with Sphinx. I reported it at https://github.com/sphinxdoc/sphinx/issues/4978
comment:29 Changed 22 months ago by
 Description modified (diff)
 Report Upstream changed from Reported upstream. No feedback yet. to Completely fixed; Fix reported upstream
 Summary changed from Upgrade to Sphinx 1.7.4 to Upgrade to Sphinx 1.7.5
comment:30 Changed 22 months ago by
 Commit changed from 5741161e15e78db9be8ef2e8a6c30af740893088 to f4ca3d2d14d73770250c99c962ea958effca3af5
comment:31 Changed 22 months ago by
 Description modified (diff)
 Report Upstream changed from Completely fixed; Fix reported upstream to Reported upstream. No feedback yet.
comment:32 Changed 22 months ago by
The Sphinx problem with Brazilian is fixed [0] but now there is a Sphinx problem with Russian [1].
comment:33 Changed 22 months ago by
 Commit changed from f4ca3d2d14d73770250c99c962ea958effca3af5 to 867372ce1c932870a9d9e4e5e0454a710f175072
comment:34 Changed 22 months ago by
 Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.
 Status changed from needs_work to needs_review
This took several iterations to get right, but now the PDF docs build for me.
comment:35 Changed 22 months ago by
 Description modified (diff)
comment:36 Changed 22 months ago by
Upstream greatly simplified the patch for PR5400 but I'll admit I am unclear if it can be used alone or need some other PR to go with it.
comment:37 followup: ↓ 38 Changed 22 months ago by
Are you referring to https://github.com/sphinxdoc/sphinx/pull/5045?
Anyway, the current branch seems to work so I'd rather keep it.
comment:38 in reply to: ↑ 37 ; followup: ↓ 39 Changed 22 months ago by
 Reviewers changed from Ralf Stephan to Ralf Stephan, François Bissey
 Status changed from needs_review to positive_review
Replying to jdemeyer:
Are you referring to https://github.com/sphinxdoc/sphinx/pull/5045?
Anyway, the current branch seems to work so I'd rather keep it.
No. 5040 has two commits. Your current patch is the first one. It is described as a temporary solution while the second commit is more "future proofing". But indeed the first commit you are using is supposed to work. Let's send this to the bots as is.
comment:39 in reply to: ↑ 38 Changed 22 months ago by
comment:40 Changed 22 months ago by
Cumulative patches on the same files confuse me no end.
comment:41 Changed 22 months ago by
 Status changed from positive_review to needs_work
sage t long src/sage/misc/sphinxify.py # 2 doctests failed sage t long local/lib/python2.7/sitepackages/sagenb/misc/sphinxify.py # 2 doctests failed
comment:42 followup: ↓ 44 Changed 22 months ago by
 Cc dimpase added
Trivial but we should have seen it in reviews
fbissey@moonloop ~ $ sage t long /usr/lib64/python2.7/sitepackages/sage/misc/sphinxify.py too many failed tests, not using stored timings Running doctests with ID 2018061020162993e55012. Using optional=optional,sage Doctesting 1 file. sage t long /usr/lib64/python2.7/sitepackages/sage/misc/sphinxify.py ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sage/misc/sphinxify.py", line 51, in sage.misc.sphinxify.sphinxify Failed example: sphinxify('**Testing**\n`monospace`') Expected: '<div class="docstring"...<strong>Testing</strong>\n<span class="math notranslate"...</p>\n\n\n</div>' Got: '<div class="docstring">\n \n <p><strong>Testing</strong>\n<span class="math notranslate nohighlight">monospace</span></p>\n\n\n</div>' ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sage/misc/sphinxify.py", line 53, in sage.misc.sphinxify.sphinxify Failed example: sphinxify('`x=y`') Expected: '<div class="docstring">\n \n <p><span class="math notranslate">x=y</span></p>\n\n\n</div>' Got: '<div class="docstring">\n \n <p><span class="math notranslate nohighlight">x=y</span></p>\n\n\n</div>' ********************************************************************** 1 item had failures: 2 of 10 in sage.misc.sphinxify.sphinxify [9 tests, 2 failures, 1.02 s]  sage t long /usr/lib64/python2.7/sitepackages/sage/misc/sphinxify.py # 2 doctests failed  Total time for all tests: 1.0 seconds cpu time: 1.0 seconds cumulative wall time: 1.0 seconds fbissey@moonloop ~ $ sage t long /usr/lib64/python2.7/sitepackages/sagenb/misc/sphinxify.py too many failed tests, not using stored timings Running doctests with ID 20180610201744197b0e5d. Using optional=optional,sage Doctesting 1 file. sage t long /usr/lib64/python2.7/sitepackages/sagenb/misc/sphinxify.py ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sagenb/misc/sphinxify.py", line 68, in sagenb.misc.sphinxify.sphinxify Failed example: sphinxify('**Testing**\n`monospace`') Expected: '...<div class="docstring"...<strong>Testing</strong>\n<span class="math"...</p>\n\n\n</div>' Got: '<div class="docstring">\n \n <p><strong>Testing</strong>\n<span class="math notranslate nohighlight">monospace</span></p>\n\n\n</div>' ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sagenb/misc/sphinxify.py", line 70, in sagenb.misc.sphinxify.sphinxify Failed example: sphinxify('`x=y`') Expected: '...<div class="docstring">\n \n <p><span class="math">x=y</span></p>\n\n\n</div>' Got: '<div class="docstring">\n \n <p><span class="math notranslate nohighlight">x=y</span></p>\n\n\n</div>' ********************************************************************** 1 item had failures: 2 of 10 in sagenb.misc.sphinxify.sphinxify [14 tests, 2 failures, 0.74 s]  sage t long /usr/lib64/python2.7/sitepackages/sagenb/misc/sphinxify.py # 2 doctests failed  Total time for all tests: 1.2 seconds cpu time: 0.7 seconds cumulative wall time: 0.7 seconds
That unfortunately requires a new version of sagenb
. @Dima could we get a 1.0.3 for this.
comment:43 followup: ↓ 45 Changed 22 months ago by
OK, so that's just doctests in sagenb, right? No problem, I'll fix these. https://github.com/sagemath/sagenb/issues/445
comment:44 in reply to: ↑ 42 Changed 22 months ago by
Replying to fbissey:
Trivial but we should have seen it in reviews
Right. I just did not think about running doctests after fixing the PDF documentation stuff.
comment:45 in reply to: ↑ 43 Changed 22 months ago by
Replying to dimpase:
OK, so that's just doctests in sagenb, right? No problem, I'll fix these. https://github.com/sagemath/sagenb/issues/445
The commit works for me. We just have to reproduce it in sage's sphinxify.py
.
comment:46 followup: ↓ 47 Changed 22 months ago by
here is the new sagenb https://github.com/sagemath/sagenb/releases/download/1.0.3/sagenb1.0.3.tar.bz2
Could you bump up the version in build/pkgs/sagenb/ etc to include it on this ticket?
comment:47 in reply to: ↑ 46 ; followup: ↓ 48 Changed 22 months ago by
Replying to dimpase:
here is the new sagenb https://github.com/sagemath/sagenb/releases/download/1.0.3/sagenb1.0.3.tar.bz2
Could you bump up the version in build/pkgs/sagenb/ etc to include it on this ticket?
Definitely. I am leaving it to Jeroen as the ticket author, unless he indicates he is too busy to handle it in which case I will step up.
comment:48 in reply to: ↑ 47 Changed 22 months ago by
Replying to fbissey:
I am leaving it to Jeroen as the ticket author, unless he indicates he is too busy to handle it in which case I will step up.
It's literally my job to do this...
comment:49 Changed 22 months ago by
 Description modified (diff)
comment:50 Changed 22 months ago by
 Commit changed from 867372ce1c932870a9d9e4e5e0454a710f175072 to e398cf8db43373b1b1c2f1974404d33e9e164d63
comment:51 Changed 22 months ago by
 Status changed from needs_work to needs_review
comment:52 Changed 22 months ago by
 Status changed from needs_review to positive_review
Works for me.
comment:53 followup: ↓ 54 Changed 22 months ago by
Damn. Could we get rid of the "import __builtin__"
in sagenb/misc/sphinxify.py and sagenb/misc/support.py ??
This is breaking the compatibility of sagenb with python3...
comment:54 in reply to: ↑ 53 ; followup: ↓ 55 Changed 22 months ago by
Replying to chapoton:
Damn. Could we get rid of the
"import __builtin__"
in sagenb/misc/sphinxify.py and sagenb/misc/support.py ??
Perhaps, but not in this ticket please.
comment:55 in reply to: ↑ 54 Changed 22 months ago by
Replying to jdemeyer:
Replying to chapoton:
Damn. Could we get rid of the
"import __builtin__"
in sagenb/misc/sphinxify.py and sagenb/misc/support.py ??Perhaps, but not in this ticket please.
Totally agree with Jeroen, another ticket please.
That being said, historically we have migrated sphinxify.py
from sagenb
into sage to reduce dependency on sagenb
(that's what I remember at least). If you want to work on that, I would really suggest to try eliminating the sphinxify.py
of sagenb
and use sage's one instead.
comment:56 Changed 22 months ago by
 Status changed from positive_review to needs_work
File "src/doc/common/conf.py", line 627, in doc.common.conf.call_intersphinx Failed example: for line in open(thematic_index).readlines(): if "padics" in line: sys.stdout.write(line) Expected: <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sageringspadicstutorial" title="(in Sage Reference Manual: pAdics ...)"><span>Introduction to the adics</span></a></li> Got: <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sageringspadicstutorial" title="(in Sage Reference Manual: pAdics)"><span>Introduction to the adics</span></a></li> ********************************************************************** 1 item had failures: 1 of 4 in doc.common.conf.call_intersphinx [3 tests, 1 failure, 0.61 s]
comment:57 Changed 22 months ago by
I have seen it too in the vbraun branch and it confused me quite a bit because I don't remember seeing it the first time round but it should have been there. It seems like before this ticket the title field ends up with v
as in title="(in Sage Reference Manual: pAdics v)
not just for this particular one that's tested but for all of them. After this ticket it is reduced to just title="(in Sage Reference Manual: pAdics)
and the v
has disappeared everywhere.
comment:58 Changed 22 months ago by
It's supposed to say v8.3.beta5
so that's a regression somehow.
comment:59 Changed 22 months ago by
 Dependencies set to #25570
 Status changed from needs_work to positive_review
I opened #25570 for the version number regression.
comment:60 Changed 22 months ago by
 Commit changed from e398cf8db43373b1b1c2f1974404d33e9e164d63 to 4e066baffdce49ccc9bcc402b1ff534022b5f355
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:
3b57916  Fix version number in built documentation

b321995  Run TeX for docbuild noninteractively

8a71e29  Upgrade to Sphinx 1.7.5

a22da72  Upgrade sagenb

4e066ba  Fix sphinxify doctests

comment:61 Changed 22 months ago by
 Status changed from needs_review to positive_review
comment:62 Changed 22 months ago by
 Branch changed from u/jdemeyer/upgrade_to_sphinx_1_7_1 to 4e066baffdce49ccc9bcc402b1ff534022b5f355
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
added sphinxcontrib_websupport
Minor fixes to sphinxcontrib_websupport
Upgrade to Sphinx 1.7.1