Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor setup.py a bit #106

Merged
merged 2 commits into from
Nov 13, 2017
Merged

Refactor setup.py a bit #106

merged 2 commits into from
Nov 13, 2017

Conversation

embray
Copy link
Contributor

@embray embray commented Nov 13, 2017

This moves several of the operations that were performed at module-level in the setup.py into a subclass of the distutils build_ext command. This way, operations that are only relevant to building extension modules are only performed when actually building extension modules, rather than unconditionally.

This allows certain operations like ./setup.py --name and ./setup.py --version to work without unnecessary side-effects.

One possible small regression this introduces is that if you want the Cython-generated C sources to be included in a source distribution it's now necessary to run ./setup.py build_ext sdist, rather than ./setup.py sdist, though we could also make it so that build_ext is run automatically as a prerequisite of sdist. That said, I'm not sure if this is really desirable in this case, since the Cython compilation depends on some DEF statements, which in turn depend on external values.

@embray
Copy link
Contributor Author

embray commented Nov 13, 2017

I should add, this was motivated originally by the desire to add more build configurations to the Travis-CI builds. I wanted to be able to use environment variables to force values of the DEFs, like HAVE_QD (a feature which this PR adds). But in the process I decided to move more stuff around.

@embray
Copy link
Contributor Author

embray commented Nov 13, 2017

Something not quite right here with handling of include dirs; worked for me locally for some reason.

the build_ext command.

In particular compile-time flags are not checked, and Cython is not run
unless we are running the build_ext command.
@malb
Copy link
Collaborator

malb commented Nov 13, 2017

LGTM.

@malb malb merged commit 691452e into fplll:master Nov 13, 2017
@embray
Copy link
Contributor Author

embray commented Nov 17, 2017

Great, thanks.

@embray embray deleted the setup-updates branch November 17, 2017 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants