sage.env.sage_include_directories: Don't use distutils and SAGE_LIB
tornaria opened this issue · 25 comments
src/sage/env.pyusesdistutils.sysconfig.get_python_inc()which is deprecated, it has no replacement in sysconfig.
Suggestion from fbissey: "we could use sysconfig.get_config_var('INCLUDEPY') which should give the same value." #33135 comment:7
We also remove the use of SAGE_LIB in preparation of namespace packages.
This is the last use of distutils in the Sage library, except for uses in src/sage/features/__init__.py and src/sage/misc/cython.py, which are merely fallbacks when setuptools is not present.
Comment from arojas: "I don't see any code in sagelib or cython that would emit this. Is this still needed at all?"
Follow-up: Once these uses of distutils are removed, their modules can be removed from the regex in the call to filterwarnings introduced by #33135.
Part of
- Meta-ticket #31295: Replace imports from deprecated
distutils - Meta-ticket #33037: Remove use of
SAGE_LIBandSAGE_EXTCODEvariables
Depends on #32873
Component: misc
Author: Frédéric Chapoton, Matthias Koeppe
Issue created by migration from https://trac.sagemath.org/ticket/33137
get_config_var as the other advantage to be usable from python 3.8 and 3.9. So we could use it now.
fbissey@tarazed ~ $ python3.9
Python 3.9.9 (main, Dec 1 2021, 10:05:37)
[GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import distutils.sysconfig
>>> distutils.sysconfig.get_python_inc()
'/usr/include/python3.9'
>>> import sysconfig
>>> sysconfig.get_config_var('INCLUDEPY')
'/usr/include/python3.9'
>>>
fbissey@tarazed ~ $ python3.8
Python 3.8.12 (default, Nov 4 2021, 22:08:42)
[GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sysconfig
>>> sysconfig.get_config_var('INCLUDEPY')
'/usr/include/python3.8'
>>>
Branch: public/ticket-33137
I'd have done the first part last night [exactly that code] but I couldn't push to trac. Turns out my ssh key is now too old and needs replacing :)
Salut,
pour le reste, on peut aussi virer le bloc, plutot que de mettre RuntimeError
Replying to @fchapoton:
Salut,
pour le reste, on peut aussi virer le bloc, plutot que de mettre RuntimeError
Toi, tu vas encore te faire taper sur les doigts parce que tu parle Français sur un système où l'Anglais est la "lingua Franca" (je sais, c'est bizarre d'écrire ça).
Otherwise, I am not too fussed about removing that block. I cannot see a test showing what it is supposed to catch and how. Antonio says it is a dead code path and there is clearly no proof otherwise anywhere.
Description changed:
---
+++
@@ -9,3 +9,6 @@
Comment from arojas: "I don't see any code in sagelib or cython that would emit this. Is this still needed at all?"
Once these uses of distutils are removed, their modules can be removed from the regex in the call to filterwarnings introduced by #33135.
+
+
+Part of Meta-ticket #31295: Replace imports from deprecated distutilsBranch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
58b0948 | less distutils in env.py |
Replying to @fchapoton:
apparemment le changement dans features est en conflit avec #32873
Yes and I positively reviewed that ticket. I thought we were dealing with something different. In another ticket of mine about doctest failures on distro, Matthias pointed me out to three tickets - one of which I reviewed, again. It seems to me there are far too many tickets of that kind not included in 9.5.rc0. It is hard to figure out what is already on trac.
From tornaria #33473 comment:22:
indeed it seems setuptools adds the python include location to the compiler command line, in fact with the following comment in setuptools/_distutils/command/build_ext.py:
# Put the Python "system" include dir at the end, so that
# any local include dirs take precedence.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
824e9fa | sage.env.sage_include_directories: Do not append distutils.sysconfig.get_python_inc() |
Description changed:
---
+++
@@ -4,11 +4,13 @@
Suggestion from fbissey: "we could use sysconfig.get_config_var('INCLUDEPY') which should give the same value." [#33135 comment:7](https://github.com/sagemath/sage/issues/33135#comment:7)
-- `src/sage/features/__init__.py` uses `distutils.errors.CCompilerError` which is not present in `setuptools.errors`.
+This is the last use of `distutils` in the Sage library, except for uses in `src/sage/features/__init__.py` and `src/sage/misc/cython.py`, which are merely fallbacks when `setuptools` is not present.
Comment from arojas: "I don't see any code in sagelib or cython that would emit this. Is this still needed at all?"
-Once these uses of distutils are removed, their modules can be removed from the regex in the call to filterwarnings introduced by #33135.
+Follow-up: Once these uses of distutils are removed, their modules can be removed from the regex in the call to filterwarnings introduced by #33135.
+Part of
+- Meta-ticket #31295: Replace imports from deprecated `distutils`
+- Meta-ticket #33037: Remove use of `SAGE_LIB` and `SAGE_EXTCODE` variables
-Part of Meta-ticket #31295: Replace imports from deprecated distutilsBranch pushed to git repo; I updated commit sha1. New commits:
fb965ec | sage.env.sage_include_directories: Append sysconfig variable INCLUDEPY |
Author: Frédéric Chapoton, Matthias Koeppe
Replying to @mkoeppe:
From
tornaria#33473 comment:22:
indeed it seems setuptools adds the python include location to the compiler command line, in fact with the following comment in setuptools/_distutils/command/build_ext.py:# Put the Python "system" include dir at the end, so that # any local include dirs take precedence.
Nevertheless, I have put sysconfig.get_config_var('INCLUDEPY') back in: sage.misc.cython.cython appends some directories to the result of sage_include_directories(), so let's not provoke some subtle changes in the order of looking up libraries when users use the runtime Cython.
Description changed:
---
+++
@@ -3,6 +3,8 @@
- `src/sage/env.py` uses `distutils.sysconfig.get_python_inc()` which is deprecated, it has no replacement in sysconfig.
Suggestion from fbissey: "we could use sysconfig.get_config_var('INCLUDEPY') which should give the same value." [#33135 comment:7](https://github.com/sagemath/sage/issues/33135#comment:7)
+
+We also remove the use of `SAGE_LIB` in preparation of namespace packages.
This is the last use of `distutils` in the Sage library, except for uses in `src/sage/features/__init__.py` and `src/sage/misc/cython.py`, which are merely fallbacks when `setuptools` is not present.