eggheads / eggdrop

The Eggdrop IRC Bot
GNU General Public License v2.0
508 stars 84 forks source link

Fix runtime python version check #1625

Closed michaelortmann closed 4 months ago

michaelortmann commented 5 months ago

Found by: michaelortmann Patch by: michaelortmann Fixes:

One-line summary: Fix runtime python version check

Additional description (if needed):

Test cases demonstrating functionality (if applicable):

thommey commented 5 months ago

The version check is pointless because it's not runtime. Runtime version check seems to be unsupported until much later Python versions unless you can find a workaround? Otherwise just remove the check entirely, right now it checks the header which we can do at compile time.

michaelortmann commented 4 months ago

We would also have to fix: https://github.com/eggheads/eggdrop/commit/093cc36cf99582b9c57f338b2a7c695db179e82a#diff-43cfee5e2cf04e6a2a99e0e92600b37c9701db5d94adf8d515785f97c1ebe626 ?

What does "much later" mean? Py_GetVersion() is available since at least python 2.6:

https://docs.python.org/2.6/c-api/init.html#Py_GetVersion

Digging deeper, since at least python 2.2:

https://docs.python.org/release/2.2/api/initialization.html

Not enough?

vanosg commented 4 months ago

Because the version is checked at compile, please remove the check here. This is an artifact from before the check was added to compile. Thanks!

michaelortmann commented 4 months ago

Done. I left the change to char *init_python(), for as the TODO comment above the function already says, we dont want to exit eggdrop if we dont have to, and quickly going over the code i cant see why we have to. To keep PRs small lets resolve the TODO in another/future PR.

thommey commented 4 months ago

I confused Py_GetVersion with Py_Version (constant) which has only been added to 3.11, looks good to me