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

Make sagelib setup.py self-contained and independent of SAGE_ROOT #21480

Closed
mkoeppe opened this issue Sep 12, 2016 · 162 comments
Closed

Make sagelib setup.py self-contained and independent of SAGE_ROOT #21480

mkoeppe opened this issue Sep 12, 2016 · 162 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 12, 2016

This ticket changes the build process of sagelib in the following way:

  • src/Makefile delegates ALL building to src/setup.py
  • src/setup.py no longer depends on environment variables $SAGE_ROOT, $SAGE_SRC, $SAGE_DOC_SRC etc. (to demonstrate this, Makefile poisons these environment variables). It still depends on $SAGE_LOCAL and environment variables that point below it.

This ticket is meant as:

More specifically, the goal of this ticket is that only SAGE_LOCAL needs to be set when the user does 'pip install' of the sagelib. (This ticket almost achieves this, except it also needs SAGE_PKGS and SAGE_CYTHONIZED to be set. The hope is that #20382 and other future tickets will develop better mechanisms to communicate package and directory information to the build.)

. . . . . . .

Some possibly useful information:

  • Documentation on distutils (https://docs.python.org/2/install/), describing use of --build-base to do VPATH builds.
  • pip install keeps the source directory clean, building instead in a temporary directory, by copying the sources.
    pip install also offers options --build to select a build directory, but there are some pip issues: 2060, 2053, 804 that affect this
  • Transition to build system for sage (the library) #14807 has some tricks to making VPATH builds work without copying all python source files. But it uses automake instead of setup.sh; we will not do this in our ticket.

configure tarball: http://sage.ugent.be/www/jdemeyer/sage/configure-185.tar.gz

CC: @sagetrac-felixs @jdemeyer @kiwifb @embray @nexttime @vbraun @dimpase @jhpalmieri @videlec @saraedum @seblabbe @nthiery @mezzarobba

Component: build

Author: Matthias Koeppe

Branch/Commit: 0c2ac95

Reviewer: Jeroen Demeyer

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

@mkoeppe mkoeppe added this to the sage-7.4 milestone Sep 12, 2016
@kiwifb
Copy link
Member

kiwifb commented Sep 12, 2016

comment:1

Am interesting issue is that technically cython_debug has to be installed somewhere (preferably somewhere standard) to be accessible at runtime separately from the source.

I do something in sage-on-gentoo but that's not really satisfactory.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2016

Replying to @mkoeppe:

In preparation for VPATH builds of sage-the-distribution (#21469), let's keep src/ clean by using setup.py --build-base=$SAGE_ROOT/var/tmp/sage/build/sagelib

Please use --build-base=$SAGE_BUILD_DIR/sagelib.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2016

comment:3

... and sagelib perhaps with a version suffix.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 12, 2016

comment:4

Replying to @nexttime:

Replying to @mkoeppe:

In preparation for VPATH builds of sage-the-distribution (#21469), let's keep src/ clean by using setup.py --build-base=$SAGE_ROOT/var/tmp/sage/build/sagelib

Please use --build-base=$SAGE_BUILD_DIR/sagelib.

Yes, the plan after #21469 is to use the Sage builddir -- not just for sagelib, but also for other packages.

Before #21469 is merged, I want to use $SAGE_ROOT/var/tmp/sage/build/sagelib to match what other packages do.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 12, 2016

comment:5

Ah, I see what you meant, now I've found $SAGE_BUILD_DIR. Changed description accordingly.

@mkoeppe

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2016

comment:6

Replying to @mkoeppe:

Replying to @nexttime:

Replying to @mkoeppe:

In preparation for VPATH builds of sage-the-distribution (#21469), let's keep src/ clean by using setup.py --build-base=$SAGE_ROOT/var/tmp/sage/build/sagelib

Please use --build-base=$SAGE_BUILD_DIR/sagelib.

Yes, the plan after #21469 is to use the Sage builddir -- not just for sagelib, but also for other packages.

??? SAGE_BUILD_DIR exists since years already... (Its default is $SAGE_ROOT/var/tmp/sage/build/.)

Before #21469 is merged, I want to use $SAGE_ROOT/var/tmp/sage/build/sagelib to match what other packages do.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 12, 2016

comment:7

Replying to @nexttime:

??? SAGE_BUILD_DIR exists since years already... (Its default is $SAGE_ROOT/var/tmp/sage/build/.)

Yes, thanks, see my other comment.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2016

comment:8

Ah ok, race condition.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 12, 2016

comment:9

By the way, help with implementing this change would be appreciated. I haven't looked at the sagelib build system at all so far.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 13, 2016

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 13, 2016

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 13, 2016

New commits:

e399bf4First, wishful step

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 13, 2016

Commit: e399bf4

@mkoeppe

This comment has been minimized.

@jdemeyer
Copy link

comment:13

Really sagelib-$SAGE_VERSION? Do you want to fill up everybody's hard disks with Sage build directories? Not to mention that this would require to rebuild everything whenever the Sage version changes.

@jdemeyer

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 13, 2016

comment:15

Fine with me without $SAGE_VERSION; I was just following leif's suggestion in comment 3.

@sagetrac-felixs
Copy link
Mannequin

sagetrac-felixs mannequin commented Sep 13, 2016

comment:16

i understand that SAGE_BUILD_DIR is the srcdir for toplevel configure.

as long as sagelib is rooted in $(toplevel)/src, it might be less confusing to choose $SAGE_BUILD_DIR/src (not $SAGE_BUILD_DIR/sagelib) as the builddir for sagelib.

(future: replace src by sagelib, but on both ends)

i dont know exactly how the approach using setup.py will emulate VPATH builds. i think it should imitate "what automake would do", where applicable.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 13, 2016

comment:17

Replying to @sagetrac-felixs:

i understand that SAGE_BUILD_DIR is the srcdir for toplevel configure.

No, it's $SAGE_ROOT/var/tmp/sage/build/

@sagetrac-felixs
Copy link
Mannequin

sagetrac-felixs mannequin commented Sep 13, 2016

comment:18

ok, nevermind (i meant to write "SAGE_BUILD_DIR is the builddir for toplevel"). that does not seem to be the case either.

still i am wondering why "src" does not (simply) translate to "src". (sure, i am slightly autotools biased).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2016

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

f3f275ePut .cython_version next to cythonized files
94a58f1Make SAGE_BUILD_DIR variable available in sage-env, not just in sage-spkg
a73fa06Use SAGE_BUILD_DIR

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2016

Changed commit from e399bf4 to a73fa06

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 14, 2016

comment:20

Here's a first version for review. It seems to work for me.

There are still some to-do items (see comments in src/Makefile):

  • I am now using --build-base, but setup.sh also depends on SAGE_CYTHONIZED, defined in src/sage/env.py.

I think it would be better if setup.sh instead inferred that location from the build-base that was passed to it.

However, setup.sh already does a lot of stuff depending on SAGE_CYTHONIZED before distutils.core.setup is even called.
Can this be fixed?

I think I could use some help from the Python experts in the cc list of this ticket on this.

  • sage/libs/pari/auto_gen.pxi and sage/ext/interpreters/__init__.py still need to be taken care of in preparation for the VPATH build.

@mkoeppe

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Sep 29, 2016

comment:132

Replying to @embray:

Replying to @jdemeyer:

Replying to @kiwifb:

  • Is there a standard place where cython_debug can be installed? It is needed for debugging and shouldn't live under SAGE_SRC.

On the other hand, debugging also requires the sources to be available. So it is not a crazy idea to keep the debug info close to the sources.

This is why I also like to output the C(++) sources themselves to be close to the cython sources but you said you don't like :)

For the record: in sage-on-gentoo I ship the .pyx files next to the matching .so files. In any case distro ship debugging symbols without the sources. Of course at some point you may need the source code but this is trivial to get and unfold separately in most cases.

@vbraun
Copy link
Member

vbraun commented Oct 3, 2016

comment:133

This ticket introduces an autotools dependency (src/Makefile.in) and fails on OSX

@jdemeyer
Copy link

jdemeyer commented Oct 3, 2016

comment:134

The is the usual breakage of your release management scripts in case of changes to configure.

@jdemeyer
Copy link

jdemeyer commented Oct 3, 2016

Changed commit from fdedb02 to 78ab5d7

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Oct 3, 2016

New commits:

2cf7352Merge tag '7.4.beta6' into t/21480/keep_src__clean_by_using___build_base_when_building_sagelib
78ab5d7Bump configure version

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 8, 2016

comment:137

This now conflicts with the configure spkg version change in 7.4.rc0.
Seems to me incrementing configure spkg versions is something that should be done by the release manager.

@jdemeyer
Copy link

jdemeyer commented Oct 9, 2016

comment:138

Replying to @mkoeppe:

This now conflicts with the configure spkg version change in 7.4.rc0.
Seems to me incrementing configure spkg versions is something that should be done by the release manager.

I know, it is really annoying.

@jdemeyer
Copy link

jdemeyer commented Oct 9, 2016

comment:139

This problem happens every time that you make non-trivial changes to configure.ac

@jdemeyer

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 9, 2016

Changed commit from 78ab5d7 to 0c2ac95

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 9, 2016

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

2871769Merge tag '7.4.rc0' into t/21480/keep_src__clean_by_using___build_base_when_building_sagelib
0c2ac95Bump configure version

@jdemeyer
Copy link

jdemeyer commented Oct 9, 2016

comment:143

This should be merged as soon as possible to avoid further problems with configure.

@vbraun
Copy link
Member

vbraun commented Oct 12, 2016

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

6 participants