Opened 6 years ago
Closed 2 years ago
#21516 closed defect (duplicate)
Fix sagelib sdist (src/setup.py sdist)
Reported by:  Matthias Köppe  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  build  Keywords:  
Cc:  Jeroen Demeyer, Volker Braun, Erik Bray, Leif Leonhardy, François Bissey, Dima Pasechnik, Michael Orlitzky, Frédéric Chapoton  Merged in:  
Authors:  Reviewers:  Dima Pasechnik  
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  #13190  Stopgaps: 
Description (last modified by )
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: 'recursiveinclude' expects <dir> <pattern1> <pattern2> ... warning: no previouslyincluded files matching '*.h' found under directory 'sage/ext/interpreters' warning: no previouslyincluded files found matching 'sage/libs/pari/gen.h' warning: no previouslyincluded files found matching 'sage/modular/arithgroup/farey_symbol.h' warning: no previouslyincluded files found matching 'sage/rings/real_mpfi.h' warning: no previouslyincluded 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 previouslyincluded 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/sagerebasing/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#sdistsection
 https://pythonpoetry.org/docs/pyproject/
See also: #29845  PEP 517 buildapi for sage_setup
Change History (32)
comment:1 Changed 6 years ago by
Branch:  → u/mkoeppe/sagelib_sdist 

comment:2 Changed 6 years ago by
Cc:  Jeroen Demeyer Volker Braun Erik Bray Leif Leonhardy François Bissey added 

Commit:  → c7964a0047837b76fa265d96780e5b3e07c5284a 
Dependencies:  → #21480 
Description:  modified (diff) 
comment:3 Changed 6 years ago by
Obviously you are missing an include that would point where to find sage/ext/fast_callable.*
. I will have a deeper look.
comment:4 followup: 5 Changed 6 years ago by
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/sitepackage
).
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 :)
comment:5 Changed 6 years ago by
Replying to fbissey:
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_SRC
when, once you have installed sage properly you should point toSAGE_LIB
(usuallySAGE_LOCAL/lib/python2.7/sitepackage
).
I don't understand, could you explain more?
comment:6 Changed 6 years ago by
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 23 hours.
comment:7 Changed 6 years ago by
Commit:  c7964a0047837b76fa265d96780e5b3e07c5284a → 4fe9088cd75ebe54d694c74895697a628fc0d5c0 

Branch pushed to git repo; I updated commit sha1. New commits:
4fe9088  New target: sdistcheckcomparetrees

comment:8 Changed 6 years ago by
I added another debugging helper make sdistcheckcomparetrees
, 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.
comment:9 Changed 6 years ago by
Dependencies:  #21480 → #21480, #21565 

Description:  modified (diff) 
comment:10 Changed 6 years ago by
Commit:  4fe9088cd75ebe54d694c74895697a628fc0d5c0 → 286adb0efd1d75837b578d9acbf609dbfd165646 

comment:11 Changed 6 years ago by
Commit:  286adb0efd1d75837b578d9acbf609dbfd165646 → 6a314d7d113bf5426b7bf7250641c737230c23b8 

comment:12 Changed 6 years ago by
Milestone:  sage7.4 → sage7.5 

This ticket needs some help from the distutils experts
comment:14 Changed 6 years ago by
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.
comment:15 Changed 6 years ago by
I think this calls for a cleaning up of the MANIFEST.in. I'll play around with it.
comment:16 Changed 6 years ago by
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 Cythonespecially 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 parithat's a problem to be pushed down the line though...)
comment:17 Changed 3 years ago by
Cc:  Dima Pasechnik added 

Milestone:  sage7.5 → sage9.1 
@embray This is the ticket that I mentioned in #26964.
comment:18 Changed 3 years ago by
Cc:  Michael Orlitzky added 

comment:19 Changed 3 years ago by
Can you tell me how did that error go from no previouslyincluded files
to. no directories found
, the time I recreated this error, this happened after I had quit the command of ./sage
comment:20 Changed 3 years ago by
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?
comment:21 Changed 3 years ago by
Milestone:  sage9.1 → sage9.2 

comment:22 Changed 3 years ago by
Is this still relevant? Has the world converged onto distributing via pip?
comment:24 Changed 3 years ago by
Commit:  6a314d7d113bf5426b7bf7250641c737230c23b8 → 086fc8678a6f0074337ae882848b3df1fb19b2d5 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8ce279c  sdist* Makefile targets

3abffd8  New target: sdistcheckcomparetrees

9c7ecbe  New target 'make sagelibsdistcheck'

4d8b72c  Refactor sdistcheck targets and fix dependencies

086fc86  More work on sdistcheckcomparetrees

comment:25 Changed 3 years ago by
Commit:  086fc8678a6f0074337ae882848b3df1fb19b2d5 → 51c4da612a99e61ba2975af9e7982087493e12bc 

Branch pushed to git repo; I updated commit sha1. New commits:
51c4da6  src/MANIFEST.in: prune .tox

comment:26 Changed 2 years ago by
Description:  modified (diff) 

comment:27 Changed 2 years ago by
Dependencies:  #21480, #21565 → #13190 

Description:  modified (diff) 
comment:28 Changed 2 years ago by
Description:  modified (diff) 

comment:29 Changed 2 years ago by
Milestone:  sage9.2 → sageduplicate/invalid/wontfix 

Status:  new → needs_review 
Development continues on #29950 (Build sagelib using the installed sage_setup
, add spkgsrc
). This ticket can be closed.
comment:30 Changed 2 years ago by
Cc:  Frédéric Chapoton added 

comment:31 Changed 2 years ago by
Reviewers:  → Dima Pasechnik 

Status:  needs_review → positive_review 
comment:32 Changed 2 years ago by
Authors:  Matthias Koeppe 

Branch:  u/mkoeppe/sagelib_sdist 
Commit:  51c4da612a99e61ba2975af9e7982087493e12bc 
Resolution:  → duplicate 
Status:  positive_review → closed 
Last 10 new commits:
Ignore generated files
Reword TODO item
Fix typo in comment
Respect environment variable MAKE
beautification
More comments
Remove buildbase code
Pass SAGE_SRC to generate_py_source.mk
Add new file to MANIFEST.in
sdist* Makefile targets