sage_setup: Remove import-time dependency (`setup_requires`) on `pkgconfig`, `numpy`
mkoeppe opened this issue · 23 comments
Just loading src/setup.py already pulls in Cython, numpy, and pkgconfig via sage_setup, so these distributions would have to be declared as [build_system] requires in src/pyproject.toml (ex setup_requires)
By moving some computations from import-time to runtime, we get rid of this early dependency on pkgconfig, numpy. (They are, of course, still required for building the package.)
This makes setup.py sdist work using a Python that does not have numpy or pkgconfig installed. To test (with a system python that has Cython):
$ (cd build/pkgs/sagelib/src && python3 -u setup.py --no-user-cfg sdist)
(We also reduce the load-time dependency on Cython; however, we do not address the whole load-time dependency of setup.py on Cython (via sage_setup.find, which uses open_source_file and is_package_dir) in this ticket. This is best done after #28925.)
Depends on #30709
CC: @tobiasdiez @jhpalmieri @kiwifb @dimpase
Component: build
Author: Matthias Koeppe
Branch/Commit: 8f04684
Reviewer: Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/30580
Commit: 88c4e8c
Branch pushed to git repo; I updated commit sha1. New commits:
88c4e8c | src/setup.py: Make 'setup.py sdist' work without Cython |
Description changed:
---
+++
@@ -1,5 +1,8 @@
Just loading `src/setup.py` already pulls in Cython, `numpy`, and `pkgconfig` via `sage_setup`, so these distributions would have to be declared as `[build_system] requires` in `src/pyproject.toml` (ex `setup_requires`)
-By moving some computations from import-time to runtime, we get rid of this dependency on `pkgconfig`, `numpy`.
+By moving some computations from import-time to runtime, we get rid of this early dependency on `pkgconfig`, `numpy`. (They are, of course, still required for building the package.)
+We also make `setup.py sdist` work using a Python that does not have Cython installed.
+(However, we do not address the whole load-time dependency of `setup.py` on `Cython` (via `sage_setup.find`, which uses `open_source_file` and `is_package_dir`) in this ticket. This is best done after #28925.)
+Author: Matthias Koeppe
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4f56517 | Duplicate src/setup.py |
6fe4d56 | Merge branch 't/30779/duplicate_src_setup_py' into t/30580/sage_setup__remove_import_time_dependency___setup_requires___on__pkgconfig____numpy_ |
698a6ea | src/sage_setup/command/sage_build_cython.py: Remove top-level imports from sage.env, Cython |
bb32e80 | build/pkgs/sagelib/src/setup.py: Make 'setup.py sdist' work without Cython |
Ready for review.
Description changed:
---
+++
@@ -2,7 +2,11 @@
By moving some computations from import-time to runtime, we get rid of this early dependency on `pkgconfig`, `numpy`. (They are, of course, still required for building the package.)
-We also make `setup.py sdist` work using a Python that does not have Cython installed.
+This makes `setup.py sdist` work using a Python that does not have `numpy `or `pkgconfig` installed. To test (with a system python that has `Cython`):
-(However, we do not address the whole load-time dependency of `setup.py` on `Cython` (via `sage_setup.find`, which uses `open_source_file` and `is_package_dir`) in this ticket. This is best done after #28925.)
+```
+ $ (cd build/pkgs/sagelib/src && python3 -u setup.py --no-user-cfg sdist)
+```
+(We also reduce the load-time dependency on Cython; however, we do not address the whole load-time dependency of `setup.py` on `Cython` (via `sage_setup.find`, which uses `open_source_file` and `is_package_dir`) in this ticket. This is best done after #28925.)
+the 1st line of the output seems to try to say something about --version I don't get:
$ python3 -u setup.py --no-user-cfg sdist >/tmp/res
/bin/sh: 1: --version: not found
distributions = ['']
warning: sdist: standard file not found: should have one of README, README.txt, README.rst
warning: no files found matching '*.hh' anywhere in distribution
...
Replying to @dimpase:
the 1st line of the output seems to try to say something about
--versionI don't get:$ python3 -u setup.py --no-user-cfg sdist >/tmp/res /bin/sh: 1: --version: not found
This is from src/sage_setup/command/sage_build_cython.py:
# Work around GCC-4.8 bug which miscompiles some sig_on() statements:
# * https://github.com/sagemath/sage-prod/issues/14460
# * https://github.com/sagemath/sage-prod/issues/20226
# * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56982
if subprocess.call("""$CC --version | grep -i 'gcc.* 4[.]8' >/dev/null """, shell=True) == 0:
extra_compile_args.append('-fno-tree-copyrename')
This gives an (ignored) error when CC is not set -- this defect is not new in this ticket.
Do we need to support gcc 4.8 ? The last release of gcc 4.8.* was over 5 years ago.
lgtm - the gcc version could be bumped elsewhere
Reviewer: Dima Pasechnik
Replying to @dimpase:
Do we need to support gcc 4.8 ? The last release of gcc 4.8.* was over 5 years ago.
Yes until we drop ubuntu trusty and similar.
Thanks for the review! I have opened #30876 for the issue with the unset CC environment variable.
Merge conflict
Branch pushed to git repo; I updated commit sha1. New commits:
f53bc50 | Merge commit 'cc96c6dbae448cd361e798a1f29ec5bf10b0c57b' of git://trac.sagemath.org/sage into t/30580/sage_setup__remove_import_time_dependency___setup_requires___on__pkgconfig____numpy_ |
8f04684 | Merge tag '9.3.beta2' into t/30580/sage_setup__remove_import_time_dependency___setup_requires___on__pkgconfig____numpy_ |