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

Sphinx hangs when making a clone #7473

Closed
qed777 mannequin opened this issue Nov 16, 2009 · 27 comments
Closed

Sphinx hangs when making a clone #7473

qed777 mannequin opened this issue Nov 16, 2009 · 27 comments

Comments

@qed777
Copy link
Mannequin

qed777 mannequin commented Nov 16, 2009

This is a follow-up to #6187.

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

CC: @jhpalmieri @nthiery @nathanncohen

Component: documentation

Author: Mitesh Patel

Reviewer: John Palmieri

Merged: sage-4.3.rc0

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

@qed777 qed777 mannequin added c: documentation labels Nov 16, 2009
@qed777 qed777 mannequin assigned sagetrac-mvngu Nov 16, 2009
@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 18, 2009

comment:2

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

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 18, 2009

comment:3

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.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 19, 2009

comment:4

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.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 20, 2009

comment:5

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?

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 20, 2009

comment:6

I think this happens because sphinx.environment.BuildEnvironment.topickle first dumps the environment to a temporary file, then moves it environment.pickle.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 20, 2009

Attachment: trac_7473-sage_builder.patch.gz

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

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 20, 2009

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

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 20, 2009

comment:7

Attachment: trac_7473-scripts_clone.patch.gz

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.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 22, 2009

Attachment: trac_7473-scripts_clone_v2.patch.gz

Use cp -pr to preserve .rst file times. This may work. Apply only this patch to scripts repo.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 22, 2009

Author: Mitesh Patel

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 22, 2009

comment:8

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.

@qed777 qed777 mannequin added this to the sage-4.3 milestone Nov 22, 2009
@qed777 qed777 mannequin added the s: needs review label Nov 22, 2009
@jhpalmieri
Copy link
Member

comment:9

#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"?

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 23, 2009

Attachment: trac_7473-scripts_clone_v3.patch.gz

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

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 23, 2009

comment:10

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

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Nov 23, 2009

comment:11

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.

@jhpalmieri
Copy link
Member

comment:12

I'm happy with it (Mac OS X 10.6 and sage.math).

On what other platforms does it need to be tested?

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Nov 24, 2009

comment:13

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

@nthiery
Copy link
Contributor

nthiery commented Nov 24, 2009

comment:14

Replying to @qed777:

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!

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:15

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.

@mwhansen
Copy link
Contributor

Merged: sage-4.3.alpha1

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Dec 4, 2009

comment:17

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.

@mwhansen
Copy link
Contributor

mwhansen commented Dec 4, 2009

comment:18

Oops, I must only seen the last patch. I'll add it first thing to the next release.

@mwhansen
Copy link
Contributor

mwhansen commented Dec 4, 2009

Changed merged from sage-4.3.alpha1 to none

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Dec 4, 2009

comment:21

Thanks!

@mwhansen
Copy link
Contributor

mwhansen commented Dec 6, 2009

comment:22

Merged trac_7473-sage_builder.patch in 4.3.rc0.

@mwhansen
Copy link
Contributor

mwhansen commented Dec 6, 2009

Merged: sage-4.3.rc0

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

3 participants