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

Skip compilation on TruffleRuby #38

Merged
merged 1 commit into from
Jan 12, 2023
Merged

Conversation

eregon
Copy link
Contributor

@eregon eregon commented Jan 10, 2023

  • Running libv8 on GraalVM LLVM is unlikely to ever work, instead using Graal.js makes more sense. This is already the case in mini_racer. libv8-node is a necessary dependency of mini_racer on CRuby, so for TruffleRuby the extconf.rb simply creates a dummy Makefile.

As mentioned in rubyjs/mini_racer#261 (comment) this is useful if for some reason libv8-node is installed "from source" on TruffleRuby.
Which can happen either when using gem github: in a Gemfile like there, or potentially due to a Bundler issue for example rubygems/rubygems#6165, or if the gem was installed on a platform where it's not available precompiled.

@lloeki
Copy link
Collaborator

lloeki commented Jan 11, 2023

@eregon thanks for this! Can you make this PR target the node-16 branch instead?

@eregon eregon changed the base branch from master to node-16 January 11, 2023 19:08
* Running libv8 on GraalVM LLVM is unlikely to ever work, instead using
  Graal.js makes more sense. This is already the case in mini_racer.
  libv8-node is a necessary dependency of mini_racer on CRuby, so for
  TruffleRuby the extconf.rb simply creates a dummy Makefile.
@eregon
Copy link
Contributor Author

eregon commented Jan 11, 2023

Done

@lloeki
Copy link
Collaborator

lloeki commented Jan 12, 2023

Thanks!

@lloeki lloeki merged commit 6da5856 into rubyjs:node-16 Jan 12, 2023
@eregon
Copy link
Contributor Author

eregon commented Jan 12, 2023

@lloeki Would it make sense to cherry-pick this changes on other branches too and on master? It seems best to be included on every active branch.

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