Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SWI-Prolog versions > 8.5.2 #133

Merged
merged 4 commits into from Jan 3, 2023

Conversation

david-r-cox
Copy link
Contributor

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.

After SWI-Prolog 8.5.2, PL_version was renamed to PL_version_info,
causing pyswip to crash when used with SWI-Prolog versions > 8.5.2.
@JanWielemaker
Copy link
Contributor

@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.

pyswip/core.py Outdated
PL_VERSION=70000 # Best guess. When was PL_version introduced?
# swi-prolog <= 8.5.2:
try:
PL_version = _lib.PL_version
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use Aleph and it wasn't working with SWI_prolog 9.x until I changed this line to PL_version = _lib.PL_version_info.

@yuce
Copy link
Owner

yuce commented Dec 1, 2022

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

@david-r-cox
Copy link
Contributor Author

@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
Copy link
Owner

yuce commented Jan 3, 2023

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

@yuce
Copy link
Owner

yuce commented Jan 3, 2023

Thanks for your contribution!

@yuce yuce merged commit 841d08f into yuce:master Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants