sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.46k stars 484 forks source link

Enable VPATH builds (several independent build trees connected to one source tree) #21469

Open mkoeppe opened 8 years ago

mkoeppe commented 8 years ago

Executive summary Currently, --prefix selects the install tree (SAGE_LOCAL) of sage-the-distribution. But often we want several builds out of (almost) the same source tree.

The vpath mechanism proposed here selects the build tree (SAGE_ROOT) of sage-the-distribution. This includes the configuration and build artefacts such as src/build/....


VPATH builds are a powerful feature of autotools (and other) build systems. This feature allows the developer to build a package in a separate (initially empty) build directory tree. The source directory tree is only read from (and could even be mounted read-only) and is therefore always clean.

This is a powerful feature for the developer because from the same source tree many different configurations can be built and tested, without having to go through "make distclean" and reconfiguration. The source tree can also be shared, for example using a networked file system between different hosts, running different architectures. Another modern use case involves VMs. For example, Docker allows to mount the source directory from the host in the VM (see #21474).

For Sage, "different configurations" could mean different architectures (via VMs), different sets of installed packages, Py2 vs Py3, etc.

This is how it is used.

 cd SAGE_ROOT
 autoreconf

and then

 mkdir BUILDDIR
 cd BUILDDIR
 SRCDIR/configure --srcdir=SRCDIR
 make

The present patch implements this. BUILDDIR from the above example is reflected in the old environment variable $SAGE_ROOT; this is now to be distinguished from SRCDIR, which is reflected in the new environment variable $SAGE_VPATH. $SAGE_LOCAL defaults to BUILDDIR/local, but can of course be changed using configure --prefix.

The present patch changes the various places where $SAGE_ROOT is used. When "./configure" detects a VPATH build, it installs patched copies of sage, Makefile and build/make/install in BUILDDIR.

With the current set of patches, the compilation goes through and gives a working sage, except for the docbuild.

CC: @sagetrac-felixs @jdemeyer @kiwifb @embray @nexttime @vbraun @dimpase @fchapoton @jhpalmieri

Component: build: configure

Work Issues: merge conflicts

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/vpath_build @ 05f3a91

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

jhpalmieri commented 7 years ago
comment:39

On OS X, some directories are not being created:

$ mkdir BUILD
$ cd BUILD
$ /path/to/sage/configure --srcdir=/path/to/sage
...
/path/to/sage/configure: line 7017: build/make/Makefile: No such file or directory

If I create that directory, configure finishes but with warning messages like this

$ mkdir -p build/make
$ /path/to/sage/configure --srcdir=/path/to/sage
...
touch: /path/to/BUILD/src/bin/sage-env-config: No such file or directory
find: /path/to/BUILD/src/ext: No such file or directory
cat: /path/to/BUILD/build/make/deps: No such file or directory
...
$ make
make: *** No targets specified and no makefile found.  Stop.
mkoeppe commented 7 years ago
comment:40

Did you autoreconf?

jhpalmieri commented 7 years ago

Description changed:

--- 
+++ 
@@ -5,6 +5,12 @@
 For Sage, "different configurations" could mean different architectures (via VMs), different sets of installed packages, Py2 vs Py3, etc.

 This is how it is used.
+
+```
+ cd SAGE_ROOT
+ autoreconf
+```
+and then

mkdir BUILDDIR

jhpalmieri commented 7 years ago
comment:41

No, that fixed it.

jhpalmieri commented 7 years ago
comment:42

With the changes

diff --git a/src/bin/sage-env b/src/bin/sage-env
index dc2bd34cc0..8aa523d7ea 100644
--- a/src/bin/sage-env
+++ b/src/bin/sage-env
@@ -313,7 +313,7 @@ export SAGE_EXTCODE="$SAGE_SHARE/sage/ext"
 export SAGE_SPKG_INST="$SAGE_LOCAL/var/lib/sage/installed"
 export SAGE_LOGS="$SAGE_ROOT/logs/pkgs"
 export SAGE_SRC="$SAGE_ROOT/src"
-export SAGE_DOC_SRC="$SAGE_SRC/doc"
+export SAGE_DOC_SRC="$SAGE_SRC_ROOT/src/doc"
 export SAGE_DOC="$SAGE_SHARE/doc/sage"

 if [ -z "${SAGE_ORIG_PATH_SET}" ]; then
diff --git a/src/doc/common/conf.py b/src/doc/common/conf.py
index 2e115fab88..88f3fdfa4a 100644
--- a/src/doc/common/conf.py
+++ b/src/doc/common/conf.py
@@ -1,10 +1,10 @@
 import sys, os, sphinx
-from sage.env import SAGE_DOC_SRC, SAGE_DOC, SAGE_SRC, THEBE_DIR
+from sage.env import SAGE_DOC_SRC, SAGE_DOC, SAGE_SRC_ROOT, THEBE_DIR
 from datetime import date
 from six import iteritems

 # If your extensions are in another directory, add it here.
-sys.path.append(os.path.join(SAGE_SRC, "sage_setup", "docbuild", "ext"))
+sys.path.append(os.path.join(SAGE_SRC_ROOT, "src", "sage_setup", "docbuild", "ext"))

 # General configuration
 # ---------------------

the documentation starts to build, but I eventually get an error:

[dochtml] [arithgrou] /path/to/SAGE_ROOT/src/doc/en/reference/arithgroup/sage/modular/arithgroup/arithgroup_element.rst:11: WARNING: error while formatting arguments for sage.modular.arithgroup.arithgroup_element.ArithmeticSubgroupElement.b: 'NoneType' object has no attribute 'find'

I can't figure out where this is coming from.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from d0366b0 to e7a78e0

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

db34265Merge tag '8.1.beta3' into t/21469/vpath_build
e7a78e0VPATH fix for docbuild
mkoeppe commented 7 years ago
comment:44

Thanks, I've added your changes as a commit.

mkoeppe commented 7 years ago
comment:45

Replying to @jhpalmieri:

the documentation starts to build, but I eventually get an error:

[dochtml] [arithgrou] /path/to/SAGE_ROOT/src/doc/en/reference/arithgroup/sage/modular/arithgroup/arithgroup_element.rst:11: WARNING: error while formatting arguments for sage.modular.arithgroup.arithgroup_element.ArithmeticSubgroupElement.b: 'NoneType' object has no attribute 'find'

I get the same error

jhpalmieri commented 7 years ago
comment:46

One more change, to help make distclean work:

diff --git a/build/make/deps b/build/make/deps
index 3128f4c656..03f2c8fff1 100644
--- a/build/make/deps
+++ b/build/make/deps
@@ -235,7 +235,7 @@ doc-pdf: $(DOC_DEPENDENCIES)
 doc-clean: doc-src-clean doc-output-clean

 doc-src-clean:
-       cd "$(SAGE_SRC)/doc" && $(MAKE) clean
+       cd "$(SAGE_SRC_ROOT)/src/doc" && $(MAKE) clean

 doc-output-clean:
        rm -rf "$(SAGE_SHARE)/doc/sage"
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from e7a78e0 to 734fb4b

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

734fb4bVPATH fix for doc-src-clean
mkoeppe commented 7 years ago
comment:48

Perhaps the sphinx problem is related to failing source discovery for Cython modules:

sage: sage.rings.number_field.number_field_element_quadratic??
Error getting source: s must be a string
Type:            module
...
jhpalmieri commented 7 years ago
comment:49

Yes, that looks like it.

jhpalmieri commented 7 years ago
comment:50

Okay, how about this:

diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
index f9dffc1ce3..3a286c7934 100644
--- a/src/sage/misc/sageinspect.py
+++ b/src/sage/misc/sageinspect.py
@@ -125,7 +125,7 @@ import tokenize
 import types
 import re
 EMBEDDED_MODE = False
-from sage.env import SAGE_SRC
+from sage.env import SAGE_SRC_ROOT

 def loadable_module_extension():
     r"""
@@ -222,7 +222,7 @@ def _extract_embedded_position(docstring):
         return None

     raw_filename = res.group('FILENAME')
-    filename = os.path.join(SAGE_SRC, raw_filename)
+    filename = os.path.join(SAGE_SRC_ROOT, 'src', raw_filename)
     if not os.path.isfile(filename):
         from sage.misc.misc import SPYX_TMP
         filename = os.path.join(SPYX_TMP, '_'.join(raw_filename.split('_')[:-1]), raw_filename)
@@ -2308,7 +2308,7 @@ def __internal_tests():

     Test _extract_embedded_position:

-    We cannot test the filename since it depends on SAGE_SRC.
+    We cannot test the filename since it depends on SAGE_SRC_ROOT.

     Make sure things work with no trailing newline::
jhpalmieri commented 7 years ago
comment:51

Now the cross-references are wrong or missing:

[dochtml] OSError: [parallel ] /.../sage-8.1.beta3/src/doc/en/reference/parallel/index.rst:17: WARNING: undefined label: sage.interfaces.psage (if the link has no caption the label must precede a section header)
jhpalmieri commented 7 years ago
comment:52

Also, it seems that the docbuilding doesn't recognize when a document has been built already, so everything is rebuilt every time.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

8c801f7VPATH: Set SAGE_SRC to actual source tree throughout
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 734fb4b to 8c801f7

mkoeppe commented 7 years ago
comment:54

Replying to @jhpalmieri:

Okay, how about this:

diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
index f9dffc1ce3..3a286c7934 100644
--- a/src/sage/misc/sageinspect.py
+++ b/src/sage/misc/sageinspect.py
@@ -222,7 +222,7 @@ def _extract_embedded_position(docstring):
         return None

     raw_filename = res.group('FILENAME')
-    filename = os.path.join(SAGE_SRC, raw_filename)
+    filename = os.path.join(SAGE_SRC_ROOT, 'src', raw_filename)
     if not os.path.isfile(filename):
         from sage.misc.misc import SPYX_TMP
         filename = os.path.join(SPYX_TMP, '_'.join(raw_filename.split('_')[:-1]), raw_filename)

Instead, I have changed SAGE_SRC to point to the actual source tree. This was inconsistent before.

mkoeppe commented 7 years ago
comment:55

I'm now testing the VPATH build using a separate user account that cannot write into the source tree. More changes to the docbuild are necessary to separate sources from built files:

[dochtml] IOError: [Errno 13] Permission denied: '/Users/mkoeppe/s/sage/sage-rebasing/vpath/B/../src/doc/en/reference/repl/sage/misc/trace.rst'
jhpalmieri commented 7 years ago
comment:56

By the way, I get warnings like

touch: /.../BUILDDIR/src/bin/sage-env-config: No such file or directory

I assume this isn't a big deal since you're planning on getting rid of this file. It also doesn't seem to interfere with the build or running Sage.

mkoeppe commented 7 years ago
comment:57

I don't plan to eliminate sage-env-config (perhaps you mean sage-arch-env, #23733)?

jhpalmieri commented 7 years ago
comment:58

Sorry, yes, that's what I meant, and that's what most of the warnings have been about.

mkoeppe commented 7 years ago
comment:59

The sage-arch-env message is harmless. But there's indeed also something wrong with sage-env-config too. Fix will come shortly.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 8c801f7 to 5530827

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

5530827sage-env-config: Handle special case in build/make/deps, rather than configure.ac
jhpalmieri commented 7 years ago
comment:61

I'm getting this build error:

make[2]: *** No rule to make target `/.../BUILDDIR/local/share/sage/ext/doctest/invalid/syntax_error.tachyon', needed by `sagelib'.  Stop.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 5530827 to 64515c1

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

64515c1Fix last change
jhpalmieri commented 7 years ago
comment:63

The make target "sage-starts" fails for me. This fixes it; is it the right way to fix it?

diff --git a/src/bin/sage-starts b/src/bin/sage-starts
index 85cbae67e5..498c930810 100755
--- a/src/bin/sage-starts
+++ b/src/bin/sage-starts
@@ -14,7 +14,7 @@ echo "[`date +'%Y-%m-%d %H:%M:%S'`] `cat VERSION.txt 2>/dev/null`" | tee -a logs
 # sysadmin.  We use --nodotsage so we don't force a .sage directory
 # in the sysadmin's HOME directory.
 cmd='sage.all._write_started_file(); print "Yes, Sage starts."'
-build/bin/sage-logger "./sage --nodotsage -c '$cmd'" logs/start.log
+"$SAGE_SRC_ROOT"/build/bin/sage-logger "./sage --nodotsage -c '$cmd'" logs/start.log

 if [ $? -ne 0 ]; then
     echo >&2 "Sage failed to start up."
jhpalmieri commented 7 years ago
comment:64

With that change, make ptestlong fails:

Testing that Sage starts...
[2017-08-30 09:19:29] 
This looks like the first time you are running Sage.
Cleaning up, do not interrupt this.
Done cleaning.
Yes, Sage starts.

real    52m33.269s
user    191m2.435s
sys 19m38.034s
Sage build/upgrade complete!

To install small scripts to directly run Sage's versions of GAP,
the PARI/GP interpreter, Maxima, or Singular etc. (by typing e.g.
just 'gap' or 'gp') into a standard 'bin' directory, start Sage
by typing 'sage' (or './sage') and enter something like

    install_scripts('/usr/local/bin')

at the Sage command prompt ('sage:').

/.../SAGE_SRC_ROOT/sage -t -p --all --long --logfile=logs/ptestlong.log
************************************************************************
It seems that you are attempting to run Sage from an unpacked source
tarball, but you have not compiled it yet (or maybe the build has not
finished). You should run `make` in the Sage root directory first.
If you did not intend to build Sage from source, you should download
a binary tarball instead. Read README.txt for more information.
************************************************************************
make: *** [ptestlong] Error 1

I think that the make target

TESTALL = $(top_srcdir)/sage -t --all

needs to be changed so that it runs SAGE_ROOT/sage instead of SAGE_SRC_ROOT/sage.

./sage -t -all also fails:

$ ./sage -t --all
/.../SAGE_SRC_ROOT/src/bin/sage-env: line 527: /.../BUILDDIR/src/bin/sage-arch-env: No such file or directory
Traceback (most recent call last):
  File "/.../SAGE_SRC_ROOT/src/bin/sage-runtests", line 97, in <module>
    DC = DocTestController(options, args)
  File "/.../BUILDDIR/local/lib/python2.7/site-packages/sage/doctest/control.py", line 334, in __init__
    for pkg in list_packages('optional', local=True).values():
  File "/.../BUILDDIR/local/lib/python2.7/site-packages/sage/misc/package.py", line 230, in list_packages
    for p in os.listdir(SAGE_PKGS):
OSError: [Errno 2] No such file or directory: '/.../BUILDDIR/build/pkgs'

This can almost be fixed by

diff --git a/src/sage/env.py b/src/sage/env.py
index 33cd754a06..f91c70ba7d 100644
--- a/src/sage/env.py
+++ b/src/sage/env.py
@@ -111,7 +111,7 @@ _add_variable_or_fallback('SAGE_LIB',        SITE_PACKAGES[0])
 _add_variable_or_fallback('SAGE_CYTHONIZED', opj('$SAGE_ROOT', 'src', 'build', 'cythonized'))

 # Used by sage/misc/package.py.  Should be SAGE_SRC_ROOT in VPATH.
-_add_variable_or_fallback('SAGE_PKGS', opj('$SAGE_ROOT', 'build', 'pkgs'))
+_add_variable_or_fallback('SAGE_PKGS', opj('$SAGE_SRC_ROOT', 'build', 'pkgs'))

 _add_variable_or_fallback('SAGE_EXTCODE',    opj('$SAGE_SHARE', 'sage', 'ext'))

but there is a bug in the function _add_variable_or_fallback in how it parses variable names prefixed with dollar signs. I'll open a new ticket for that.

jhpalmieri commented 7 years ago
comment:65

If I hack the top-level Makefile so that make ptestlong works, then I only get test failures in sage/tests/cmdline.py.

jhpalmieri commented 7 years ago
comment:66

Replying to @jhpalmieri:

there is a bug in the function _add_variable_or_fallback in how it parses variable names prefixed with dollar signs. I'll open a new ticket for that.

See #23758.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

bc6263cVPATH fix for SAGE_PKGS
f9c25bcVPATH fix for test makefile targets
ecd2f9ddoc-src-clean: Clean in source directory until docbuild is fully VPATHified
60927e5trac 23758: in env.py, _add_variable_or_fallback should depend less
c23cc1ctrac 23758: restore "import os"
343d685trac 23758: add a doctest
3e36b75Merge remote-tracking branch 'trac/u/jhpalmieri/variable_replacement' into t/21469/vpath_build
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 64515c1 to 3e36b75

mkoeppe commented 7 years ago
comment:68

I'm still getting docbuild errors of the type WARNING: undefined label: sage.interfaces.psage (if the link has no caption the label must precede a section header)

dimpase commented 7 years ago
comment:69

Replying to @mkoeppe:

I'm still getting docbuild errors of the type WARNING: undefined label: sage.interfaces.psage (if the link has no caption the label must precede a section header)

This is a general problem, nothing to worry in the context of this ticket.

By the way you might like to use here the upgraded sphinx on #23023, which will most probably be in the next beta.

jhpalmieri commented 7 years ago
comment:70

I agree that the docbuild warnings are not new. (My issue in comment:51 was that these were errors, causing docbuilding to actually fail. I am now able to build the docs successfully.)

I am still having the problem in comment:63 with sage-starts.

mkoeppe commented 7 years ago
comment:71

Replying to @jhpalmieri:

I agree that the docbuild warnings are not new. (My issue in comment:51 was that these were errors, causing docbuilding to actually fail. I am now able to build the docs successfully.)

Unfortunately these warnings get upgraded to errors like this:

[dochtml] OSError: [parallel ] /Users/mkoeppe/s/sage/sage-rebasing/vpath/src/doc/en/reference/parallel/index.rst:17: WARNING: undefined label: sage.interfaces.psage (if the link has no caption the label must precede a section header)

which stops this build for me.

I'll try with #23023 as suggested by Dima.

jhpalmieri commented 7 years ago
comment:72

Replying to @mkoeppe:

Replying to @jhpalmieri:

I agree that the docbuild warnings are not new. (My issue in comment:51 was that these were errors, causing docbuilding to actually fail. I am now able to build the docs successfully.)

Unfortunately these warnings get upgraded to errors like this:

[dochtml] OSError: [parallel ] /Users/mkoeppe/s/sage/sage-rebasing/vpath/src/doc/en/reference/parallel/index.rst:17: WARNING: undefined label: sage.interfaces.psage (if the link has no caption the label must precede a section header)

which stops this build for me.

Your change in comment:53 fixed that issue for me. (At least I think that's what fixed it.)

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

754ec81sage-starts: Fix sage-logger invocation for VPATH build
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 3e36b75 to 754ec81

mkoeppe commented 7 years ago
comment:74

Replying to @jhpalmieri:

I am still having the problem in comment:63 with sage-starts.

I agree with your patch and have added a commit; but see #23769.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 754ec81 to 0c357dd

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

0c357ddSimplify VPATH patch
mkoeppe commented 7 years ago

Description changed:

--- 
+++ 
@@ -22,8 +22,10 @@
 The present patch implements this. BUILDDIR from the above example is reflected in the old environment variable $SAGE_ROOT; this is now to be distinguished from SRCDIR, which is reflected in the new environment variable $SAGE_SRC_ROOT. $SAGE_LOCAL defaults to BUILDDIR/local, but can of course be changed using `configure --prefix`.

 The present patch changes the various places where $SAGE_ROOT is used.
-When "./configure" detects a VPATH build, it installs patched copies of `sage`, `Makefile` and `build/make/install` in BUILDDIR. 
+When "./configure" detects a VPATH build, it installs patched copies of `sage` and `Makefile` in BUILDDIR. 

-With the current set of patches, the compilation goes through and gives a working `sage`, except for the docbuild.
+With the current set of patches, the compilation goes through and gives a working `sage`.
+However, the source tree is not kept quite clean yet. Some generated sources and files generated by the docbuild are put in the source tree. This should be fixed in follow-up tickets.

+
mkoeppe commented 7 years ago

Changed author from Matthias Koeppe to Matthias Koeppe, John H. Palmieri

mkoeppe commented 7 years ago
comment:77

I think this ticket is ready for broader review.

jhpalmieri commented 7 years ago

Changed author from Matthias Koeppe, John H. Palmieri to Matthias Koeppe