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

seems to require re-writing in places #65

Open
simon-dedeo opened this issue Jan 31, 2021 · 14 comments
Open

seems to require re-writing in places #65

simon-dedeo opened this issue Jan 31, 2021 · 14 comments

Comments

@simon-dedeo
Copy link

simon-dedeo commented Jan 31, 2021

I've tried to get this gem working for ruby 3.0; unfortunately, it seems that some major changes to ruby under the hood make this a rather complex problem.

A few things are simple to fix (e.g., moving "EXTERN" to "RUBY_EXTERN" in some declarations). But other issues seem to involve changes to the macros sitting inside the ruby code itself, e.g., "RB_OBJ_WRITE", which has a new definition in ruby 3.0.

linalg.c:154:5: error: cannot take the address of an rvalue of type 'VALUE' (aka 'unsigned long')
    RBGSL_SET_CLASS(omatrix, cgsl_matrix_LU);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/rb_gsl_common.h:302:5: note: expanded from macro 'RBGSL_SET_CLASS'
    RBGSL_SET_CLASS0(_obj_, cls); \
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/rb_gsl_common.h:296:56: note: expanded from macro 'RBGSL_SET_CLASS0'
#define RBGSL_SET_CLASS0(obj0, cls) RB_OBJ_WRITE(obj0, &(RBASIC_CLASS(obj0)), cls)
                                    ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/local/include/ruby-3.0.0/ruby/internal/rgengc.h:108:52: note: expanded from macro 'RB_OBJ_WRITE'
    RBIMPL_CAST(rb_obj_write((VALUE)(a), (VALUE *)(slot), (VALUE)(b), __FILE__, __LINE__))
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/local/include/ruby-3.0.0/ruby/internal/cast.h:33:29: note: expanded from macro 'RBIMPL_CAST'
# define RBIMPL_CAST(expr) (expr)

After a lot of fiddling, I think there's a solution.

  1. First, replace all "EXTERN" and "extern" with "RUBY_EXTERN" and "RUBY_EXTERN". You can do this using rpl.
  2. The remaining problem is the RBGSL_SET_CLASS macro. In the updated ruby 3.0, there's a new macro that sets the class directly. You'll want to edit the older definition of RBGSL_SET_CLASS as follows (I think):
#ifndef RBGSL_SET_CLASS
#define RBGSL_SET_CLASS(obj, cls) do { \
    VALUE _obj_ = (obj); \
    RBASIC_SET_CLASS(_obj_, cls); \
} while (0)
#endif
  1. then things actually do compile. You have to wrestle with gems for awhile, but it seems to install and run from irb3.0

If anyone's reading this and is interested in updating the gem to work with 3.0, let me know.

@katafrakt
Copy link
Contributor

@simon-dedeo thanks for this investigation! I ended up downgrading to Ruby 2.7 to get GSL to work but I guess this is not a good solution. It'd be great to see a fork of this working for Ruby 3 (and modern Ruby 2 as well I guess).

@simon-dedeo
Copy link
Author

simon-dedeo commented Feb 23, 2021 via email

@katafrakt
Copy link
Contributor

katafrakt commented Feb 23, 2021

I forked the repo and I plan to update tests to check all modern Ruby versions. Then I can release it as a gem. Any ideas for the name? gsl-reborn? gsl-next?

@simon-dedeo
Copy link
Author

simon-dedeo commented Feb 23, 2021 via email

@david-macmahon
Copy link
Contributor

I'm no longer active on this project, but I would support adding new maintainers (i.e. folks who are actually using/developing rb-gsl). That might be better than forking/renaming.

@katafrakt
Copy link
Contributor

@david-macmahon that's an option too. I'm no expert in GSL though, I only use it in Jekyll for similar posts 😉 But for sure I can do some maintenance chores, this is just Ruby after all.

@simon-dedeo
Copy link
Author

How does that work? I agree that it would be nice to keep things on the same name. I use GSL quite a bit, but don't have the technical skills to do more than help out occasionally.

@david-macmahon
Copy link
Contributor

The most recent commit seems to be from almost 4 years ago so it seems unlikely to be current/compatible with modern Ruby and GSL implementations. Looking through the issues seems to confirm that. You two seem active, motivated, and competent so I'd vote for adding you both.

It looks like the "SciRuby" organization has a "gsl-admin" team that you would need to be added to. I am a member of that team, but I cannot add new members. I think you might have to lobby someone in the SciRuby "core" team to get added to the "gsl-admin" team.

@katafrakt
Copy link
Contributor

It took some time to understand all the preprocessor magic and subtle changes in Ruby 3 C API, but I created a pull request that compiles against Ruby 3, passes tests and does not break backwards compatibility: #66

I have no idea who to "lobby" in order to get this live.

@simon-dedeo
Copy link
Author

simon-dedeo commented Mar 2, 2021 via email

@katafrakt
Copy link
Contributor

@simon-dedeo just replace gem 'gsl' in your Gemfile with

gem 'gsl', git: 'https://github.com/katafrakt/rb-gsl.git', branch: 'ruby3-compatibility'

@simon-dedeo
Copy link
Author

Thank you! It certainly installed; I'm giving it a workout now.

@0xdevalias
Copy link

Just wanted to update this thread in 2023.

It seems the above PR got merged:

But at least according to this issue, hasn't been released to rubygems yet:

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

No branches or pull requests

4 participants