Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14570 closed enhancement (fixed)

Cythonize Sage library out of tree

Reported by: ohanar Owned by: GeorgSWeber
Priority: major Milestone: sage-5.10
Component: build Keywords:
Cc: leif, robertwb, jdemeyer Merged in: sage-5.10.beta4
Authors: R. Andrew Ohana Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14569 Stopgaps:

Description (last modified by jdemeyer)

Nice for at least two reasons:

  1. Can grep through source code and not pick up any matches from autogenerated c files
  2. (mostly for git) Don't have to have all c/cpp files ignored by revision control

Apply:

Attachments (2)

cython-0.19.1.p0.diff (2.2 KB) - added by ohanar 7 years ago.
spkg diff
trac14570.patch (3.8 KB) - added by ohanar 7 years ago.
apply to sage library

Download all attachments as: .zip

Change History (20)

comment:1 Changed 7 years ago by ohanar

  • Summary changed from cythonize out of tree to Cythonize Sage library out of tree

comment:2 Changed 7 years ago by ohanar

  • Cc leif robertwb jdemeyer added
  • Description modified (diff)
  • Status changed from new to needs_review

Changed 7 years ago by ohanar

spkg diff

comment:3 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 7 years ago by jdemeyer

Doesn't

cdef extern from *:

work instead of "empty.h"?

Changed 7 years ago by ohanar

apply to sage library

comment:5 Changed 7 years ago by ohanar

yup, new patch does that instead

comment:6 Changed 7 years ago by jdemeyer

Why are some c_lib header files copied to build/cythonized/c_lib, is that supposed to be? I'm not saying it is a bad thing, I am just trying to understand what is happening.

buildbot@sage sage$ ls -l devel/sage/build/cythonized/c_lib/include/
total 74k
4.1k -rw-r--r-- 2 buildbot buildbot  254 2013-05-16 23:52 convert.h
4.1k -rw-r--r-- 2 buildbot buildbot 3.8k 2013-05-16 23:52 ginac_wrap.h
4.1k -rw-r--r-- 2 buildbot buildbot  545 2013-05-16 23:52 gmp_globals.h
 17k -rw-r--r-- 2 buildbot buildbot  15k 2013-05-16 23:52 interrupt.h
4.1k -rw-r--r-- 2 buildbot buildbot  128 2013-05-16 23:52 mpz_longlong.h
4.1k -rw-r--r-- 2 buildbot buildbot  380 2013-05-16 23:52 mpz_pylong.h
 17k -rw-r--r-- 2 buildbot buildbot  13k 2013-05-16 23:52 ntl_wrap.h
8.2k -rw-r--r-- 2 buildbot buildbot 6.1k 2013-05-16 23:52 pb_wrap.h
8.2k -rw-r--r-- 2 buildbot buildbot 5.2k 2013-05-16 23:52 stdsage.h
4.1k -rw-r--r-- 2 buildbot buildbot 1.2k 2013-05-16 23:52 ZZ_pylong.h

comment:7 Changed 7 years ago by ohanar

Every file that is in SAGE_SRC that is detected as a dependency or source file for an extension module will be copied to SAGE_SRC/build/cythonized (this is to make sure that including with relative paths still works). Since SAGE_LOCAL/include/csage is symlinked to a subdirectory of SAGE_SRC, these particular files appear. They won't actually be used during compilation since these files were never included using a relative path (at least as far as I'm aware).

Last edited 7 years ago by ohanar (previous) (diff)

comment:8 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.10.beta4
  • Resolution set to fixed
  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to closed

comment:9 Changed 7 years ago by leif

Oh, boys...

This completely breaks sage -sync-build! >8O

comment:10 Changed 7 years ago by leif

We should probably wait with this feature until Cython supports a true VPATH, or at least symlinks the source files instead of copying all of them.

P.S.: GNU grep supports inclusion / exclusion of files / dirs / filename patterns, to be specified on the command line or taken from a file (similar to tar). And (probably dynamically) generating .gitignore files shouldn't be too hard either.

comment:11 Changed 7 years ago by jdemeyer

Reverting this patch in #14721.

comment:12 follow-up: Changed 7 years ago by felixs

gnu autotools has VPATH support. it should be implemented with the usual interface, not with ad-hoc hacks.

once sage (the distribution) is configurable (im working on it...) it will be extemely convenient to be able to have differently configured sage builds without complicating/breaking the internal sage build logic even more.

if the cythonize doesnt support VPATH cleanly (yet?) use autotools for the build. it's much simpler, straightforward, faster and well designed. no need to reinvent the wheel.

comment:13 in reply to: ↑ 12 Changed 7 years ago by jdemeyer

Replying to felixs:

gnu autotools has VPATH support. it should be implemented with the usual interface, not with ad-hoc hacks.

once sage (the distribution) is configurable (im working on it...) it will be extemely convenient to be able to have differently configured sage builds without complicating/breaking the internal sage build logic even more.

if the cythonize doesnt support VPATH cleanly (yet?) use autotools for the build. it's much simpler, straightforward, faster and well designed. no need to reinvent the wheel.

This kind of feedback should really be directed to Cython upstream. If somebody is reinventing the wheel here, it's Cython, not Sage.

comment:14 follow-up: Changed 7 years ago by jdemeyer

Also note that Python doesn't use automake either. So what you ask might not easily be doable.

comment:15 in reply to: ↑ 14 ; follow-ups: Changed 7 years ago by felixs

Replying to jdemeyer:

This kind of feedback should really be directed to Cython upstream. If somebody is reinventing the wheel here, it's Cython, not Sage.

The build system choice is clearly on the sage side. Just because the "build system" that cython provides lacks functionality, doesn't mean that I expect them to fix it.

Replying to jdemeyer:

Also note that Python doesn't use automake either.

What are you trying to say (I dont want python to use automake.)?

Do you agree that

  • Sage is built mostly by cython and gcc,
  • gcc does not use distutils and
  • Sage is not python?

So what you ask might not easily be doable.

I am not asking for anything. Its just a proposal on how to fix it.

Most things are implemented and ready-to-use within autotools, much more than distutils can ever dream of. Currently, just cython build rules need to be added manually (and are supported like any other custom build rules). native support for pyx->c->so within autotools is just a question of time.

Note that one alternative would be autogenerating setup.py and module_list.py from templates with autoconf (yes, autoconf implements the checks we need). I doubt that this can either be as functional nor maintainable as a clean-room autotools-based implementation...

And yes, it's doable with autotools. Any better ideas?

comment:16 in reply to: ↑ 15 Changed 7 years ago by jdemeyer

Replying to felixs:

The build system choice is clearly on the sage side. Just because the "build system" that cython provides lacks functionality, doesn't mean that I expect them to fix it.

Sage uses Cython's cythonize() as build system. There is Cython-the-compiler and cythonize+distutils-the-build-system.

Do you agree that

  • Sage is built mostly by cython and gcc,

For different meanings of "built by": yes.

comment:17 in reply to: ↑ 15 ; follow-up: Changed 7 years ago by jdemeyer

Replying to felixs:

Most things are implemented and ready-to-use within autotools

Really, does autotools support Cython dependency checking?

comment:18 in reply to: ↑ 17 Changed 7 years ago by felixs

Replying to jdemeyer:

Really, does autotools support Cython dependency checking?

Thats a question of wheter cython supports writing out dependencies. See #14728 for a patch.

Note: See TracTickets for help on using tickets.