Fix sagelib sdist (src/setup.py sdist)
mkoeppe opened this issue · 41 comments
This ticket adds some targets to src/Makefile.in: sdist and sdistcheck.
The latter, after building an sdist (using distutils), unpacks it into a subdirectory, and builds and installs (into SAGE_LOCAL) from there.
(cd src && make sdist) gives the following warnings. They need fixing.
warning: sdist: standard file not found: should have one of README, README.txt
(this one is #21565)
reading manifest template 'MANIFEST.in'
warning: sdist: MANIFEST.in, line 6: 'recursive-include' expects <dir> <pattern1> <pattern2> ...
warning: no previously-included files matching '*.h' found under directory 'sage/ext/interpreters'
warning: no previously-included files found matching 'sage/libs/pari/gen.h'
warning: no previously-included files found matching 'sage/modular/arithgroup/farey_symbol.h'
warning: no previously-included files found matching 'sage/rings/real_mpfi.h'
warning: no previously-included files found matching 'sage/symbolic/pynac.h'
warning: no directories found matching 'doc/common/static'
warning: no files found matching 'doc/en/bordeaux_2008/birch.png'
warning: no files found matching 'doc/en/bordeaux_2008/modpcurve.png'
no previously-included directories found matching 'doc/output'
There is something more seriously wrong with the sdist. The sage that is built from there (using (cd src && make sdistcheck)) crashes as follows.
/Users/mkoeppe/s/sage/sage-rebasing/src/sage/ext/interpreters/wrapper_rdf.pxd in init sage.plot.plot3d.parametric_surface (build/cythonized/sage/plot/plot3d/parametric_surface.c:11409)()
1 # Automatically generated by sage_setup/autogen/interpreters.pyc. Do not edit!
2
3 from cpython cimport PyObject
4
5 from sage.ext.fast_callable cimport Wrapper
6
----> 7 cdef class Wrapper_rdf(Wrapper):
global cdef = undefined
global Wrapper_rdf = undefined
global Wrapper = undefined
8 cdef int _n_args
9 cdef double* _args
10 cdef int _n_constants
11 cdef double* _constants
12 cdef object _list_py_constants
13 cdef int _n_py_constants
14 cdef PyObject** _py_constants
15 cdef int _n_stack
16 cdef double* _stack
17 cdef int _n_code
18 cdef int* _code
19 cdef object _domain
20 cdef bint call_c(self,
21 double* args,
22 double* result) except 0
ImportError: No module named wrapper_rdf
This needs fixing.
The branch is on top of #21480.
References:
Other technologies:
- https://flit.readthedocs.io/en/latest/pyproject_toml.html#sdist-section
- https://python-poetry.org/docs/pyproject/
See also: #29845 - PEP 517 buildapi for sage_setup
Depends on #13190
CC: @jdemeyer @vbraun @embray @nexttime @kiwifb @dimpase @orlitzky @fchapoton
Component: build
Reviewer: Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/21516
Branch: u/mkoeppe/sagelib_sdist
Last 10 new commits:
bd670af | Ignore generated files |
751bd0f | Reword TODO item |
3a8cc0e | Fix typo in comment |
0dd9c50 | Respect environment variable MAKE |
17f90d8 | beautification |
e5f9065 | More comments |
7791cd9 | Remove --buildbase code |
74169e7 | Pass SAGE_SRC to generate_py_source.mk |
0394333 | Add new file to MANIFEST.in |
c7964a0 | sdist* Makefile targets |
Description changed:
---
+++
@@ -55,3 +55,5 @@
This needs fixing.
+The branch is on top of #21480.
+Obviously you are missing an include that would point where to find sage/ext/fast_callable.*. I will have a deeper look.
So this fails after you built and you try to run ./sage? There may be some points in sage that sets where to find the hierarchy of include files under SAGE_SRC when, once you have installed sage properly you should point to SAGE_LIB (usually SAGE_LOCAL/lib/python2.7/site-package).
If that's the case that may recoup with some concerns I expressed in one of the earlier tickets. sage's internal should be fixed before you do something like that to the build system. Off course I could be wrong :)
Replying to @kiwifb:
So this fails after you built and you try to run
./sage?
Yes.
There may be some points in sage that sets where to find the hierarchy of include files under
SAGE_SRCwhen, once you have installed sage properly you should point toSAGE_LIB(usuallySAGE_LOCAL/lib/python2.7/site-package).
I don't understand, could you explain more?
Is SAGE_SRC pointing to anything or is it blank? From the little bit I see, I am guessing some piece of code is autogenerated at runtime to be compiled and executed. And to work, it needs to have an include (-I$SOMEPATH) that points to the top of the sage python install (SAGE_LIB) or as it is usually wrongly done in sage, where the source is in SAGE_SRC. You would find this in sage/misc/cython.py.
Have to prepare dinner for my family. Be back in about 2-3 hours.
Branch pushed to git repo; I updated commit sha1. New commits:
4fe9088 | New target: sdistcheck-compare-trees |
I added another debugging helper make sdistcheck-compare-trees, which shows the differences between the source tree and the sdist tree.
There are various differences (for example, all README files are missing); I won't have time to take a closer look before tomorrow.
Description changed:
---
+++
@@ -5,6 +5,10 @@
```
warning: sdist: standard file not found: should have one of README, README.txt
+```
+(this one is #21565)
+
+```
reading manifest template 'MANIFEST.in'
warning: sdist: MANIFEST.in, line 6: 'recursive-include' expects <dir> <pattern1> <pattern2> ...
This ticket needs some help from the distutils experts
What can I help with?
Replying to @embray:
What can I help with?
I'd need help in figuring out why some files are missing in the sdist and what is the idiomatic way (or the most appropriate way for Sage) of adding them.
I think this calls for a cleaning up of the MANIFEST.in. I'll play around with it.
I think that #21682 might help with this issue as well. As I've repeatedly suggested, it's best to include Cythonized sources in the sdist (taking the onus off the user to create the files with the correct version of Cython--especially important since we currently use a patched Cython for Sage...). This would also ensure that generated modules like the pari interfaces are included (on possible problem with this is if we want to be able to target different versions of pari--that's a problem to be pushed down the line though...)
Can you tell me how did that error go from no previously-included files to. no directories found, the time I recreated this error, this happened after I had quit the command of ./sage
Also I had obtained such an error after a direct installation of SageMath as an application, did you do it the same way? or was it through the tar file and unpacking?
Is this still relevant? Has the world converged onto distributing via pip?
Yes, still need to fix this.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8ce279c | sdist* Makefile targets |
3abffd8 | New target: sdistcheck-compare-trees |
9c7ecbe | New target 'make sagelib-sdistcheck' |
4d8b72c | Refactor sdistcheck targets and fix dependencies |
086fc86 | More work on sdistcheck-compare-trees |
Branch pushed to git repo; I updated commit sha1. New commits:
51c4da6 | src/MANIFEST.in: prune .tox |
Description changed:
---
+++
@@ -61,3 +61,13 @@
The branch is on top of #21480.
+---
+
+References:
+
+- https://flit.readthedocs.io/en/latest/pyproject_toml.html#sdist-section
+
+
+
+
+Description changed:
---
+++
@@ -65,9 +65,10 @@
References:
+Other technologies:
- https://flit.readthedocs.io/en/latest/pyproject_toml.html#sdist-section
+- https://python-poetry.org/docs/pyproject/
-Description changed:
---
+++
@@ -69,6 +69,5 @@
- https://flit.readthedocs.io/en/latest/pyproject_toml.html#sdist-section
- https://python-poetry.org/docs/pyproject/
+See also: #29845 - PEP 517 buildapi for sage_setup
-
-Development continues on #29950 (Build sagelib using the installed sage_setup, add spkg-src). This ticket can be closed.
Reviewer: Dima Pasechnik
Changed branch from u/mkoeppe/sagelib_sdist to none
Changed author from Matthias Koeppe to none