classification
Title: PyRun_String not exported in python38.dll
Type: Stage: resolved
Components: Windows Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, cgohlke, eryksun, miss-islington, paul.moore, pyscripter, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2019-06-07 07:55 by cgohlke, last changed 2019-06-18 10:06 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 14142 merged vstinner, 2019-06-17 07:42
PR 14177 merged miss-islington, 2019-06-17 22:15
Messages (18)
msg344905 - (view) Author: Christoph Gohlke (cgohlke) Date: 2019-06-07 07:55
While testing third party packages on Python 3.8.0b1 for Windows, I noticed that the `PyRun_String` function is no longer exported from `python38.dll`.

Is this intentional? I can't see this mentioned at <https://docs.python.org/3.8/whatsnew/3.8.html> or <https://docs.python.org/3.8/c-api/veryhigh.html#c.PyRun_String>

This change breaks existing code. But then `PyRun_String` is easy to replace with `PyRun_StringFlags`.
msg344982 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-06-07 17:59
Guessing Victor may have touched something in this area.

Was it exported before? Or did we just have a macro for it? Maybe the macro was moved to an internal header by mistake?
msg344989 - (view) Author: Christoph Gohlke (cgohlke) Date: 2019-06-07 18:13
`PyRun_String` was exported at least since `python23.dll`.

Python.Net relies on it at <https://github.com/pythonnet/pythonnet/blob/master/src/runtime/runtime.cs#L858>
msg344993 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2019-06-07 18:20
A look through `git log -p` looks like bpo-34646 is likely to be related.
msg345056 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2019-06-08 17:21
It shouldn't break existing code because PyRun_String has a macro expansion to PyRun_StringFlags. ABI compatibility between major releases is not provded.
msg345792 - (view) Author: PyScripter (pyscripter) Date: 2019-06-17 00:01
This does break PyScripter Python for Delphi as well.  The question whether this change was intentional in which case it would need to be explained and documented, or accidental and will be reversed begs an answer.
msg345809 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-17 07:48
Attached PR 14142 fix bpo-34646 regression.

> It shouldn't break existing code because PyRun_String has a macro expansion to PyRun_StringFlags. ABI compatibility between major releases is not provided.

Many applications don't use Python header files, but access directly libpython. For example, py2app uses dlsym():
https://bitbucket.org/ronaldoussoren/py2app/src/default/py2app/apptemplate/src/main.c

PyInstaller uses GetProcAddress() on Windows or dlsym() on other platforms:
* https://github.com/pyinstaller/pyinstaller/blob/1844d69f5aa1d64d3feca912ed1698664a3faf3e/bootloader/src/pyi_python.h
* https://github.com/pyinstaller/pyinstaller/blob/1844d69f5aa1d64d3feca912ed1698664a3faf3e/bootloader/src/pyi_pythonlib.c

That's why PyRun_String() is defined as an alias using a macro *and* as a function in pythonrun.c:

#undef PyRun_String
PyObject *
PyRun_String(const char *str, int s, PyObject *g, PyObject *l)
{
    return PyRun_StringFlags(str, s, g, l, NULL);
}
msg345871 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-17 15:52
> A look through `git log -p` looks like bpo-34646 is likely to be related.

New changeset e5024517811ee990b770fca0ba7058742d00e032 by Benjamin Peterson in branch 'master':
closes bpo-34646: Remove PyAPI_* macros from declarations. (GH-9218)
https://github.com/python/cpython/commit/e5024517811ee990b770fca0ba7058742d00e032

Extract of this change:

 
 #undef PyRun_String
-PyAPI_FUNC(PyObject *)
+PyObject *
 PyRun_String(const char *str, int s, PyObject *g, PyObject *l)
 {
     return PyRun_StringFlags(str, s, g, l, NULL);
 }
 

On Windows, this change removed dllexport from PyRun_String(). My PR 14142 adds it back, to pythonrun.h.
msg345873 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-06-17 16:15
I haven't fully tested this, but a suitable test using ctypes might look like:

py = ctypes.PyDLL("", handle=sys.dllhandle)
missing = {name for name in EXPECTED_NAMES if not hasattr(py, name)}
# assert 'missing' is empty
msg345915 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-17 21:29
I tested the following code:
---
import ctypes, sys 
names = """
PyRun_String
PyRun_AnyFile
PyRun_AnyFileEx
PyRun_AnyFileFlags
PyRun_SimpleString
PyRun_SimpleFile
PyRun_SimpleFileEx
PyRun_InteractiveOne
PyRun_InteractiveLoop
PyRun_File
PyRun_FileEx
PyRun_FileFlags
"""
api = ctypes.pythonapi
api2 = ctypes.PyDLL("", handle=sys.dllhandle)
for name in names.split():
    if not hasattr(api, name) or not hasattr(api2, name):
        print("MISSING NAME", name)
---

Current output:

Missing names ['PyRun_AnyFile', 'PyRun_AnyFileEx', 'PyRun_File', 'PyRun_FileEx',
 'PyRun_FileFlags', 'PyRun_InteractiveLoop', 'PyRun_InteractiveOne', 'PyRun_Simp
leFile', 'PyRun_SimpleFileEx', 'PyRun_SimpleString', 'PyRun_String']

With my PR 14142:

Missing names []
msg345919 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-06-17 21:51
> api = ctypes.pythonapi
> api2 = ctypes.PyDLL("", handle=sys.dllhandle)

Those should be the same. In Windows, pythonapi is defined as PyDLL("python dll", None, sys.dllhandle). 

Wouldn't it be better to add a function to _testcapi that checks GetProcAddress(PyWin_DLLhModule, name)?
msg345926 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-17 22:02
"Those should be the same."

Well, I wasn't 100% sure so I tested both. At least, I can now confirm that missing symbols are now exposed in both :-D
msg345928 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-06-17 22:10
Ah, that's what ctypes.pythonapi is :) I looked at PyDLL first and figured it out from there.

Should we add a regression test to avoid this happening in the future?
msg345929 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-17 22:15
New changeset 343ed0ffe0d5ddd4f17c31e14a656a04ac7dfc19 by Victor Stinner in branch 'master':
bpo-37189: Export old PyRun_XXX() functions (#14142)
https://github.com/python/cpython/commit/343ed0ffe0d5ddd4f17c31e14a656a04ac7dfc19
msg345930 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-17 22:18
> Should we add a regression test to avoid this happening in the future?

I'm not sure where to add such test, nor which kind of test is needed. I mean, should we only test that the symbol is present? Or should we also test the ABI? Or even write a functional test?

Since I didn't know, I just merged my fix, to make sure that it lands before 3.8beta2.
msg345931 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-17 22:19
My notes on checking an ABI:
https://pythoncapi.readthedocs.io/stable_abi.html#check-for-abi-changes

I didn't check if https://abi-laboratory.pro/tracker/timeline/python/ supports Windows or not.
msg345934 - (view) Author: miss-islington (miss-islington) Date: 2019-06-17 22:42
New changeset 8cb8d5de4bcc587b35d1b2f4166dad98c202805c by Miss Islington (bot) in branch '3.8':
bpo-37189: Export old PyRun_XXX() functions (GH-14142)
https://github.com/python/cpython/commit/8cb8d5de4bcc587b35d1b2f4166dad98c202805c
msg345975 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-18 10:06
I close the issue. The initial issue has been fixed (PyRun_String is now exported again in the Python DLL).

Eryk Sun:
> Wouldn't it be better to add a function to _testcapi that checks GetProcAddress(PyWin_DLLhModule, name)?

I have no idea.

Steve Dower:
> Should we add a regression test to avoid this happening in the future?

As I explained, I'm not sure where to put such test, I'm not sure what should be tested. If someone wants to work on that, I suggest to open a separated issue.
History
Date User Action Args
2019-06-18 10:06:56vstinnersetstatus: open -> closed
versions: + Python 3.9
messages: + msg345975

resolution: fixed
stage: patch review -> resolved
2019-06-17 22:42:33miss-islingtonsetnosy: + miss-islington
messages: + msg345934
2019-06-17 22:19:00vstinnersetmessages: + msg345931
2019-06-17 22:18:19vstinnersetmessages: + msg345930
2019-06-17 22:15:24miss-islingtonsetpull_requests: + pull_request14016
2019-06-17 22:15:16vstinnersetmessages: + msg345929
2019-06-17 22:10:24steve.dowersetmessages: + msg345928
2019-06-17 22:02:26vstinnersetmessages: + msg345926
2019-06-17 21:51:30eryksunsetnosy: + eryksun
messages: + msg345919
2019-06-17 21:29:53vstinnersetmessages: + msg345915
2019-06-17 16:15:00steve.dowersetmessages: + msg345873
2019-06-17 15:52:09vstinnersetmessages: + msg345871
2019-06-17 07:48:56vstinnersetmessages: + msg345809
2019-06-17 07:42:37vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request13983
2019-06-17 00:01:49pyscriptersetnosy: + pyscripter
messages: + msg345792
2019-06-08 17:21:00benjamin.petersonsetmessages: + msg345056
2019-06-07 18:20:17zach.waresetnosy: + benjamin.peterson
messages: + msg344993
2019-06-07 18:13:47cgohlkesetmessages: + msg344989
2019-06-07 17:59:19steve.dowersetnosy: + vstinner
messages: + msg344982
2019-06-07 07:55:23cgohlkecreate