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
Python modules check: Assert exit code, not empty output #329
Conversation
Distutils is deprecated in Python 3.10+: https://www.python.org/dev/peps/pep-0632/ When importing it, there is a warning: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives The configure script falsely assumed this is an error. Instead, we now check for the exit code, which is more explicit. In the long term, we need to migrate away from distutils, this is a stopgap measure only.
I've edited this trough GitHub's web UI, somebody please regenerate configure with this change when merging. Thanks |
I've edited this trough GitHub's web UI,
So what you're saying is that the change itself hasn't been tested? I'm not a fan of that!
somebody please regenerate configure with this change when merging. Thanks
This isn't an issue as configure is derrived (by running autogen).
I'd much prefer migrating to an actual solution rather than to a stopgap measure. The current situation is, after all, working well enough.
They wouldn't have deprecated something for which there isn't already a preferred replacement. Do you know what it is?
|
Sorry about that. It hasn't. I've assumed a CI will test it.
Oh, they did :D But nevertheless, for this use case, you should use sysconfig (not distutils.sysconfig). All the information this script queries is there, but the API is different. https://docs.python.org/3/library/sysconfig.html#installation-paths |
Sorry about that. It hasn't. I've assumed a CI will test it.
Of course it'll get tested, but that isn't the point. Designers should test everything they submit - not just to brltty. By the time it reaches someone else for testing, there shouldn't be a problem. Of course there might be, but that's a different issue. Imperfection is expected, but carelessness shoudn't be. Just me giving a lecture.
> They wouldn't have deprecated something for which there isn't already a preferred replacement. Do you know what it is?
Oh, they did :D
Really? Maybe they think that pkg-config should now be used instead. That's imprecise, though, as it's not reasonable to expect ones use of pkg-config to match the interpreter that's been autodetected.
But nevertheless, for this use case, you should use sysconfig (not distutils.sysconfig). All the information this script queries is there, but the API is different.
Okay, maybe that's the preferred replacement. Fair enough. When was sysconfig introduced?
|
2.7: https://docs.python.org/3.10/whatsnew/2.7.html#new-module-sysconfig |
Thanks. To save me some time, do you know the new function calls for get_python_inc(), get_python_lib(), and get_config_var()?
I think I'm going to move this code out of bindings.m4 into an external script (as has already been done for Tcl and Java). If you're curious, see Tools/*cmd (i.e. javacmd and tclcmd). I've already started it, like this:
try:
import sysconfig
except ModuleNotFoundError:
import distutils.sysconfig
|
This should work: sysconfig.get_path('include') # instead of distutils.sysconfig.get_python_inc()
sysconfig.get_path('stdlib') # instead of distutils.sysconfig.get_python_lib(0,1)
sysconfig.get_path('platlib') # instead of distutils.sysconfig.get_python_lib(1,0)
sysconfig.get_config_var('LIBS') # instead of distutils.sysconfig.get_config_var('LIBS') |
Thanks. "purelib" is needed (rather than "stdlib"). Hav1e a look at Tools/pythoncmd on the (new) python-sysconfig branch.
|
purelib is distutils.sysconfig.get_python_lib(0,0) |
purelib is distutils.sysconfig.get_python_lib(0,0)
stdlib is distutils.sysconfig.get_python_lib(0,1)
Yes, I understand that. Maybe that's been a long-standing bug. The idea is where should the BrlAPI Python bindings be installed. My understanding (though I'm not a Python expert) is that it should be in site-packages.
|
yes, in site-packages, i.e. into platlib. |
I've sent the PR to make it build. I am not available to invest any more time than that, sorry about that. |
No problem. Thanks for drawing this issue to my attention. As you've seen, I've started working on what I think is the proper solution.
|
Fixed. Again, thanks for bringing this issue to our attention. I'll be closing this PR.
|
Distutils is deprecated in Python 3.10+:
https://www.python.org/dev/peps/pep-0632/
When importing it, there is a warning:
The configure script falsely assumed this is an error.
Instead, we now check for the exit code, which is more explicit.
In the long term, we need to migrate away from distutils, this is a stopgap measure only.