On Mon, 30 Sep 2019 at 09:37, Victor Stinner <vstin...@python.org> wrote:
>
> Le lun. 30 sept. 2019 à 00:33, Nick Coghlan <ncogh...@gmail.com> a écrit :
> > As noted above, despite what I wrote on BPO, you no longer need to persuade
> > me that the version check is desirable, only that a narrow check on
> > specific struct sizes is preferable to a broad check on the expected API
> > version.
>
> I understand that your main motivation to use the Python version
> number rather than sizeof(PyConfig) is the error message.
No, my main motivation is to create an API that can emit a useful
error message on *ALL* version conflicts between an embedding
application and the embedded runtime, not just version conflicts
involving versions that change the size of the config structs.
The latter option is a good one if all we want to version is the
struct itself, but I don't think that's what we really want here: I
think we want to version check the entire API/ABI.
The improved error message is just a nice bonus, as is the fact that I
think the following code is more self-explanatory than the size
checks:
PyConfig config;
config.header_version = PY_VERSION_HEX;
PyStatus status = PyConfig_InitPythonConfig(&config);
The documentation would say something like "header_version should
specify the version of the CPython headers that was used to build and
link the embedding application. The simplest way to set it correctly
is to set it to ``PY_VERSION_HEX``".
> If we implement support for older PyConfig ("stable ABI"), you will
> simply never see this error: it will just work transparently.
But that's true regardless of whether the consistency check is based
on the struct size or the header version.
> IMHO the current error message is good enough:
>
> if (config->struct_size != sizeof(PyConfig)) {
> return _PyStatus_ERR("unsupported PyConfig structure size "
> "(Python version mismatch?)");
> }
That error message won't trigger if the public API struct that changed
size is PyTypeObject or PyObject (or any of the other structs in the
full API/ABI), whereas a PY_VERSION_HEX based check can be made to
fail based on any policy we choose to enforce.
For example, we could make the runtime enforce the following pair of rules:
* if both the header version and runtime version are final releases or
release candidates, then only the major and minor version fields need
to match (the X.Y in X.Y.Z)
* if either the header version or the runtime version is an alpha or
beta release (or an unknown release type), then the entire version
number must match exactly
Or we could be more permissive, and only ever enforce the first rule,
even for alpha and beta releases.
> I wrote a proof-of-concept to check if it would be doable to support
> multiple versions (sizes) of PyConfig: it's doable and it's quite easy
> to implement, a few lines of code. For example, support Python 3.8
> PyConfig in Python 3.9.
That doesn't change all that much with a Py_VERSION_HEX based check -
it just needs an extra table for range based lookups to find the
relevant struct size for the given version, together with keeping
definitions of the old config structs around.
typedef struct {
uint32_t firstVersion;
Py_ssize_t structSize;
} _Py_versionToSize;
static _Py_versionToSize _PyConfigStructSizes {
{0x03090000, sizeof(_PyConfig_3_9)},
{0, sizeof(_PyConfig_3_8)}
}
If the given version matched, you wouldn't need to do the size lookup.
You also wouldn't be constrained to having the lookup table be from
versions to struct sizes - it could instead be from versions to
function pointers, or some other such thing, based on whatever made
the most sense given how the rest of the config system worked in any
given release.
Cheers,
Nick.
--
Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at
https://mail.python.org/archives/list/python-dev@python.org/message/RL2TEQ6BEGOHSWPPUVML5SLNIBVGFJ5G/
