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: sage-duplicate/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:

Status badges

Description (last modified by Matthias Köppe)

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:

See also: #29845 - PEP 517 buildapi for sage_setup

Change History (32)

comment:1 Changed 6 years ago by Matthias Köppe

Branch: u/mkoeppe/sagelib_sdist

comment:2 Changed 6 years ago by Matthias Köppe

Cc: Jeroen Demeyer Volker Braun Erik Bray Leif Leonhardy François Bissey added
Commit: c7964a0047837b76fa265d96780e5b3e07c5284a
Dependencies: #21480
Description: modified (diff)

Last 10 new commits:

bd670afIgnore generated files
751bd0fReword TODO item
3a8cc0eFix typo in comment
0dd9c50Respect environment variable MAKE
17f90d8beautification
e5f9065More comments
7791cd9Remove --buildbase code
74169e7Pass SAGE_SRC to generate_py_source.mk
0394333Add new file to MANIFEST.in
c7964a0sdist* Makefile targets

comment:3 Changed 6 years ago by François Bissey

Obviously you are missing an include that would point where to find sage/ext/fast_callable.*. I will have a deeper look.

comment:4 Changed 6 years ago by François Bissey

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 :)

comment:5 in reply to:  4 Changed 6 years ago by Matthias Köppe

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 to SAGE_LIB (usually SAGE_LOCAL/lib/python2.7/site-package).

I don't understand, could you explain more?

comment:6 Changed 6 years ago by François Bissey

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.

comment:7 Changed 6 years ago by git

Commit: c7964a0047837b76fa265d96780e5b3e07c5284a4fe9088cd75ebe54d694c74895697a628fc0d5c0

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

4fe9088New target: sdistcheck-compare-trees

comment:8 Changed 6 years ago by Matthias Köppe

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.

comment:9 Changed 6 years ago by Matthias Köppe

Dependencies: #21480#21480, #21565
Description: modified (diff)

comment:10 Changed 6 years ago by git

Commit: 4fe9088cd75ebe54d694c74895697a628fc0d5c0286adb0efd1d75837b578d9acbf609dbfd165646

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f93a6a2sdist* Makefile targets
286adb0New target: sdistcheck-compare-trees

comment:11 Changed 6 years ago by git

Commit: 286adb0efd1d75837b578d9acbf609dbfd1656466a314d7d113bf5426b7bf7250641c737230c23b8

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

4c6efbdreconfigure if src/Makefile.in changed
be24c34New target 'make sagelib-sdistcheck'
376bf1aRefactor sdistcheck targets and fix dependencies
6a314d7More work on sdistcheck-compare-trees

comment:12 Changed 6 years ago by Matthias Köppe

Milestone: sage-7.4sage-7.5

This ticket needs some help from the distutils experts

comment:13 Changed 6 years ago by Erik Bray

What can I help with?

comment:14 in reply to:  13 Changed 6 years ago by Matthias Köppe

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 Erik Bray

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 Erik Bray

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...)

comment:17 Changed 3 years ago by Matthias Köppe

Cc: Dima Pasechnik added
Milestone: sage-7.5sage-9.1

@embray This is the ticket that I mentioned in #26964.

comment:18 Changed 3 years ago by Matthias Köppe

Cc: Michael Orlitzky added

comment:19 Changed 3 years ago by gh-Shlokatadistance

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

comment:20 Changed 3 years ago by gh-Shlokatadistance

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 Matthias Köppe

Milestone: sage-9.1sage-9.2

comment:22 Changed 3 years ago by Dima Pasechnik

Is this still relevant? Has the world converged onto distributing via pip?

comment:23 Changed 3 years ago by Matthias Köppe

Yes, still need to fix this.

comment:24 Changed 3 years ago by git

Commit: 6a314d7d113bf5426b7bf7250641c737230c23b8086fc8678a6f0074337ae882848b3df1fb19b2d5

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8ce279csdist* Makefile targets
3abffd8New target: sdistcheck-compare-trees
9c7ecbeNew target 'make sagelib-sdistcheck'
4d8b72cRefactor sdistcheck targets and fix dependencies
086fc86More work on sdistcheck-compare-trees

comment:25 Changed 3 years ago by git

Commit: 086fc8678a6f0074337ae882848b3df1fb19b2d551c4da612a99e61ba2975af9e7982087493e12bc

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

51c4da6src/MANIFEST.in: prune .tox

comment:26 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:27 Changed 2 years ago by Matthias Köppe

Dependencies: #21480, #21565#13190
Description: modified (diff)

comment:28 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:29 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-duplicate/invalid/wontfix
Status: newneeds_review

Development continues on #29950 (Build sagelib using the installed sage_setup, add spkg-src). This ticket can be closed.

comment:30 Changed 2 years ago by Matthias Köppe

Cc: Frédéric Chapoton added

comment:31 Changed 2 years ago by Dima Pasechnik

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

comment:32 Changed 2 years ago by Samuel Lelièvre

Authors: Matthias Koeppe
Branch: u/mkoeppe/sagelib_sdist
Commit: 51c4da612a99e61ba2975af9e7982087493e12bc
Resolution: duplicate
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.