tests: some Windows fixes with newer Python #12088
Conversation
meson tests enable PYTHONWARNDEFAULTENCODING by default and make EncodingWarning fatal too. Starting with Python 3.11 CPython not only warns if no encoding is passed to open() but also to things like subprocess.check_output(). This made the call in vsenv.py fail and in turn made test_vsenv_option fail. check_output() here calls a .bat file which in turn calls vcvars. I don't know what the encoding is supposed to be used there, so just be explicit with the locale encoding to silence the warning.
…ndows Since CPython 3.8 .pyd files no longer look in PATH for loading libraries, but require the DLL directory to be explicitely added via os.add_dll_directory(). This resulted in those tests failing with 3.8+ on Windows. Add the DLL build directory with os.add_dll_directory() to fix them. This was never noticed in CI because it only uses Python 3.7 and the MSYS2 CPython still used the old behaviour until now.
lazka
force-pushed
the
tests-python-add-dll-dir
branch
from
b6de6df to
ac4f257
Compare
lazka
changed the title
lazka
changed the title
| bat_file.close() | ||
| bat_output = subprocess.check_output(bat_file.name, universal_newlines=True) | ||
| bat_output = subprocess.check_output(bat_file.name, universal_newlines=True, | ||
| encoding=locale.getpreferredencoding(False)) |
There was a problem hiding this comment.
Technically the default here is encoding='locale' (identical to encoding=None except with the warning squelched) but that was only added in python 3.10.
The difference is if you set PYTHONUTF8=1 before running meson then locale is still locale but getpreferredencoding is utf-8.
So maybe just check if sys.version_info >= (3, 10) and pass encoding='locale' as an unpacked dict of kwargs?
There was a problem hiding this comment.
According to the docs (https://docs.python.org/3/library/subprocess.html#frequently-used-arguments) and looking at the code (https://github.com/python/cpython/blob/aab6f7173a3b825599629dd6fa5cb7e477421595/Lib/subprocess.py#L375-L378) the default seems to be equivalent to locale.getpreferredencoding(False).
I'm happy to adjust it to ignore utf-8 mode with newer Python if wanted though.
There was a problem hiding this comment.
The default is similar to, not equal to.
There are a couple possibilities here.
encoding=Noneencoding='locale'encoding=getencoding()encoding=getpreferredencoding(False)
The first three are all equivalent, because the first means to literally execute the third inside the internal implementation of subprocess. The second is a symbolic encoding that means to use the same encoding as whatever the third would produce. The third returns the name of a final encoding and doesn't include symbolism.
The fourth is adjacent to the third, and always returns the same value except for when utf-8 mode is enabled. The issue here is "except for", because if there are exceptions then it isn't equivalent, only "mostly equivalent".
As far as fixing it goes, the python ecosystem is in a bind:
- the first triggers an EncodingWarning
- the second and third require a minimum of python 3.10 or 3.11
- the fourth will change its behavior in python 3.15 and can opt in to changing its behavior starting with python 3.7
That being said, we already use the fourth in a couple places in meson, so it may just be fine to defer the issue until python 3.15 and then fix all of those issues at once.
|
Is related to what I tried in #11883. One problem is that locale.getpreferedencoding() does not take into account the console encoding that may be set using |
4 participants
This fixes "test_vsenv_option" with Python 3.11+ and both "4 custom target depends extmodule" on Windows with Python 3.8+. See the commits for details.
(This is required to keep MSYS2 CI green after the next Python update there)