search

yuce / pyswip

PySwip is a Python - SWI-Prolog bridge enabling to query SWI-Prolog in your Python programs. It features an (incomplete) SWI-Prolog foreign language interface, a utility class that makes it easy querying with Prolog and also a Pythonic interface.
MIT License
464 stars 97 forks source link

Support SWI-Prolog versions > 8.5.2 #133

Closed david-r-cox closed 1 year ago

david-r-cox commented 2 years ago

PL_version was renamed to PL_version_info in SWI-Prolog due to Exported symbol PL_version conflicts with symbol in Perl.

I noticed that this change resulted in a crash in PySwip, and opened swipl-devel #910.

After reviewing the notes added to that issue by @dgelessus, I decided to open this PR with a fix.

Jan recently had to rename SWI's PL_version function to PL_version_info, to avoid a name conflict with Perl - see #900 for the full context.

It looks like PySwip tries to access the PL_version function, and if it can't find it, it assumes SWI-Prolog 7. With the recent renaming, PL_version doesn't exist anymore, so PySwip is misdetecting SWI-Prolog 8.5 as version 7. It then uses an older set of constant definitions, which are not compatible with the current SWI version, and this is probably causing the assertion failure.

The way to fix this would be to extend your version check code to try calling PL_version_info first, and if that function doesn't exist, fall back to PL_version.

Perhaps it would also be a good idea to throw an error if neither PL_version_info nor PL_version can be found? It looks like PySwip requires at least SWI-Prolog 8.2.0 anyway - so if the SWI-Prolog library doesn't define any of the version symbols, it's either too old to work with PySwip, or the library is broken in some way.

JanWielemaker commented 1 year ago

@yuce, could you please merge this one. It could also be done a bit simpler, e.g.,

    if _lib.PL_version_info != None:
       PL_version = _lib.PL_version_info
    else:
       PL_version = _lib.PL_version

Both will run on older versions as well as on the latest stable and dev versions.

yuce commented 1 year ago

@david-r-cox Would it be possible for you to incorporate @JanWielemaker 's suggestion?

david-r-cox commented 1 year ago

@yuce I've added @JanWielemaker's suggested changes and tested them against swipl-devel V9.1.2 (see this Dockerfile).

Let me know if any further changes are required.

yuce commented 1 year ago

@david-r-cox Thanks for the update! Could you also add your name to CONTRIBUTORS.txt?

yuce commented 1 year ago

Thanks for your contribution!