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 Libv8::Node::Paths with RubyGems >=3.3.21 on *-linux-gnu platforms (Ported to current master) #52

Open
wants to merge 49 commits into
base: master
Choose a base branch
from

Conversation

MaZderMind
Copy link

RubyGems 3.3.21 changed the value format of Gem::Platform.local for Ruby configured with *-linux-gnu as the platform name.

$ ruby -ve 'p Gem::VERSION; p Gem::Platform.local.to_s'
ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [x86_64-linux-gnu]
"3.3.20"
"x86_64-linux"

$ ruby -ve 'p Gem::VERSION; p Gem::Platform.local.to_s'
ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [x86_64-linux-gnu]
"3.3.21"
"x86_64-linux-gnu"

$ ruby -ve 'p Gem::VERSION; p Gem::Platform.local.to_s'
ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [x86_64-linux-gnu]
"3.3.24"
"x86_64-linux-gnu"

As you can see, it now has the -gnu prefix and Libv8::Node::Paths.object_paths thus has it when the built binary actually lives in the directory x86_64-linux, breaking the build of the gems like mini_racer on those platforms.

RUBY_TARGET_PLATFORM is set to *-linux, so Libv8::Node::Paths.platform needs to align with that.

Ported forward from #36 by @MaZderMind

lloeki and others added 30 commits January 24, 2022 16:01

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
- Add make targets for local building and testing

  This is especially useful for building and testing on Darwin

- Split makefile in two: one direct and one for docker

  This is necessary because direct targets and dockerized targets
  overlap on Linux.

- Fix a few Dockerfile oversights due to updates in libexec
- arm64-darwin from x86_64-darwin (also added to CI)
- x86_64-darwin from arm64-darwin
- x86_64-linux from aarch64-linux
Use `xargs` to build the `ar` command line instead of running `ar`
manually to reduce startup costs.

Build the symbol table after adding each file instead of continually
rebuilding it.
* 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.
Skip compilation on TruffleRuby
This fixes an issue where functions that require ICU data segfault
the process due to an unexpected lack of ICU data.

The actual ICU data is provided by the icudt71_dat object.
seanmakesgames and others added 19 commits January 20, 2023 19:09
Linking on Linux currently errors with `undefined reference to `'dlsym'`
and one that supports the newer cmake.
Alpine Linux's `ar` program doesn't support having objects with the
same name in the archive twice, so we have to use the GNU extension
for including paths.

This commit *should* honestly be split into multiple separate commits,
but unfortunately pretty much all of these changes have to be applied
as a unit or a build step fails.
Add cpp test, fix locale issues and fix CI issues
RubyGems 3.3.21 changed the value format of `Gem::Platform.local` for Ruby configured with `*-linux-gnu` as the platform name.

```console
$ ruby -ve 'p Gem::VERSION; p Gem::Platform.local.to_s'
ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [x86_64-linux-gnu]
"3.3.20"
"x86_64-linux"

$ ruby -ve 'p Gem::VERSION; p Gem::Platform.local.to_s'
ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [x86_64-linux-gnu]
"3.3.21"
"x86_64-linux-gnu"

$ ruby -ve 'p Gem::VERSION; p Gem::Platform.local.to_s'
ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [x86_64-linux-gnu]
"3.3.24"
"x86_64-linux-gnu"
```

As you can see, it now has the `-gnu` prefix and `Libv8::Node::Paths.object_paths` thus has it when the built binary actually lives in the directory `x86_64-linux`, breaking the build of
the gems like mini_racer on those platforms.

[`RUBY_TARGET_PLATFORM` is set to `*-linux`](https://github.com/knu/libv8-node/blob/ad9105562185571f7b486f7770986f6a160318b2/libexec/platform#L28-L29), so `Libv8::Node::Paths.platform` needs to align with that.

Ported forward by @MaZderMind
@cschroed
Copy link

Jumping in here as this is now blocking my deployment/upgrade plan to move to Ruby 3.2. I see that updates are being made to the repo, which is awesome!! However the PR this is based on has been sitting there for over two years. Sorry if that across as judgy, I don't mean it too.

Just trying to understand if this can be merged at the next opportunity or do I need to figure out how to fork this and build it myself so that I can continue to move forward.

@lloeki
Copy link
Collaborator

lloeki commented Jun 23, 2024

Ubuntu 24.04 is the oddball here (not that it's incorrect to have the -gnu suffix, it's actually more accurate and matches compiler triplets):

$ docker run --rm -it ubuntu:24.04 bash -c "apt update; apt install -y ruby; ruby -e 'pp ruby_description: RUBY_DESCRIPTION, rubygems_version: Gem::VERSION, ruby_platform: RUBY_PLATFORM, gem_platform_local: Gem::Platform.local.to_s'"
{:ruby_description=>"ruby 3.2.3 (2024-01-18 revision 52bb2ac0a6) [x86_64-linux-gnu]",
 :rubygems_version=>"3.4.20",
 :ruby_platform=>"x86_64-linux-gnu",
 :gem_platform_local=>"x86_64-linux-gnu"}

Unfortunately Debian Bookworm is a bit lagging and its rubygems cannot be update --system because of being apt-managed, but it will presumably behave the same as above once it bumps rubygems:

$ docker run --rm -it debian:12 bash -c "apt update; apt install -y ruby; ruby -e 'pp ruby_description: RUBY_DESCRIPTION, rubygems_version: Gem::VERSION, ruby_platform: RUBY_PLATFORM, gem_platform_local: Gem::Platform.local.to_s'"
{:ruby_description=>"ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux-gnu]",
 :rubygems_version=>"3.3.15",
 :ruby_platform=>"x86_64-linux-gnu",
 :gem_platform_local=>"x86_64-linux"}

EDIT: Heh, actually rubygems is bumped on Sid yet it still gives a non--gnu-suffixed parsed result even though RUBY_PLATFORM has it.

$ docker run --rm -it debian:sid bash -c "apt update; apt install -y ruby; ruby -e 'pp ruby_description: RUBY_DESCRIPTION, rubygems_version: Gem::VERSION, ruby_platform: RUBY_PLATFORM, gem_platform_local: Gem::Platform.local.to_s'"
{:ruby_description=>"ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux-gnu]",
 :rubygems_version=>"3.4.20",
 :ruby_platform=>"x86_64-linux-gnu",
 :gem_platform_local=>"x86_64-linux"}

Interestingly enough, Docker Hub Ruby images which are Debian Bookworm based have a non--gnu-suffixed RUBY_PLATFORM altogether:

$ docker run --rm -it ruby:3.3-bookworm bash -c "ruby -e 'pp ruby_description: RUBY_DESCRIPTION, rubygems_version: Gem::VERSION, ruby_platform: RUBY_PLATFORM, gem_platform_local: Gem::Platform.local.to_s'"
{:ruby_description=>"ruby 3.3.3 (2024-06-12 revision f1c7b6f435) [x86_64-linux]",
 :rubygems_version=>"3.5.11",
 :ruby_platform=>"x86_64-linux",
 :gem_platform_local=>"x86_64-linux"}

Similarly the ruby-lang.org official images are Ubuntu-based but do not have the -gnu suffix either:

$ docker run --rm -it rubylang/ruby:3.3-jammy ruby -e 'pp ruby_description: RUBY_DESCRIPTION, rubygems_version: Gem::VERSION, ruby_platform: RUBY_PLATFORM, gem_platform_local: Gem::Platform.local.to_s'
{:ruby_description=>"ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]",
 :rubygems_version=>"3.5.9",
 :ruby_platform=>"x86_64-linux",
 :gem_platform_local=>"x86_64-linux"}

ArchLinux has no -gnu suffix either:

$ docker run --rm -it archlinux bash -c "pacman --sync --refresh --noconfirm ruby; ruby -e 'pp ruby_description: RUBY_DESCRIPTION, rubygems_version: Gem::VERSION, ruby_platform: RUBY_PLATFORM, gem_platform_local: Gem::Platform.local.to_s'"
{:ruby_description=>
  "ruby 3.0.6p216 (2023-03-30 revision 23a532679b) [x86_64-linux]",
 :rubygems_version=>"3.3.25",
 :ruby_platform=>"x86_64-linux",
 :gem_platform_local=>"x86_64-linux"}

NixOS has no -gnu suffix either:

$ docker run --rm -it ghcr.io/nixos/nix nix-shell -p ruby --run "ruby -e 'pp ruby_description: RUBY_DESCRIPTION, rubygems_version: Gem::VERSION, ruby_platform: RUBY_PLATFORM, gem_platform_local: Gem::Platform.local.to_s'"
{:ruby_description=>"ruby 3.1.5p252 (2024-04-23 revision 1945f8dc0e) [x86_64-linux]",
 :rubygems_version=>"3.5.9",
 :ruby_platform=>"x86_64-linux",
 :gem_platform_local=>"x86_64-linux"}

has been sitting there for over two years. Sorry if that across as judgy, I don't mean it too.

"has been sitting there" stings a little :( but I understand the frustration.

It may look like a simple one line change but this change has rippling consequences on small but important details† so that it really works everywhere on every combination. And then it's a bit painful to build and publish all binary artifacts.

† Some time ago I started a refactoring of the build process to move away from shell to Ruby in order to be much more DRY.

do I need to figure out how to fork this and build it myself

If you want to go this route to immediately unblock you:

  • fork + patch your fork as you see fit in some branch, or not and use this PR's branch
  • in Gemfile: gem 'libv8-node', git: 'github.com/foo/bar.git', branch: 'baz' (or github: ... if you prefer)

And it'll build from source.

But I'll try to address it in some way in the coming weeks.

@cschroed
Copy link

@lloeki , first and foremost, thank you for your quick reply, and can certainly wait a few weeks if that is what it takes to get this merged into the main branch and released. It seems to me that the disconnect is that the libv8 library produces a x86_64-linux named lib, yet the platform has changed. So regardless of who is "correct", the libv8 needs to produce a binary with x86_64-linux-gnu or deal with the changes to x86_64-linux like it does for darwin who appends the version number to it.

Maybe I'm being too simple here and missing what you speak about regarding this change has rippling consequences on small but important details† so that it really works everywhere on every combination however, one or the other needs to happen, correct?

Regardless, if this can be fixed with the published library, that is awesome and will save me time from setting up local environments to do the build.

@lloeki
Copy link
Collaborator

lloeki commented Jun 25, 2024

can certainly wait a few weeks

I'll carve out some time to have a proper fix.

Regardless, if this can be fixed with the published library, that is awesome and will save me time from setting up local environments to do the build.

Of course, I am very mindful of that.

tomhughes added a commit to Firefishy/chef that referenced this pull request Aug 30, 2024
tomhughes added a commit to Firefishy/chef that referenced this pull request Aug 30, 2024
@cschroed
Copy link

@lloeki coming back around and checking on this as my previous project is done and would like to pick this work back up. I see some activity on other PRs, so is this resolved or still needs to be worked on?

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

8 participants