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

fix and update ruby bindings #200

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Conversation

thedavemarshall
Copy link
Contributor

This introduces a few changes/fixes for the ruby bindings. While the process for building and installing the gem locally was working, I ran into some issues in another project when trying to use bundler to build the gem from the remote git source for me. I've verified these changes address those issues.

  • since building the gem appears to copy the Cargo.toml and Cargo.lock files before attempting to build native extensions on the host, this fails with relative paths to regorus from regorusrb. I think this might eventually be solvable with more advanced build scripts for the ruby gem, or would eventually be solved if the gem ends up built/published as part of regorus CI and includes pre-compiled binaries for distribution. In the meantime, I'm addressing this by pointing regorusrb to depend on regorus = { git = "https://github.com/microsoft/regorus" }
  • related to above, change bindings/ruby/LICENSE.txt from a symlink to a copy of LICENSE
  • Update bindings/ruby/README.md to indicate the ruby gem name is regorusrb, remove bundle add command until glob option is supported, add instructions for building and installing the rubygem from source
  • require 'rb-sys' in gemspec for building the gem, specify extension through extconf.rb, which will auto-install the rust toolchain if not present. Also include rake-compiler-rust gem when building and developing locally, which allows cross-compiling for different architectures, i.e. rake 'native[x86_64-linux]'
  • change the output location for dynamic libraries from bindings/ruby/regorusrb/regorusb.so (or appropriate file extension) to bindings/ruby/regorus/regorusrb.so. This is more in keeping with the ruby module/namespace Regorus, while allowing referencing the dynamice library regorusrb distinctly from the regorus ruby code, which is uesful to avoid ambiguous require and require_relative statements in ruby
  • add Regorus::Engine#eval_bool_query(), #eval_allow_query, and #eval_deny_query methods

@anakrish
Copy link
Collaborator

Thanks for the contribution!

ext.lib_dir = "lib/regorusrb"
ext.lib_dir = "lib/regorus"
ext.cross_compile = true
ext.cross_platform = %w[x86-mingw32 x64-mingw-ucrt x64-mingw32 x86-linux x86_64-linux x86_64-darwin arm64-darwin]
Copy link
Collaborator

@anakrish anakrish Apr 10, 2024

Choose a reason for hiding this comment

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

Does Ruby have a non mingw Windows target that we could use?

Copy link
Collaborator

@anakrish anakrish left a comment

Choose a reason for hiding this comment

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

LGTM

@anakrish anakrish merged commit 6a16714 into microsoft:main Apr 10, 2024
4 checks passed
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.

2 participants