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

Don't delete built Sage documentation until Sphinx has been successfully (re)installed #11665

Closed
nexttime mannequin opened this issue Aug 8, 2011 · 26 comments
Closed

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 8, 2011

As the title says.


New spkg: http://spkg-upload.googlecode.com/files/sphinx-1.0.4.p8.spkg

md5sum: 5529e918d8a73e1476c0366fd5fcee35 sphinx-1.0.4.p8.spkg

sphinx-1.0.4.p8 (Leif Leonhardy, August 17th 2011)

  • Don't delete built Sage documentation until Sphinx has been successfully (re)installed #11665: Don't delete previously built Sage documentation (in
    $SAGE_ROOT/devel/sage-*/doc/output/) until Sphinx has been successfully
    reinstalled.
  • Also, don't delete a previous Sphinx installation until Sphinx has been
    successfully (re)built. To achieve this, we now first run 'python
    setup.py build', then delete any previously installed version, and after
    that run 'python setup.py install'.
  • Quote the filename of patches.
  • Some clean-up, add some messages (and blank lines to separate the output).
  • Added some more of Sphinx' dependencies, corrected minimal version numbers
    required.

Depends on #11659

Component: packages: standard

Keywords: spkg doc/output html latex

Author: Leif Leonhardy

Reviewer: John Palmieri

Merged: sage-4.7.2.alpha2

Issue created by migration from https://trac.sagemath.org/ticket/11665

@nexttime nexttime mannequin added this to the sage-4.7.2 milestone Aug 8, 2011
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 8, 2011

comment:1

I wonder if we should delete previously built Sage documentation at all (in all devel branches btw.), since rebuilding the documentation takes a fair amount of time, and if someone's going to rebuild it (from scratch), he'll certainly be smart enough to delete whatever generated output he wants (or needs) to.

The current behaviour is IMHO somewhat surprising, at least in that it affects all branches as mentioned above.

And worse, this even happens whenever Sphinx gets reinstalled just because some of its (direct or indirect) dependencies changed (when using make with SAGE_UPGRADING=yes to also rebuild all dependent packages).

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 8, 2011

Author: Leif Leonhardy

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 8, 2011

comment:2

New spkg is up.

@nexttime nexttime mannequin added the s: needs review label Aug 8, 2011
@jhpalmieri
Copy link
Member

comment:3

I know this isn't your change, but setup.py says

requires = ['Pygments>=0.8', 'Jinja2>=2.2', 'docutils>=0.5']

So why are the prerequisites "Pygments >= 1.3.1" and "docutils >= 0.4"?

The changes look okay at first glance, otherwise. I'll keep looking at them, as should other people.

@jhpalmieri
Copy link
Member

comment:4

Another pre-existing condition (i.e., this was a problem with the previous version of the spkg):

Processing dependencies for Sphinx==1.0.4
Searching for docutils==0.7
Best match: docutils 0.7
Adding docutils 0.7 to easy-install.pth file

Using /Applications/sage_builds/sage-4.7.1.rc2/local/lib/python2.6/site-packages

Why wasn't docutils in easy-install.pth already? The other prerequisites look okay, for example

Searching for Jinja2==2.5.5
Best match: Jinja2 2.5.5
Processing Jinja2-2.5.5-py2.6.egg
Jinja2 2.5.5 is already the active version in easy-install.pth

Using /Applications/sage_builds/sage-4.7.1.rc2/local/lib/python2.6/site-packages/Jinja2-2.5.5-py2.6.egg

Looking at easy-install.pth, indeed there is no entry for docutils, either before or after (!) the Sphinx installation. Before installing Sphinx, this is perhaps the fault of the docutils spkg, although I'm not sure how to fix it. After installing Sphinx, either there is a problem with the Sphinx installation process, or the line "Adding docutils 0.7 to easy-install.pth file" is just wrong.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

comment:5

Replying to @jhpalmieri:

Why wasn't docutils in easy-install.pth already? [...]

Looking at easy-install.pth, indeed there is no entry for docutils, either before or after (!) the Sphinx installation. Before installing Sphinx, this is perhaps the fault of the docutils spkg, although I'm not sure how to fix it. After installing Sphinx, either there is a problem with the Sphinx installation process, or the line "Adding docutils 0.7 to easy-install.pth file" is just wrong.

Perhaps attachment: ticket:10176:easy_install.py.debug.patch helps in debugging this.

(I don't have an entry for docutils in easy-install.pth in any Sage installation since [at least] Sage 4.5.3; haven't checked earlier versions just because they're located on other machines or filesystems.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

comment:6

Replying to @nexttime:

(I don't have an entry for docutils in easy-install.pth in any Sage installation since [at least] Sage 4.5.3; haven't checked earlier versions just because they're located on other machines or filesystems.)

FWIW, in no Sage installation since [at least] Sage 4.2.1, so I guess it's never been there.

@jhpalmieri
Copy link
Member

comment:7

Do you understand why the Sphinx installation process says "Adding docutils 0.7 to easy-install.pth file" but doesn't actually do anything?

Otherwise, I'm happy with the changes, except that the list of prerequisites in SPKG.txt should be changed (or explained, if it should stay as is). The issue with docutils and easy-install.pth can be deferred to another ticket, if it even needs to be addressed at all. (Sphinx seems to work, so I don't know how important it is.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

comment:8

Replying to @jhpalmieri:

Do you understand why the Sphinx installation process says "Adding docutils 0.7 to easy-install.pth file" but doesn't actually do anything?

Well, Sphinx just uses distutils / distribute, and IIRC that prints that message.

A problem we previously had was that packages found in Python's current path won't get added, despite of the message (which is printed in advance).

Otherwise, I'm happy with the changes, except that the list of prerequisites in SPKG.txt should be changed (or explained, if it should stay as is).

Yes, I can correct them. AFAIK a couple of spkgs have rather "random" numbers as the minimal version of some other package required; usually simply the current version in Sage is taken. (And not even the upstream pages always give the correct versions required, some not even the full list of requirements.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

Reviewer: John Palmieri

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

comment:9

P.S.:

As long as the packages are installed in .../site-packages/<package-basename>/ (like docutils are), this doesn't matter or cause any trouble.

Just the message is perhaps confusing.

@jhpalmieri
Copy link
Member

comment:10

Okay. Once you upgrade the list of prerequisites, I'll give this a positive review. It doesn't sound like it's necessary to open a follow-up ticket on docutils and easy-install.pth.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

Diff between Jeroens's p7 spkg (#11659) and the p8. For reference / review.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

comment:11

Attachment: sphinx-1.0.4.p7-p8.diff.gz

diff -r 2febeec86a8f -r be96c2edf57b SPKG.txt
--- a/SPKG.txt	Mon Aug 08 17:46:04 2011 +0200
+++ b/SPKG.txt	Wed Aug 17 01:17:37 2011 +0200
@@ -11,7 +11,7 @@
 
 == License ==
 
-Modified BSD
+Modified BSD; see e.g. its egg-info file for other options
 
 == SPKG Maintainers ==
 
@@ -19,16 +19,17 @@
 
 == Upstream Contact ==
 
-Author:  Georg Brandl <georg at python org>
-Home Page: http://sphinx.pocoo.org
+Author: Georg Brandl <georg at python org>
+Home Page: http://sphinx.pocoo.org, 
+  see also http://pypi.python.org/pypi/Sphinx
 
 == Dependencies ==
- * Jinja >= 2.2
- * Pygments >= 1.3.1
- * docutils >= 0.4
- * distutils
+ * Jinja2 >= 2.2
+ * Pygments >= 0.8
+ * docutils >= 0.5
+ * setuptools / distribute
  * Python
- * GNU patch
+ * GNU patch (shipped with Sage)
 
 == Special Update/Build Instructions ==
 
@@ -63,7 +64,7 @@
 
 == Changelog ==
 
-=== sphinx-1.0.4.p8 (Leif Leonhardy, August 8th 2011) ===
+=== sphinx-1.0.4.p8 (Leif Leonhardy, August 17th 2011) ===
  * #11665: Don't delete previously built Sage documentation (in
    $SAGE_ROOT/devel/sage-*/doc/output/) until Sphinx has been successfully
    reinstalled.
@@ -73,7 +74,8 @@
    that run 'python setup.py install'.
  * Quote the filename of patches.
  * Some clean-up, add some messages (and blank lines to separate the output).
- * Added some more of Sphinx' dependencies.
+ * Added some more of Sphinx' dependencies, corrected minimal version numbers
+   required.
 
 === sphinx-1.0.4.p7 (Jeroen Demeyer, 2011-08-08) ===
  * Ticket #11659: Increase LaTeX SAVE_SIZE from 20000 to 50000

I've updated the attached diff (between the p7 and the p8); I'll replace the spkg on Google code soon.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

comment:12

Replying to @nexttime:

I'll replace the spkg on Google code soon.

New spkg is up now. Funny how many people already downloaded the previous one.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 16, 2011

comment:13

P.S.: I still wonder whether we should delete previously built Sage documentation at all, and if so, of which branch(es).

I believe doing so has more potential of annoyance than not doing so.

@jhpalmieri
Copy link
Member

comment:14

Replying to @nexttime:

P.S.: I still wonder whether we should delete previously built Sage documentation at all, and if so, of which branch(es).

I believe doing so has more potential of annoyance than not doing so.

There have certainly been cases where old doctree directories were incompatible between Sphinx versions, resulting in crashes for sage -docbuild ....

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 17, 2011

comment:15

Replying to @jhpalmieri:

Replying to @nexttime:

I believe doing so has more potential of annoyance than not doing so.

There have certainly been cases where old doctree directories were incompatible between Sphinx versions, resulting in crashes for sage -docbuild ....

Well, that's easily fixed by a rm -rf devel/sage/doc/output; the deletion of work which perhaps took hours (or more) of CPU time isn't.

@jhpalmieri
Copy link
Member

comment:16

If #6495 gets reviewed and merged, it will speed up generation of the reference manual on machines with many processors. So deleting the output wouldn't be as much of an issue then. (The reference manual is the real bottleneck; everything else takes just a few minutes.)

The deletion of the output was introduced in #7286. Some options:

  • Delete the output, as is done now.
  • Don't delete the output, but print a message after installation about potential problems. Such a message may get lost among many others during a run of sage -upgrade.
  • Delete the output if we've updated the version of Sphinx (which we can tell by looking at the name of the Sphinx directory in SAGE_LOCAL/lib/python/site-packages), but not otherwise. But Sage-specific changes in Sphinx, which wouldn't affect the Sphinx version number, could conceivably lead to incompatible pickles.

The first of these is in some sense the safest, since it is the status quo. It will make some operations take more time, but it won't cause things to fail in confusing ways. The others could lead to failures and confusing error messages. So your comment that deleting the docs has "more potential of annoyance than not doing so" depends on your point of view: is it better to make things work but take longer, or to have them be faster but possibly fail in confusing ways.

Speaking of confusing error messages, for a while now, running "sage -docbuild tutorial pickle" (or replace 'pickle' by 'json') has failed...

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 17, 2011

comment:17

The funny thing is that #6495's description says:

"Building the Sage reference manual can use significant computer resources. Easing the burden could speed up Sage development."

Building the documentation in parallel doesn't take less resources, but even more. It only makes things faster if you have more resources available.

@jhpalmieri
Copy link
Member

comment:18

That's a bit misleading. Building the entire reference manual in one piece takes more memory, for example, than building it in chunks — it doesn't scale up well — so building with two threads should use less memory, even with both threads combined, than building before the patches at #6495. This is especially true when building in LaTeX format, it seems to me. Based on my anecdotal experiences: building, say, the PDF version of the reference manual was pretty painful before the patches on my home computer: it slowed other things down noticeably. Building after the patches is more pleasant.

(I didn't write the description at #6495, for what that's worth.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 17, 2011

comment:19

Replying to @jhpalmieri:

(I didn't write the description at #6495, for what that's worth.)

I know, Mitesh did.

@jhpalmieri
Copy link
Member

comment:20

The current spkg gets a positive review.

@jdemeyer
Copy link

Merged: sage-4.7.2.alpha2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants