PEP 587 implementation is not ABI forward compatible

[vstinner]

BPO	38304
Nosy	@ncoghlan, @vstinner, @ned-deily, @ambv, @miss-islington
PRs	bpo-38304: Add PyConfig.struct_size #16451 [3.8] bpo-38304: Add PyConfig.struct_size (GH-16451) #16453 bpo-38304: Fix PyConfig usage in python_uwp.cpp #16487 [3.8] bpo-38304: Fix PyConfig usage in python_uwp.cpp #16488 [3.8] bpo-38304: Remove PyConfig.struct_size #16500 bpo-38304: Remove PyConfig.struct_size (GH-16500) #16508 bpo-38304: PyConfig_InitPythonConfig() cannot fail anymore #16509 [3.8] bpo-38304: PyConfig_InitPythonConfig() cannot fail anymore (GH-16509) #16510

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-10-01.10:27:28.387>
created_at = <Date 2019-09-28.00:32:44.723>
labels = ['interpreter-core', '3.8']
title = 'PEP 587 implementation is not ABI forward compatible'
updated_at = <Date 2019-10-01.10:27:28.386>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2019-10-01.10:27:28.386>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-10-01.10:27:28.387>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2019-09-28.00:32:44.723>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 38304
keywords = ['patch']
message_count = 18.0
messages = ['353430', '353432', '353435', '353436', '353437', '353487', '353490', '353509', '353510', '353563', '353564', '353621', '353644', '353651', '353652', '353656', '353657', '353658']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'vstinner', 'ned.deily', 'lukasz.langa', 'miss-islington']
pr_nums = ['16451', '16453', '16487', '16488', '16500', '16508', '16509', '16510']
priority = None
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38304'
versions = ['Python 3.8']

[vstinner]

The current implementation of the PEP-587 is not ready for future evolutions of the PyPreConfig and PyConfig structure. Any change will break the ABI compatibility. Their _config_version field is useless.

The first versions of the PEP-587 used a macro to statically initialize the structure. It was decided to get ride of macros to use functions instead.

Example:

   PyConfig config;
   PyStatus status = PyConfig_InitPythonConfig(&config);

PyConfig_InitPythonConfig() gets unitialized memory and doesn't know the size of the config variable.

I propose to require to store the size of the structure in the structure directly:

• Add PyPreConfig.struct_size and PyConfig.struct_size
• Require to initialize these fields to sizeof(PyPreConfig) and sizeof(PyConfig) respectively

Example:
     
   PyConfig config;
   config.struct_size = sizeof(PyConfig);
   PyStatus status = PyConfig_InitPythonConfig(&config);

Thanks to the struct_size field, PyConfig_InitPythonConfig() can support old PyConfig structures without breaking the ABI.

If an old PyConfig is passed to PyConfig_Read(): newer fields will be ignored.

[vstinner]

I updated the PEP-587:
python/peps@afa38c0

[vstinner]

New changeset 441b10c by Victor Stinner in branch 'master':
bpo-38304: Add PyConfig.struct_size (GH-16451)
441b10c

[vstinner]

New changeset 6e12838 by Victor Stinner in branch '3.8':
bpo-38304: Add PyConfig.struct_size (GH-16451) (GH-16453)
6e12838

[vstinner]

Ok, the API should now be ABI future proof thanks to this change.

I experimented a change locally: it's possible to support two sizes of the PyConfig structure (need to modify a few functions in initconfig.c).

I sent an email to python-dev:
https://mail.python.org/archives/list/python-dev@python.org/thread/C7Z2NA2DTM3DLOZCFQAK5A2WFYO3PHHX/

[ncoghlan]

The hidden _config_version field wasn't in the accepted PEP.

Both it and struct_size should be removed, as we don't support updating an embedded Python runtime to a new X.Y.0 release without rebuilding the embedding application.

[ned-deily]

Nick, do you consider this a 3.8.0 release blocker? If so, you should reopen it and mark it as such, since the 3.8.0rc1 cutoff is imminent.

[vstinner]

The hidden _config_version field wasn't in the accepted PEP.

It was in the accepted PEP and I replaced it with struct_size:
python/peps@afa38c0

I suggest to discuss the issue on the thread that I started on python-dev.

[vstinner]

Both it and struct_size should be removed, as we don't support updating an embedded Python runtime to a new X.Y.0 release without rebuilding the embedding application.

See my reply on python-dev:
https://mail.python.org/archives/list/python-dev@python.org/message/ZE5K7MEKN32YGHNA5457AROWUNI7JOWC/

[vstinner]

New changeset 89f8177 by Victor Stinner in branch 'master':
bpo-38304: Fix PyConfig usage in python_uwp.cpp (GH-16487)
89f8177