Opened 12 years ago
Closed 12 years ago
#7473 closed defect (fixed)
Sphinx hangs when making a clone
Reported by: | mpatel | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3 |
Component: | documentation | Keywords: | |
Cc: | jhpalmieri, nthiery, ncohen | Merged in: | sage-4.3.rc0 |
Authors: | Mitesh Patel | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This is a follow-up to #6187.
Attachments (4)
Change History (26)
comment:1 Changed 12 years ago by
- Cc jhpalmieri added
comment:2 Changed 12 years ago by
comment:3 Changed 12 years ago by
What if we capture stderr
and stdin
, too, in
proc = subprocess.Popen([cmd], stdout=subprocess.PIPE, shell=True)
? Or do the opposite? For example, builder.py
issues subprocess.call(build_command, shell=True)
, which is shorthand for subprocess.Popen(build_command, shell=True).wait()
. But this may not be relevant.
I'll try to take a closer look soon.
comment:4 Changed 12 years ago by
I've noticed that switching among existing branches via sage -b
, even if I've changed no files, can touch or byte-compile files in SAGE_LOCAL/lib/python/site-packages/sage
. Sphinx notices the changed dependencies and rebuilds the manual.
comment:5 Changed 12 years ago by
It strange that
cd SAGE_ROOT/devel ls -lsFi `find -name environment.pickle`|grep ref
shows the clones to have different Sphinx pickles --- their inodes (the first column on sage.math) are distinct. Compare with
ls -lsFi `find -name steenrod_algebra.html` ls -lsFi `find -name steenrod_algebra.py`|grep -v build
But aren't the pickles hard linked?
comment:6 Changed 12 years ago by
I think this happens because sphinx.environment.BuildEnvironment.topickle
first dumps the environment to a temporary file, then moves it environment.pickle
.
Changed 12 years ago by
Don't capture Sphinx clone output. This may prevent the hang. Apply to scripts repo.
comment:7 Changed 12 years ago by
I think the attached patch for the scripts repository prevents the hang when cloning. The sage repository patch should ensure that we usually keep just one copy of the reference manual's environment.pickle
.
But I'm still not sure about how to avoid rebuilding nearly all of the manual when cloning or after trivially switching branches. The latter may be a separate problem.
Changed 12 years ago by
Use cp -pr
to preserve .rst file times. This may work. Apply only this patch to scripts repo.
comment:8 Changed 12 years ago by
- Milestone set to sage-4.3
- Report Upstream set to N/A
- Status changed from new to needs_review
Version 2 of the scripts repo (i.e., sage-clone
) patch uses cp -pr
instead of shutil.copytree to copy the auto-generated .rst files. Could someone please test this and the sage repo patch? It appears to prevent the hang and unnecessary rebuilds of the reference manual on sage.math.
According to its documentation, shutil.copytree
is very similar to cp -pr
. But their results aren't identical, it seems.
I don't know if cp -pr
is cross-platform.
comment:9 Changed 12 years ago by
#7407 provides the following link, saying that it describes the only options to "cp" which should be used:
http://www.opengroup.org/onlinepubs/009695399/utilities/cp.html
Reading this, I wonder if we should use "cp -pR" instead of "cp -pr".
I made a new clone, applied the patch, built the documentation, and then made another clone. The new cloning process took 2-3 minutes on my iMac running OS X 10.6, and when done the documentation did not need to be rebuilt again. On sage.math, the same thing happened, with the cloning process taking about the same amount of time. In both cases, updating the modification times was quick. Also in both cases, using "cp -pR" worked just as well as "cp -pr".
Shall we take the cited web page as enough evidence that this is cross-platform? And should we change "r" to "R"?
Changed 12 years ago by
Use cp -pR for auto-generated .rst files. Apply only this patch to the scripts repo.
comment:10 Changed 12 years ago by
- Cc nthiery added
Version 3 uses cp -pR
instead of cp -pr
. Does the Windows build environment support cp -pR
?
comment:11 follow-up: ↓ 14 Changed 12 years ago by
- Cc ncohen added
nthiery, ncohen: If you have a chance, could you let us know if the patches above work? In particular,
- Apply trac_7473-sage_builder.patch to the sage repository.
- Apply trac_7473-scripts_clone_v3.patch to the scripts repository.
If this is yet another false positive, I apologize.
comment:12 Changed 12 years ago by
I'm happy with it (Mac OS X 10.6 and sage.math).
On what other platforms does it need to be tested?
comment:13 Changed 12 years ago by
I tried it on my Fedora ( built from sources ) and it applies fine and does its job ( I'm not stuck anymore when cloning ) !
( Even though I can not control your script as I have no idea of how Sage works at this level... ) :-)
Thank you for your patch !!!
Nathann
comment:14 in reply to: ↑ 11 Changed 12 years ago by
Replying to mpatel:
nthiery, ncohen: If you have a chance, could you let us know if the patches above work? In particular,
- Apply trac_7473-sage_builder.patch to the sage repository.
- Apply trac_7473-scripts_clone_v3.patch to the scripts repository.
If this is yet another false positive, I apologize.
I tried sage -combinat install (which calls clone), and it worked smoothly (ubuntu 9.4, sage 4.2.1, macbook pro)!
Thanks!
comment:15 Changed 12 years ago by
- Reviewers set to John Palmieri
- Status changed from needs_review to positive_review
On the grounds that this is an improvement on some systems and I hope isn't any worse on any systems, I'm giving this a positive review. I really would like this to be merged, because cloning is so painful right now.
comment:16 Changed 12 years ago by
- Merged in set to sage-4.3.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
comment:17 Changed 12 years ago by
It seems that the sage repo patch didn't make it into 4.3.alpha1. This patch will prevent some unnecessary doc rebuilds when changing branches.
comment:18 Changed 12 years ago by
- Merged in sage-4.3.alpha1 deleted
- Resolution fixed deleted
- Status changed from closed to new
Oops, I must only seen the last patch. I'll add it first thing to the next release.
comment:19 Changed 12 years ago by
- Status changed from new to needs_review
comment:20 Changed 12 years ago by
- Status changed from needs_review to positive_review
comment:21 Changed 12 years ago by
Thanks!
comment:22 Changed 12 years ago by
- Merged in set to sage-4.3.rc0
- Resolution set to fixed
- Status changed from positive_review to closed
Merged trac_7473-sage_builder.patch in 4.3.rc0.
What if we run
hg clone
, thencp -pr
just the files Sphinx checks?