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:

Status badges

Description

This is a follow-up to #6187.

See sage-devel, sage-release, #sage-devel log.

Attachments (4)

trac_7473-sage_builder.patch (1.9 KB) - added by mpatel 12 years ago.
Make pickle saving preserve the hard link. Apply to sage repo.
trac_7473-scripts_clone.patch (730 bytes) - added by mpatel 12 years ago.
Don't capture Sphinx clone output. This may prevent the hang. Apply to scripts repo.
trac_7473-scripts_clone_v2.patch (1.5 KB) - added by mpatel 12 years ago.
Use cp -pr to preserve .rst file times. This may work. Apply only this patch to scripts repo.
trac_7473-scripts_clone_v3.patch (2.1 KB) - added by mpatel 12 years ago.
Use cp -pR for auto-generated .rst files. Apply only this patch to the scripts repo.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 12 years ago by jhpalmieri

  • Cc jhpalmieri added

comment:2 Changed 12 years ago by mpatel

What if we run hg clone, then cp -pr just the files Sphinx checks?

comment:3 Changed 12 years ago by mpatel

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 mpatel

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 mpatel

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 mpatel

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 mpatel

Make pickle saving preserve the hard link. Apply to sage repo.

Changed 12 years ago by mpatel

Don't capture Sphinx clone output. This may prevent the hang. Apply to scripts repo.

comment:7 Changed 12 years ago by mpatel

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 mpatel

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 mpatel

  • Authors set to Mitesh Patel
  • 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 jhpalmieri

#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 mpatel

Use cp -pR for auto-generated .rst files. Apply only this patch to the scripts repo.

comment:10 Changed 12 years ago by mpatel

  • Cc nthiery added

Version 3 uses cp -pR instead of cp -pr. Does the Windows build environment support cp -pR?

comment:11 follow-up: Changed 12 years ago by mpatel

  • Cc ncohen added

nthiery, ncohen: If you have a chance, could you let us know if the patches above work? In particular,

If this is yet another false positive, I apologize.

comment:12 Changed 12 years ago by jhpalmieri

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 ncohen

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 nthiery

Replying to mpatel:

nthiery, ncohen: If you have a chance, could you let us know if the patches above work? In particular,

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 jhpalmieri

  • 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 mhansen

  • 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 mpatel

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 mhansen

  • 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 mhansen

  • Status changed from new to needs_review

comment:20 Changed 12 years ago by mhansen

  • Status changed from needs_review to positive_review

comment:21 Changed 12 years ago by mpatel

Thanks!

comment:22 Changed 12 years ago by mhansen

  • 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.

Note: See TracTickets for help on using tickets.