Don't use make for autogenerated modules in sagelib

[embray]

Currently there is a src/Makefile at the root of the sagelib sources that is required to be invoked in order to make sure the packages in sage_setup.autogen and that autogenerated sources are placed in the sage sources before running its setup.py install.

#21480 inverts things somewhat by removing the old Makefile (really making it into a no-op for now), but pulling out the sage_setup.autogen stuff into a separate makefile that is still invoked by calling make from the setup.py (in the current version of that ticket it is even called unconditionally). This is pretty non-standard.

The appropriate time to do this would be either just before the build_py command is run, or the build_ext command is run (in the case of autogenerated sources for extension modules).

In the current cases we have its very trivial to check whether or not any of the sources actually need to be regenerated, and this is a pretty normal thing to do in some other complex Python packages. For example, the package responsible for generating the source files (e.g. currently the sage_setup.autogen sub-packages) simply needs to be responsible for knowing what its own source code files are, and knowing the paths for its generated sources--then just compare modtimes. Invoking make is not required at all.

One can do even better by having one of the generated files be a hash of the source files, and compare the hashes instead (this avoids rebuilds when switching between git branches but where the files didn't otherwise change).

CC: @mkoeppe @jdemeyer

Component: build

Author: Erik Bray

Branch: 34ca46c

Reviewer: Matthias Koeppe

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

[mkoeppe]

comment:2

From the description:

The appropriate time to do this would be either just before the build_py command is run, or the build_ext command is run (in the case of autogenerated sources for extension modules).

Could you make a ticket on top of #21480 (and probably #21604) that moves the invocation of make -f generate_py_source.mk to the right place?

[jdemeyer]

comment:3

Replying to @mkoeppe:

Could you make a ticket on top of #21480 (and probably #21604) that moves the invocation of make -f generate_py_source.mk to the right place?

That's not so easy, for the same reason that #21600 is not so easy. We need to pass the list of Python packages, Cython extensions and data_files to setup(). It is harder to get this right if not everything has been generated at that point. We should probably compute the list of packages dynamically in setup().

[jdemeyer]

comment:4

Again, the title of this ticket does not match the long description. Do you want to get rid of make or just move the invocation of make -f generate_py_source.mk inside setup()?

[embray]

comment:5

I think it does match but I'm not sure what's unclear.

[embray]

comment:6

I feel like this is not hard, having done it many times before. There's nothing particularly special about sage either in any of these regards.

[embray]

Changed dependencies from #21480 to #21600

[embray]

Author: Erik Bray

[embray]

Commit: edb3cf8

[embray]

comment:9

Determine directly in the Python code whether or not autogenerated sources need to be built, without requiring a makefile or anything. I think this is simple and clear to anyone who looks at the code.

If in the future we want more full make-like capabilities I've got something in the works for that, but it seems unlikely that anything more complicated than this is needed any time soon.

---

Last 10 new commits:

7791cd9	Remove --buildbase code
74169e7	Pass SAGE_SRC to generate_py_source.mk
0394333	Add new file to MANIFEST.in
fdedb02	Install SAGE_LOCAL/bin/sage instead of SAGE_ROOT/sage as Jupyter kernel
5ba95ed	Clean up stale installed files in install command
35ecf4b	Run cythonize() inside build_ext command
2b5fa98	Copy extra files to build directory instead of using data_files
7e3a492	Rename extra_files -> sage_extra_files
237e965	Replace some implicit relative imports with explicit imports.
edb3cf8	Do effectively exactly what the makefile for the autogen sources was doing, but purely in Python, removing the use of make.

[embray]

Branch: u/embray/ticket-21613

[mkoeppe]

comment:10

This looks nice and easy to understand.
But let's get #21600 done before we full review this one.

[mkoeppe]

comment:11

On this ticket, 0394333 needs to be reverted, generate_py_source removed again from MANIFEST.in

(Of course, the manifest is out-of-date in many respects (#21516).)

[embray]

comment:12

Indeed. I just noticed that MANIFEST.in contains MANIFEST.in, which it really doesn't need to do :)

[sagetrac-git]

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

9f92cb3

Remove generate_py_source.mk from MANIFEST.in as it no longer exists

[sagetrac-git]

Changed commit from edb3cf8 to 9f92cb3

[mkoeppe]

comment:15

This needs rebasing on top of current develop and possibly some work there.
Building from scratch (with 7.4.rc2 merged) fails for me with:

No such file or directory: '/doesnotexist/sage/libs/pari/paridecl.pxd'

(this is from #21480).

[embray]

comment:53

I don't really think that belongs here in the first place but okay.

[sagetrac-git]

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

e45216f

Clean up old autogenerated files before outputting new ones.

[sagetrac-git]

Changed commit from 9750ba7 to e45216f

[embray]

New commits:

e45216f

Clean up old autogenerated files before outputting new ones.

[jdemeyer]

comment:56

In #21820, some paths changed. In particular, the generated files are now in src/sage/libs/cypari2.

[jdemeyer]

Changed dependencies from #21820 to none

[embray]

comment:57

Yes, I see that now. Incidentally building still worked, it just built the files in the wrong place.

[sagetrac-git]

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

91fb12e

Use sage_src_pari to get the output path for generated files.

[sagetrac-git]

Changed commit from e45216f to 91fb12e

[sagetrac-git]

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

34ca46c

Don't look for the decl.pxi file anymore

[sagetrac-git]

Changed commit from 91fb12e to 34ca46c

[embray]

New commits:

91fb12e	Use sage_src_pari to get the output path for generated files.
34ca46c	Don't look for the decl.pxi file anymore

[mkoeppe]

comment:61

Looks good to me; though the description of this ticket could use updating.

[vbraun]

Changed branch from u/embray/ticket-21613 to 34ca46c

[fchapoton]

comment:63

Some changes made here are not py3 compatible, and need to be corrected to be so.

try:

export SAGE_PYTHON3=yes 
make build

[fchapoton]

Changed commit from 34ca46c to none

[fchapoton]

comment:64

proposal fix in #22044

[jdemeyer]

comment:65

Breakage: #22094.