Optionally build spkgs in $SAGE_BUILD_DIR

[sagetrac-mabshoff]

$SAGE_ROOT/spkg/build can be slow in case it is NFS-mounted for example. So using local scratch space or even better a RAM disk should speed up the build by a nice factor. To do so, use $SAGE_BUILD_DIR instead of $SAGE_ROOT/spkg/build/.

---

Apply attachment: trac_4949-root.v5.patch and attachment: 4949_review.patch to the Sage root repository.

Apply attachment: trac_4949-installation.v3.patch to the Sage library.

CC: @sagetrac-drkirkby @nexttime

Component: build

Keywords: sd32

Author: John Palmieri

Reviewer: Mariah Lenox, Leif Leonhardy, Maarten Derickx, Jeroen Demeyer

Merged: sage-5.0.beta5

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

[williamstein]

comment:2

As a temporary hack to see how this "feels" you could delete spkg/build, then make it a symlink to /tmp/build/.

[jhpalmieri]

Author: John Palmieri

[jhpalmieri]

comment:3

Here's a patch. This implements both SAGE_BUILD_TMPDIR and SAGE_KEEP_BUILT_SPKGS -- see #9444. (This is an incremental change rather than a complete reworking of the sage-spkg script, which might be called for.)

[jhpalmieri]

comment:4

A little explanation: BUILD is defined (as "build") by sage-env, but it was used sporadically in sage-spkg. With this patch, it is used more consistently.

[qed777]

comment:5

If/when this merges, we should consider closing #6550. SAGE_KEEP_BUILT_SPKGS is a much better name than SAGE_KEEP_SPKG_BUILD.

[nexttime]

comment:6

Nice to see progress in the build process... :)

See also #6550 comment:7 .

[nexttime]

comment:7

Should SAGE_BUILD_TMPDIR default to SAGE_TMPDIR?

(We have btw. lots of - in some cases not very well-named - environment variables.)

Making use of e.g. a RAM disk (or some user-provided directory) for doctesting is also worth doing.

[qed777]

comment:8

Replying to @nexttime:

Making use of e.g. a RAM disk (or some user-provided directory) for doctesting is also worth doing.

You can already set SAGE_TESTDIR (or DOT_SAGE) to do this. Or maybe I misunderstand?

[sagetrac-mariah]

comment:9

[attachment: trac_4949-scripts.patch] does not apply:

sage: hg_sage.apply("/home/mariah/trac_4949-scripts.patch")
cd "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-core2-fc/devel/sage" && hg status
cd "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-core2-fc/devel/sage" && hg status
cd "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-core2-fc/devel/sage" && hg import   "/home/mariah/trac_4949-scripts.patch"
applying /home/mariah/trac_4949-scripts.patch
unable to find 'sage-spkg' for patching
5 out of 5 hunks FAILED -- saving rejects to file sage-spkg.rej
abort: patch failed to apply
sage: 

[jhpalmieri]

comment:10

Here's a rebased version of attachment: trac_4949-scripts.patch. Note that it's for the scripts repository, so you have to apply it with "hg_scripts.apply(...)" rather than "hg_sage.apply(...)".

[sagetrac-mariah]

comment:11

I applied the patch attachment: trac_4949-scripts.patch, then moved
the modified sage-spkg file to a fresh source directory of sage-4.7.rc4. I set SAGE_BUILD_TMPDIR and SAGE_KEEP_BUILT_SPKGS, and
did 'make testlong'. The builds took place in the location SAGE_BUILD_TMPDIR and all tests passed. I applied the patch [attachment: trac_4949-installation.patch], did 'sage -b', then 'sage -docbuild installation html' and verified that the documentation change was made and makes sense. Positive review.

[sagetrac-mariah]

Reviewer: Mariah Lenox

[jdemeyer]

comment:50

Lines 210 and 216 of sage-spkg: you have twice cd "$SAGE_BUILD_DIR". Remove the second and move the check (line 219) up, after the first cd. You probably want to "exit 1" if cd fails.

Line 227 of sage-spkg: replace

if [ -e "$dir" ]; then

by

if [ -d "$dir" ]; then

[jdemeyer]

comment:51

Line 235: why "mv -f" and not simply "mv"?

[jhpalmieri]

comment:52

Replying to @jdemeyer:

Lines 210 and 216 of sage-spkg: you have twice cd "$SAGE_BUILD_DIR". Remove the second and move the check (line 219) up, after the first cd. You probably want to "exit 1" if cd fails.

Well, the old version had a second 'cd' command, justified by the comment

# Make triply sure that we are in the build directory before doing  
# a scary "rm -rf"

So I left the second one in. You think I should change this? In any case, you're right about the "exit 1".

Line 227 of sage-spkg: replace

if [ -e "$dir" ]; then

by

if [ -d "$dir" ]; then

On the off-chance that there is a file (not a directory) in the build directory with the wrong name, shouldn't we move it, too?

Replying to @jdemeyer:

Line 235: why "mv -f" and not simply "mv"?

Left over from the previous version. I can fix that.

[jhpalmieri]

Attachment: trac_4949-root-delta4to5.patch.gz

[jhpalmieri]

comment:53

Attachment: trac_4949-root.v5.patch.gz

[jdemeyer]

comment:54

Attachment: 4949_review.patch.gz

[jdemeyer]

Changed reviewer from Mariah Lenox, Leif Leonhardy, Maarten Derickx to Mariah Lenox, Leif Leonhardy, Maarten Derickx, Jeroen Demeyer

[jhpalmieri]

comment:56

The review patch looks okay to me.

[jdemeyer]

comment:57

Looks good to me too.

[jdemeyer]

Merged: sage-5.0.beta5

[jhpalmieri]

comment:59

See #12637 for a followup.