search

kkaempf / swig-issues

Issues from SWIG (testing)
0 stars 0 forks source link

[guile] guile module uses deprecated interfaces #38

Open SwigAtSF opened 11 years ago

SwigAtSF commented 11 years ago

During the 1.7 development branch of guile, a lot of the API changed, especially with regaurd to memory allocation /garbage colleciton, and string representation. SWIG currently supports only the 1.6 style API, but now that guile has stabilized and 1.8 is released, we need to update to the new interface. This patch does that.

SwigAtSF commented 11 years ago

Patch to detect and utilize new guile API

SwigAtSF commented 11 years ago

Logged In: YES user_id=14972

Moving to the patch tracker...

SwigAtSF commented 11 years ago

Logged In: YES user_id=14972 Originator: NO

I just had a quick look at this patch. Guile isn't my area of expertise at all, but I don't think we have an active guile maintainer currently and this patch has languished for over a year.

I see one major issue with it - you're performing a configure time check for functions provided by guile, but in SWIG configure probing should only be to used to find interpreters to run tests with, and to detect functions, etc which for building SWIG itself.

It should be possible to use SWIG to generate wrappers without even having guile installed (though obviously you can't build them, but you can ship them in a tarball with your library for example). And ideally the wrappers SWIG generates should be buildable against any supported version of guile, for example if guile's C header provides a "GUILE_VERSION" define, check this to see which functions to use. Where this isn't possible SWIG sometimes provides a command line option to select what version to generate for. Or you could allow the compiling user to set a C preprocessor define to enable support for a particular version, defaulting to 1.8 if that's most popular. If 1.6 just isn't of interest to users now (I really have no idea) then supporting 1.8 only might be a reasonable approach.

SwigAtSF commented 11 years ago

Logged In: YES user_id=14972 Originator: NO

Matthias Koeppe (SWIG's guile maintainer) made some comments on this issue on the swig-dev mailing list, which I'm pasting below for the record. The full message is here:

http://article.gmane.org/gmane.comp.programming.swig.devel/17577

His comments:

Basically, current SWIG does support Guile 1.8 perfectly as far as I know, though it might use some deprecated functions. At this point of time, I will not apply patches if they break compatibility with Guile 1.6 (not sure if "swig -guile -scm" still supports Guile 1.4, though) purely for the sake of fixing deprecation warnings.

I fully agree with your comment, Olly, on the bug tracker item. A patch that uses wrapper-compile-time conditionalization (rather than SWIG-configure-time conditionalization as the submitted patch) on the Guile version to avoid using deprecated interfaces on Guile 1.8 could be easily accepted and merged.

SwigAtSF commented 11 years ago

The last comment on this bug is now more than 4 years old. Meanwhile guile has released version 2.0 and the impact of using deprecated functions has changed slightly. With guile 2.0, the wrappers fail to work as they are now.

I'm a maintainer of the gnucash project, which heavily depends on the guile bindings swig generates and we're getting more and more bug reports about this problem. I'll add some links to show what I mean:

So the problem reported here seems to have more impact on guile 2.0 than it had on 1.8.

SwigAtSF commented 11 years ago

Adam Tkac from Fedora created a patch on the Fedora bugreport (https://bugzilla.redhat.com/show_bug.cgi?id=704527) for this issue: https://bugzilla.redhat.com/attachment.cgi?id=533764&action=diff

Can you give some feedback on whether this would be acceptable for upstream inclusion ?

SwigAtSF commented 11 years ago

I have created a new patch to allow swig to work with guile 2 while remaining backwards compatible with guile 1.6 and 1.8. The patch can be found at https://bugzilla.redhat.com/attachment.cgi?id=663869

I didn't find how to attach the patch to this bug report.

Please provide some feedback on how to proceed. Thank you.