Refactoring run_autogen [to generate wrappers] to pkgs/sagemath-standard/setup.py

[kiwifb]

Recently some changes in the build tools for PEP517 python packages in Gentoo lead to python setup.py build not being executed in favor of python setup.py build_ext see cschwan/sage-on-gentoo#693 and https://bugs.gentoo.org/842534

The Gentoo maintainer reasoning for removing build is that it is normally a meta target whose sole purpose is to start other phases like build_ext. The over-ridding of the build command in sage is an increasingly uncommon practice apparently.

The build target does only one thing, run run_autogen to generate some cython code and build is explicitly only called by the sage build system when it is configured without --enable-editablehttps://github.com/sagemath/sage-prod/blob/develop/build/pkgs/sagelib/spkg-install#L55. In builds with that option enabled, autogeneration is called from setup.py https://github.com/sagemath/sage-prod/blob/develop/src/setup.py#L78 and build is not called explicitly.

We take the opportunity to simplify the build system further and reduce the differences editable and non-editable build by either fully moving the autogeneration in setup.py.

(An alternative would have been to move it to build_ext the strategy I followed as a quick fix in https://github.com/cschwan/sage-on-gentoo/blob/master/sci-mathematics/sage_setup/files/sage_setup-9.6-no_build.patch)

CC: @mkoeppe

Component: build

Author: François Bissey

Branch/Commit: 278cc0d

Reviewer: Matthias Koeppe

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

[mkoeppe]

comment:1

I'm -1 on considering anything other than setup.py egg_info, setup.py sdist and setup.py bdist_wheel supported interface. Our package declares the build system via PEP 517 and distributions have no business in poking around in its internals.

[mkoeppe]

comment:2

build_ext is clearly the wrong place for it. However, it would be OK for me to get rid of sage_build and just do it in setup.py, as you suggest.

[kiwifb]

comment:3

I was replying a bit more tongue in check to your first post when the second one came :)

I agree about going with the setup.py solution and thought of my problem as an opportunity to simplify things and minimise difference between editable/non-editable. There are obviously issues there and autogen is one of them.

[mkoeppe]

comment:4

For more context, it is probably safe to say that Gorny's homegrown anti-PEP-517 scripts are an obviously misguided approach, but that's not my problem ;)

[kiwifb]

comment:5

But I take it from your first comment that you will not have any objections to work on replacing

time python3 -u setup.py --no-user-cfg build install || exit 1

in build/pkgs/sagelib/spkg-install by something more pip-ish. The fact that you need to explicitly call build here, suggests to me that pip doesn't call it either.

[mkoeppe]

comment:6

I think the build in that line is just a leftover from when we passed some arguments.

[mkoeppe]

comment:7

See #32874 for getting rid of our use of setup.py install

[kiwifb]

comment:8

Right, so I seem to have it all wrong. It's just that bizzare duplication of the autogen code between the two cases suggest otherwise. In that case the code in the current setup.py for editable sage may be redundant. Yet, that's the one I will want to keep long term.

[mkoeppe]

comment:9

Yes, it will be a nice simplification to get rid of the sage_build class.

I don't think we have a need for the autogen stuff to be called in several distribution packages, so nothing is gained from having it in a reusable class.

[kiwifb]

Branch: u/fbissey/wrappers_generations_build_simplification

[kiwifb]

Commit: 278cc0d

[kiwifb]

comment:11

First draft.

---

New commits:

278cc0d

Remove sage_build which is only used to generate wrappers and move the generation to setup.py in all cases.

[mkoeppe]

Reviewer: Matthias Koeppe

[mkoeppe]

Author: François Bissey

[mkoeppe]

comment:12

Looking good

[vbraun]

Changed branch from u/fbissey/wrappers_generations_build_simplification to 278cc0d