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 latest rust/wasmer-runtime/ruby #45

Closed

Conversation

geoffyoungs
Copy link

The current version of the gem/master doesn't compile with rustc 1.50.0 - this PR works for me, but I'm very much a rust newbie & I don't pretend to understand all the implications of these changes. With the attached patches, the code compiles and tests pass on Ruby 2.7/3.0

Changes:

  • Update crate for wasmer-runtime & update API calls
  • Update rutie for latest ruby support
  • Add no-link option to rutie to fix link error (libruby will always be present)
  • Suppress warning about AbstractObject

- Update crate for wasmer-runtime & update API calls
- Update rutie for latest ruby support
- Add no-link option to rutie to fix link error (libruby will always be present)
- Suppress warning about AbstractObject
@Hywan
Copy link
Contributor

Hywan commented Mar 3, 2021

bors try

bors bot added a commit that referenced this pull request Mar 3, 2021
@Hywan
Copy link
Contributor

Hywan commented Mar 3, 2021

Thanks for the PR :-). We are going to revamp the project entirely, meanwhile your PR is very welcome! Thanks!

@Hywan Hywan self-assigned this Mar 3, 2021
@Hywan Hywan added the 🐞 bug Something isn't working label Mar 3, 2021
@bors
Copy link
Contributor

bors bot commented Mar 3, 2021

try

Build failed:

@geoffyoungs
Copy link
Author

Thanks for taking a look - I notice the github workflow isn't working, but that is unrelated to the code changes in the PR.

I've added a commit to make the workflow to use some standard-ish actions for ruby/rust install, which is sufficient for it to run/compile again. cf. https://github.com/livelink/wasmer-ruby/actions/runs/617310318

@geoffyoungs
Copy link
Author

bors try

@bors
Copy link
Contributor

bors bot commented Mar 4, 2021

🔒 Permission denied

Existing reviewers: click here to make geoffyoungs a reviewer

@Hywan
Copy link
Contributor

Hywan commented Mar 4, 2021

OK, thanks for your PR :-). I'll get back to it as soon as I've time. I don't want to merge it like that because we have plans for this project that could conflict with the PR. But for sure, it will be integrate one way or another! Thanks!

@geoffyoungs
Copy link
Author

OK, thanks for your PR :-). I'll get back to it as soon as I've time. I don't want to merge it like that because we have plans for this project that could conflict with the PR. But for sure, it will be integrate one way or another! Thanks!

Sure - thanks for taking a look.

@Hywan
Copy link
Contributor

Hywan commented May 11, 2021

Closed by #48 which rewrites the entire project. It supports the latest version of Rutie, but it also introduces new crates like rutie-derive and rutie-derive-macros that introduce the #[rubyclass], #[rubymethods] and #[rubyfunction] procedural macros. It makes the code so much easier to write, to read, and to maintain!

I've noticed that you're using the rutie features no-link. Why? It makes tests to fail miserably on my machine with this feature.

@Hywan Hywan closed this May 11, 2021
@geoffyoungs
Copy link
Author

Thanks for the update - I'll give that a go.

I added "no-link" to rutie because I was getting link errors & the feature solved them. Tests were passing fine on my machine.

From the docs, the link option should only be needed when embedding ruby within rust - but this is compiles to a shared library loaded from libruby, so all of the symbols in libruby should be available without needing to specify it as a dependency?

@Hywan
Copy link
Contributor

Hywan commented May 11, 2021

We aren't embedding Ruby inside Rust, it's rather the opposite, so I understand it's not necessary. Can you tell me what linking issues to have (if you can reproduce them)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants