upgrade Sphinx to 1.2

[rws]

According to ​https://groups.google.com/forum/#!topic/sphinx-users/EC7_wNtE5To Sphinx-1.2 is 25% faster than 1.1.2, and this matters with doc-clean Sage builds. (This assumption turned out to be false, see comment:18)

Source:

​https://pypi.python.org/packages/source/S/Sphinx/Sphinx-1.2.2.tar.gz

[fbissey]

The sage-on-gentoo experience with this is that we need to clean up a number of things before upgrading. It of course can be done at the same time.

[rws]

New commits:

​cd12a2b

16396: sphinx-1.2.2 install: fix grammar script; bump version

---

Lots of SEEALSO problems apparently.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​50e24c2

16396: fix SEEALSO indentation

[rws]

Further, I get a crash which is reported at ​https://bitbucket.org/birkenfeld/sphinx/issue/1475/typeerror-in-get_js_index

[rws]

Upstream says:

Sphinx-1.2 has changed the signature of sphinx.search.IndexBuilder class (e5bd5a84b5fd) and Sage extension has used the IndexBuilder directly without a appended argument.

I think you can build the document with Sphinx-1.1.3. Alternatively, please contact to sage documentation maintainer.

[fbissey]

I will have a look. I have a number of patches for sphinx 1.2 in sage-on-gentoo. We have success at building the html doc with sphinx 1.2. However there is a mystery on whether or not we can build the pdf documentation at the moment.

[fbissey]

OK, you did the right thing for SEEALSO what you are missing can be found here ​https://github.com/cschwan/sage-on-gentoo/blob/master/sci-mathematics/sage-doc/files/sage-doc-6.2-sphinx.patch that should solve the crash you reported. Not that because you are not sage-on-gentoo you do not have to bother with testing the sphinx version. However we would appreciate if you did.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​f81c55d	16396: fixing sage-doc-9999-r1 with Steve's help
​53bf86e	16396: Depend on Sage since docs are now built

[rws]

I did a sage -tp --long src/doc/ if you meant that. Except for pdf it's ready for review now. Not sure if we should wait for it.

[jhpalmieri]

Can you provide a link to the new tarball?

[fbissey]

In sage-on-gentoo pdf generation is officially broken with sphinx 1.2.2. Some math is generated in pure tex rather than latex ("\(...\)" rather than "$...$") and it breaks the pdf engine. We are investigating at this stage.

[rws]

If the pdf issue is an upstream issue, is there a ticket? Can you please add it to this ticket description and set the resp. field? Any other links with information?

Tarball link is added.

[rws]

I should say that I manually untarred, renamed the Sphinx directory to sphinx and rebzipped. There must be a better way of doing this, as there is no difference in names to the previous version.

[fbissey]

Replying to rws:

If the pdf issue is an upstream issue, is there a ticket? Can you please add it to this ticket description and set the resp. field? Any other links with information?

Tarball link is added.

We don't know yet if it is an upstream issue or something that sage's doc need to be adapted to. sage-on-gentoo is technically downstream from you not upstream.

[jhpalmieri]

For what it's worth, I did several iterations of make doc-clean followed by make. Docbuilding did not speed up with the new version of Sphinx:

• old version: 219.8 seconds, 217.3 seconds, 224.7 seconds
• new version: 227.5 seconds, 218.3 seconds, 219.5 seconds

(This was with 4 threads.) Given the problems with PDF building, maybe there isn't much hurry with this? Or are other people seeing a speed improvement with the new version?

[fbissey]

OK Steve has found the commit of sphinx that break things for pdflatex and I have left a comment of the corresponding issue ​https://bitbucket.org/birkenfeld/sphinx/issue/929/parsed-literal-creates-bad-latex-math. If they don't pick up the comment in a couple of day I'll open an issue proper with them.

[rws]

Replying to jhpalmieri:

For what it's worth, I did several iterations of make doc-clean followed by make. Docbuilding did not speed up with the new version of Sphinx:

• old version: 219.8 seconds, 217.3 seconds, 224.7 seconds
• new version: 227.5 seconds, 218.3 seconds, 219.5 seconds

(This was with 4 threads.) Given the problems with PDF building, maybe there isn't much hurry with this? Or are other people seeing a speed improvement with the new version?

No, it confirms my impression. There is no hurry. I thought it might somehow fix the docbuild issue discussed in sage-releases but I cannot reproduce that at the moment, so I cannot even look if Sphinx-1.2 makes a difference there.

[fbissey]

Given the lack of answer to my first comment on the commit that breaks pdf generation I opened a brand new issue for the problem: ​https://bitbucket.org/birkenfeld/sphinx/issue/1480/latex-math-output-is-broken-for-inline

[fbissey]

Upstream is quite unresponsive. If anyone has a direct line they can try that. I think it is time to patch on our side.

[strogdon]

I believe I have a patch that's not to invasive, i.e. it basically reverts to the old behavior when using inline math and doesn't disrupt (hopefully) the need to use pure tex in a verbatim (alltt) environment. I'll post here since it will take me time to remember how to commit

--- sphinx/writers/latex.py.orig        2014-06-05 23:38:03.808669401 -0500
+++ sphinx/writers/latex.py     2014-06-05 23:27:22.752376713 -0500
@@ -264,6 +264,7 @@
         self.next_figure_ids = set()
         self.next_table_ids = set()
         # flags
+        self.verbatim = False
         self.in_title = 0
         self.in_production_list = 0
         self.in_footnote = 0
@@ -1319,6 +1320,7 @@
                                    (self.curfilestack[-1], node.line))
         if node.rawsource != node.astext():
             # most probably a parsed-literal block -- don't highlight
+            self.verbatim = True
             self.body.append('\\begin{alltt}\n')
         else:
             code = node.astext().rstrip('\n')
@@ -1351,6 +1353,7 @@
             raise nodes.SkipNode
     def depart_literal_block(self, node):
         self.body.append('\n\\end{alltt}\n')
+        self.verbatim = False
     visit_doctest_block = visit_literal_block
     depart_doctest_block = depart_literal_block
 
--- sphinx/ext/mathbase.py.orig 2014-06-05 23:39:36.773293241 -0500
+++ sphinx/ext/mathbase.py      2014-06-05 23:18:53.949020605 -0500
@@ -88,7 +88,10 @@
 
 
 def latex_visit_math(self, node):
-    self.body.append('\\(' + node['latex'] + '\\)')
+    if self.verbatim:
+        self.body.append('\\(' + node['latex'] + '\\)')
+    else:
+        self.body.append('$' + node['latex'] + '$')
     raise nodes.SkipNode
 
 def latex_visit_displaymath(self, node):

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​86ba11c	Merge branch 'develop' into t/16396/upgrade_sphinx_to_1_2
​761be30	16396: latex math output is broken for inline math in chapters

[rws]

Replying to strogdon:

I believe I have a patch that's not to invasive, i.e. it basically reverts to the old behavior when using inline math and doesn't disrupt (hopefully) the need to use pure tex in a verbatim (alltt) environment. I'll post here since it will take me time to remember how to commit

Thanks, I have added your patch with you as author.

[fbissey]

Anything left to do with this ticket?

[Snark]

I wanted to give it a try, but it failed to download the tarball from sagemath.org ; when I took the tarball from the link in the ticket description the sums didn't match.

[fbissey]

There are several wrong things. checksums.ini looks for

tarball=sphinx-VERSION.tar.bz2

the linked tarball is Sphinx-1.2.2.tar.gz so of course the checksum won't match. We need to recommit that with

tarball=Sphinx-VERSION.tar.gz

and new checksum to be sure. Once checksum.ini is corrected for the right filename run

./sage -sh sage-fix-pkg-checksums

As detailed in ​http://sagemath.org/doc/developer/packaging.html#checksums.

[Snark]

Changing the filename doesn't modify the checksum as far as I know, so I just renamed the tarball. The fact that it was wrong was enough to see the ticket wasn't ready for review.

[fbissey]

Replying to Snark:

Changing the filename doesn't modify the checksum as far as I know, so I just renamed the tarball. The fact that it was wrong was enough to see the ticket wasn't ready for review.

It is indeed not ready to review. Switching to French Julien.

Il est vrai que changer le nom de fichier ne suffit pas. Mon commentaire a deux instructions:

• changer le nom
• executer le script "./sage -sh sage-fix-pkg-checksums" qui va recalculer les checksums d'apres les fichiers que tu as deja telecharge.

Renommer un tar.gz en tar.bz2 n'est pas une bonne idee, decompresser et recompresser ne donneras pas la meme checksum, si la version de bzip2 n'est pas completement identique le bzip2 serat legerement different.

[Snark]

Ooooooooh, I hadn't noticed the original was a .gz and not a .bz2! Indeed, no wonder that didn't work... I had only seen the Sphinx vs sphinx.

I'll probably give it a try later today, to see exactly how much work is involved in fixing that ticket. I'm quite interested in getting docutils upgraded, and this is a prerequisite!

[Snark]

Ok, doing things less stupidly does make things better (who would have thought?! ;-P ) : it builds and there are three failing doctests, which are trivial to fix: those doctests expect a newline before a div, and there are none now:

sage -t --long local/lib/python2.7/site-packages/sagenb-0.10.8.2-py2.7.egg/sagenb/misc/sphinxify.py
**********************************************************************
File "local/lib/python2.7/site-packages/sagenb-0.10.8.2-py2.7.egg/sagenb/misc/sphinxify.py", line 69, in sagenb.misc.sphinxify.sphinxify
Failed example:
    sphinxify('A test')
Expected:
    '\n<div class="docstring">\n    \n  <p>A test</p>\n\n\n</div>'
Got:
    '<div class="docstring">\n    \n  <p>A test</p>\n\n\n</div>'
**********************************************************************
File "local/lib/python2.7/site-packages/sagenb-0.10.8.2-py2.7.egg/sagenb/misc/sphinxify.py", line 71, in sagenb.misc.sphinxify.sphinxify
Failed example:
    sphinxify('**Testing**\n`monospace`')
Expected:
    '\n<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">monospace</span></p>\n\n\n</div>'
**********************************************************************
File "local/lib/python2.7/site-packages/sagenb-0.10.8.2-py2.7.egg/sagenb/misc/sphinxify.py", line 73, in sagenb.misc.sphinxify.sphinxify
Failed example:
    sphinxify('`x=y`')
Expected:
    '\n<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">x=y</span></p>\n\n\n</div>'
**********************************************************************
1 item had failures:
   3 of  10 in sagenb.misc.sphinxify.sphinxify
    [14 tests, 3 failures, 0.44 s]

I also had a look at the branch to see how the patches looked like : there is a strange chunk on src/doc/common/multidocs.py with a test on whether the sphinx version is less or above 1.2... which makes no sense if we're upgrading to 1.2.2!

So I would say : rebase the branch, fix the tarball issues (name, checksum), remove the chunk mentioned above, fix the doctests and it will be good to review and close!

PS: in fact, I may give git-trac a try and push a patch fixing the doctests this evening... if I manage to.

[Snark]

Hmmmmmm... it's in sagenb... how does one proceed in such a case? Ask sagenb to make a new release with patched doctests, then push a ticket to upgrade to that, making this one depend on that new ticket, etc?

[fbissey]

Replying to Snark:

Ok, doing things less stupidly does make things better (who would have thought?! ;-P ) > I also had a look at the branch to see how the patches looked like : there is a strange chunk on src/doc/common/multidocs.py with a test on whether the sphinx version is less or above 1.2... which makes no sense if we're upgrading to 1.2.2!

See ​http://trac.sagemath.org/ticket/16396#comment:8 but like I said we are not strongly attached to it.

For sagenb you can open a ticket on their github ​https://github.com/sagemath/sagenb and yes we'll need a new release.

[fbissey]

Do you want to open an issue for sagenb and make a pull request or would you prefer I do it Julien?

[Snark]

First eclib and its detection of NTL, now sagenb and its sphinx doctests... you're late to the ​party again! ;-P

[fbissey]

Replying to Snark:

First eclib and its detection of NTL, now sagenb and its sphinx doctests... you're late to the ​party again! ;-P

Yes you work while I sleep. I cannot really complain about that ;)

[ppurka]

Something is wrong with your patches. First, checksums.ini contains the patch:

build/pkgs/sphinx/checksums.ini

@@ -1 +1 @@
 tarball=sphinx-VERSION.tar.bz2
-sha1=2a6f238185a58cbb5c597c985d715ab9653e0fbe
-md5=9ea0a6f218779ddbe3222c5ef8c0f6e6
-cksum=1040528499
+sha1=deb2a9308d50a508ed9c26b02373a3965c2a5500
+md5=a81ab4ca80eec8fa1cc72cb5858f7f5a
+cksum=2539330004

which neither matches the filename of the actual download, the extension, nor the md5sum of the download when converted to the correct extension using

gunzip -c Sphinx-1.2.2.tar.gz | bzip2 > sphinx-1.2.2.tar.bz2

[Snark]

I checked that when the sagenb upgrade #16911 will be done, this ticket will go well.

[Snark]

What's the status?

[fbissey]

Are we getting a new sagenb in that can take this and a new docutils?

[Snark]

From what I know, we need a new sagenb(#16911) so we can get a new sphinx(here) so we can get a new docutils(#16733) (in that order). And Volker has added a reverse dep from the new sphinx to the new sagenb to the mix, see #16911.

[fbissey]

OK so #16911 is positively reviewed, it looks like we need to check everything here, ppurka said the checksum is wrong so it has to be checked and given a last test. After we put on a positive review and hope that it get in the next beta/rc. Can you do the work on the checksum I have a couple of other issues I am dealing with.

[Snark]

Ok, I started from ppurka's update_sagenb branch, merged the t/16396/upgrade_sphinx_to_1_2 branch, got the upstream tarball and fixed the checksums as in the ticket description, reinstalled sagenb, reinstalled sphinx, rebuilt sage, then "make ptestlong".

All tests passed.

[ppurka]

You also need to submit the updated checksums to the ticket. Then, you can set this to needs review so that @fbissey can confirm the changes.

[Snark]

I'm actually reviewing the ticket...

[kcrisman]

If there isn't too much left to do here other than this checksum/tarball issue, it would be wonderful to get this and #16911 in the next beta.

[fbissey]

I am working on updating the branch (package name, checksums and patch refresh to eliminate fuzz). I'll also update the description once I am finished. It will then be up to Julien to push the button. I am not doing anything new, just clean up.

[fbissey]

Here it comes....

---

New commits:

​428dcc6

Correct checksum, refresh patch to eliminate the fuzz.

[kcrisman]

Is the change from sphinx to Sphinx going to cause any upgrading troubles?

[strogdon]

Replying to kcrisman:

Is the change from sphinx to Sphinx going to cause any upgrading troubles?

I've tried this and I didn't see an issue. But François may see something.

[fbissey]

We've been there before with another package. The answer is not anymore. Since Julien is not around and Steve tried it I am pushing the button to get on with this. We waited long enough.

[strogdon]

And now to get the sphinx patches out of #16733 so hopefully #16736 can be resolved.

[vbraun]

Breaks pdf docs for me with

! LATEX ERROR: BAD MATH ENVIRONMENT DELIMITER.

SEE THE LATEX MANUAL OR LATEX COMPANION FOR EXPLANATION.
Type  H <return>  for immediate help.
 ...                                              
                                                  
l.222 ... index subgroups of \({\rm SL}_2(\ZZ)\))}
                                                  
? 
! Emergency stop.
 ...                                              
                                                  
l.222 ... index subgroups of \({\rm SL}_2(\ZZ)\))}
                                                  
!  ==> Fatal error occurred, no output PDF file produced!
Transcript written on arithgroup.log.
Makefile:55: recipe for target 'arithgroup.pdf' failed
make[1]: *** [arithgroup.pdf] Error 1

I think the tex code has been around for a while, but the new sphinx doesn't allow these math delimiters. I would just change them to the usual dollar sign or backtick.

[strogdon]

This should have been taken care of with this commit:

​http://git.sagemath.org/sage.git/commit/?id=761be309a895a85c91ec24af8acc69eca0126faf

However the name of the patch file, inline-latex doesn't have a .patch extension and thus it is never applied.

[fbissey]

Darn I missed that. I will take care of this shortly.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​de9630d

Get the right extension for the inline-latex patch

[kcrisman]

New commits:

​de9630d

Get the right extension for the inline-latex patch

[fbissey]

Fixed.

[jhpalmieri]

This works for me.

[slelievre]

The upstream ​sphinx ticket has a question from 2014-07-13, can someone add a comment there?

[strogdon]

Replying to slelievre:

The upstream ​sphinx ticket has a question from 2014-07-13, can someone add a comment there?

I'm the one that commented there. And the suggested patch is included in this ticket. What were you thinking of in terms of comment?

[Snark]

Replying to fbissey:

I am working on updating the branch (package name, checksums and patch refresh to eliminate fuzz). I'll also update the description once I am finished. It will then be up to Julien to push the button. I am not doing anything new, just clean up.

Sorry, I've been away for two weeks, then my browser didn't let me connect to trac (I just found the time to dig and behead the beast). Great work, thanks!

[kcrisman]

<psa>For future reference, folks, please do use/read SPKG.txt. This caused some annoyance in #17265 because of #17369.</psa>