search

Shopify / liquid-c

Liquid performance extension in C.
MIT License
119 stars 24 forks source link

Fixes for TruffleRuby and add TruffleRuby in CI #211

Closed eregon closed 7 months ago

eregon commented 7 months ago

See the individual commits for details.

Needs https://github.com/oracle/truffleruby/pull/3400 for the truffleruby-head job to pass the test suite. (it can already be merged though, as you prefer)

casperisfine commented 7 months ago

Seems to be missing some more checks: symbol lookup error: /home/runner/work/liquid-c/liquid-c/lib/liquid_c.so: undefined symbol: rb_check_funcall

casperisfine commented 7 months ago

Nevermind, you mentioned it 🤦

eregon commented 7 months ago

Thanks for merging!

eregon commented 7 months ago

@casperisfine or @nirvdrum Could you rerun https://github.com/Shopify/liquid-c/actions/runs/7615407078/job/20739862301 ? The rb_check_funcall PR got merged so if rerun the tests should pass and I can track if it goes red with https://github.com/eregon/truffleruby-gem-tracker.

eregon commented 7 months ago

I know this might be a big ask, but would it be possible to make a release including these fixes? I would like to run liquid-c in yjit-bench on truffleruby. That currently uses 4.1.0: https://github.com/Shopify/yjit-bench/blob/main/benchmarks/liquid-c/Gemfile So a 4.1.1 release or so with these fixes would be great.

casperisfine commented 7 months ago

Could you rerun

Done: https://github.com/Shopify/liquid-c/actions/runs/7615407078/job/20764127709

I know this might be a big ask, but would it be possible to make a release including these fixes?

Hum, it's a bit tricky. Our team doesn't really handle that gem. It's OK for us to merge pure compatibilty patches, but cutting a release is over stepping a bit. I'll try to track down the owners and ask.

casperisfine commented 7 months ago

4.2.0 is out: https://rubygems.org/gems/liquid-c/versions/4.2.0

eregon commented 7 months ago

Thank you!