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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WiP] Replace therubyracer with mini_racer (updates v8) #1496

Merged
merged 3 commits into from Apr 13, 2021

Conversation

2called-chaos
Copy link
Member

@2called-chaos 2called-chaos commented Mar 23, 2021

Please don't merge yet.

Replacing therubyracer (dead since 2017) with mini_racer allows the ancient version of libv8 to get an update from 3.16 to 8.4 which fixes build errors on current systems (#1492 #1494).

  • Tests pass (CI fails because it uses bundler 2.2)
  • rails assets:precompile passes
  • bundle install passes with bundler >= 2.2

Problem with bundler 2.2

As I understand it this is caused by a bug in libv8 and by a change in bundler which had unintended side effects. With how this PR is right now this will cause bundler 2.2 to fail with an error as described in the v8 bug.

As I understand the linked bundler PR should make it so that a lock generated with <2.2 should fallback to old behaviour but I fail to understand what that effectively means as it still fails.

Anyway, for bundler 2.2 we would now need to specify the specific platforms and I'm not sure what to add here. To be able to bundle on my Mac and my Ubuntu machine I need to add the following two:

bundle lock --add-platform x86_64-darwin-19
bundle lock --add-platform x86_64-linux

As I'm not quite sure how backwards compatible this "specific platform" change is, I haven't yet added it to this PR. I just know that darwin alone has 16-19 potentially.

**Maybe we can collect a few common platforms? To get yours: **

ruby -e 'puts Gem::Platform.local'

# or if that outputs nothing ruby is probably aliased (bundle exec)

\ruby -e 'puts Gem::Platform.local'

Let me know what you think.


Update

What irks me a bit is that we don't do any of that in another project which is bundled on 2.2. It still only has the generic "ruby" platform and no darwin/linux specific libv8 versions. When I remove java as a platform from errbit's lock file (so only ruby remains) I can bundle without problems on both machines and no platform specific gem versions will be added to the lock file. Unfortunately I have no clue about jruby and Java and why this happens.

Update 2

There is just something wrong somewhere in libv8 and/bundler. Remember when I said we don't have this problem in another repository? Well...

$ ruby -v
bundler ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin19]

$ bundler -v
Bundler version 2.2.15

$ tail -2 Gemfile.lock
BUNDLED WITH
   2.2.14

$ bundle | grep libv8
Using libv8 8.4.255.0 (x86_64-darwin-19)

$ grep libv8 Gemfile.lock
    libv8 (8.4.255.0)
      libv8 (~> 8.4.255)

$ echo "gem 'dle'" >> Gemfile

$ bundle
[..]
Installing libv8 8.4.255.0 with native extensions
Installing dle 1.0.1
Gem::Ext::BuildError: ERROR: Failed to build gem native extension.
[..]
An error occurred while installing libv8 (8.4.255.0), and Bundler cannot continue.
Make sure that `gem install libv8 -v '8.4.255.0' --source 'https://www.rubygems.org/'` succeeds before bundling.

In Gemfile:
  mini_racer was resolved to 0.3.1, which depends on
    libv8

Almost uncut output: https://gist.github.com/2called-chaos/6025fb23a6cc85bb663e7fe62d7d589c

Bundle is fine and functioning, I add a totally unrelated gem and it fails? 馃槙 Also downloads 1GB every time you try it 馃檮

@2called-chaos
Copy link
Member Author

I came to the same conclusion that due to Bundler's new enhanced multi platform support we now have to add every platform we want to support to the lockfile. Since they may often change (on linux it seems to be simple on Mac not so much) we could just go with adding the following as a setup/upgrade step? The lockfile isn't really guaranteed to be "git clean" due to UserGemfile anyways.

# [..]
bundle lock --add-platform $(\ruby -e 'puts Gem::Platform.local')
bundle install
# [..]

Any objections?

@stevecrozz
Copy link
Member

I took a look around and I found discourse doing this:
https://github.com/discourse/discourse/blob/5fe1d1f84f9954d9522b59b23bb0392dd10c1634/Gemfile.lock#L469-L475

Without really studying it, I'd be inclined to follow what other popular projects are doing.

@2called-chaos
Copy link
Member Author

Adding the platforms works for me on Mac and Ubuntu but M1-Macs have a whole different issue and we are probably missing jruby variations. And I don't know how to fix travis. I still think this is mostly an libv8 issue.

As this is a rather unresolved issue with 2.2 should we just pin bundler to 2.1.X for now? Like the Dockerfile does.

@stevecrozz
Copy link
Member

As this is a rather unresolved issue with 2.2 should we just pin bundler to 2.1.X for now? Like the Dockerfile does.

Yeah, I like this idea.

@stevecrozz
Copy link
Member

Are you happy with this @2called-chaos ? It LGTM

@2called-chaos
Copy link
Member Author

Are you happy with this

Well I guess. Should I add something to the readme/install instructions? That when an error with v8 occurs while bundling to either use bundler 2.1 or run the above mentioned command (and maybe ask to post the platform here so we can add it to the lockfile)?

@stevecrozz
Copy link
Member

It works great for me, it works for you and it passes the tests. I'll merge it. We can keep an eye on the issues here and see if there's any trouble.

@stevecrozz stevecrozz merged commit a0592ab into errbit:master Apr 13, 2021
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.

None yet

2 participants