search

danielpclark / faster_path

Faster Pathname handling for Ruby written in Rust
MIT License
782 stars 33 forks source link

Test & Verify Binary Releases #143

Closed danielpclark closed 6 years ago

danielpclark commented 6 years ago

As of FasterPath v0.3.6 we now have binary releases compiled and deployed to Github. These need to be tested across many Ruby versions and operating systems to establish their reliability.

v0.3.7 Systems Tried

❌ Mac & Ruby 2.5.0

These tests need to be conducted from a dependency stand point, not from downloading from this repository.

To test these you need to switch to a specific Ruby version, do a gem install, try it out in IRB and experiment with a few methods to see if it will segfault. Please try at least 3 methods a few different ways with string parameters. The gem install should retrieve the binary release and you won't need to compile the asset.

Some issues we may need to address:

v0.3.9 Systems Tried

Since we know it worked on previous builds we really want to focus on Mac & Ruby 2.5.0.

danielpclark commented 6 years ago

Okay, Linux works! :smile: :tada: I just need a Mac user to help now. Someone who doesn't have Rust installed.

Do the following:

gem install faster_path
irb
> require 'faster_path'
> FasterPath.join('a', 'b')

That should be enough per Ruby version.

PikachuEXE commented 6 years ago

Seem failed on Mac with 2.5

MacOS 10.12.6 RVM master ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin16]

2.5.0 :001 > require 'faster_path'
Traceback (most recent call last):
       10: from /users/pikachuexe/.rvm/rubies/ruby-2.5.0/bin/irb:11:in `<main>'
        9: from (irb):1
        8: from /users/pikachuexe/.rvm/rubies/ruby-2.5.0/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:39:in `require'
        7: from /users/pikachuexe/.rvm/rubies/ruby-2.5.0/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:135:in `rescue in require'
        6: from /users/pikachuexe/.rvm/rubies/ruby-2.5.0/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:135:in `require'
        5: from /users/pikachuexe/.rvm/gems/ruby-2.5.0/gems/faster_path-0.3.7/lib/faster_path.rb:8:in `<top (required)>'
        4: from /users/pikachuexe/.rvm/gems/ruby-2.5.0/gems/faster_path-0.3.7/lib/faster_path.rb:17:in `<module:FasterPath>'
        3: from /users/pikachuexe/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/fiddle.rb:47:in `dlopen'
        2: from /users/pikachuexe/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/fiddle.rb:47:in `new'
        1: from /users/pikachuexe/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/fiddle.rb:47:in `initialize'
Fiddle::DLError (dlopen(/users/pikachuexe/.rvm/gems/ruby-2.5.0/gems/faster_path-0.3.7/lib/faster_path.so, 9): Library not loaded: /Users/travis/.rvm/rubies/ruby-2.5.0/lib/libruby.2.5.dylib
  Referenced from: /users/pikachuexe/.rvm/gems/ruby-2.5.0/gems/faster_path-0.3.7/lib/faster_path.so
  Reason: image not found)

But these versions got no issue:

Will edit and add results for other versions later (installing)

Edit 1: Update list of ruby versions with no issues

danielpclark commented 6 years ago

Thanks! @PikachuEXE

Seem failed on Mac with 2.5

That could be a Thermite issue, or an environment variable changing the type of library extension. Only .so extensions have been built but the error you've shown tried to load a .dylib. Or it could even simply be a different requirement by the OS/Ruby setup.

For the Ruby 2.5 can you check this in irb? RbConfig::CONFIG['DLEXT']

I believe I can enforce both Linux & Mac to use the .so extension. That would take another samll monkey-patch. But that makes sense since TravisCI is only generating .so files for both Mac and Linux.

PikachuEXE commented 6 years ago

These versions:

danielpclark commented 6 years ago

Thanks. It's odd that the problem only arose in 2.5. Thermite has a method to set all "bundle" as "dylib". lib/thermite/config.rb#L49-L59

PikachuEXE commented 6 years ago

I noticed it's trying to load /Users/travis/.rvm/rubies/ruby-2.5.0/lib/libruby.2.5.dylib not /users/pikachuexe/.rvm/rubies/ruby-2.5.0/lib/libruby.2.5.dylib

Otherwise the file exists

danielpclark commented 6 years ago

Ah. Thanks for the info. Would you feel comfortable sharing this info in an issue on Thermite?

PikachuEXE commented 6 years ago

Should I open a new issue on Thermite? If not just copy it :P

danielpclark commented 6 years ago

Yeah I think he should know his Thermite gem sometimes tries to load a travis user on a Mac. You can reference this issue for him to look back to as well.

PikachuEXE commented 6 years ago

Issue created

danielpclark commented 6 years ago

Thank you! You've been a great help. :tada:

PikachuEXE commented 6 years ago

I reviewed the log for v0.3.7 build on Travis It seems the only differences between different ruby versions in log are:

Links to Travis build logs

I wonder what happens if bundler and rubygems are updated before build

danielpclark commented 6 years ago

@andrewstucki Looking at the research you've done on https://github.com/steveklabnik/ruby-sys/pull/25 would applying your patch to my forks for both ruby-sys and ruru perhaps allow the Mac & Ruby 2.5.0 version to work on this issue?

I'm also curious as to whether this would allow me to not need to copy Ruby's lib to my Rust lib build directory. In ruru I put it in the .travis.yml file but for FasterPath I have two rake commands task :libruby_release and task :libruby_debug.

andrewstucki commented 6 years ago

@danielpclark So from the looks of the stack trace above it's just an issue of not being able to find libruby? If so, yeah, the patch should help you out since libruby shouldn't actually be needed at runtime (it's basically a fat library with like 90% of the Ruby internals in it that are already contained in the interpreter and dynamically used by extensions when they're loaded in memory).

Regarding the second question, yep, that should be the case, for the exact reasons above. Looking at how this library does appear to have some Rust tests that run outside of the context of the Ruby interpreter though, you'll need to do something like the re-export testing features that I added in https://github.com/steveklabnik/ruby-sys/pull/31 and https://github.com/d-unseductable/ruru/pull/95 since Rust trying to execute the tests won't actually have access to the required symbols without linking in libruby.

In other words, run the Rust tests with that --features test flag, but build and ship without it and you should be good to go. Let me know if you decide to go ahead with that and if you run into any problems.

danielpclark commented 6 years ago

The changes you've recommended @andrewstucki have been applied. The good news is I don't need --features test to make it work for either Ruby or Rust and now I don't need to copy the libruby file to the dependencies build directory anymore. So that's awesome!

If Mac & Ruby 2.5.0 doesn't work after this then the issue is Thermite specific for sure. I have a slight suspicion that the path to the lib being built may not be related to this fix. In which case Mac & Ruby 2.5.0 still won't work.

andrewstucki commented 6 years ago

@danielpclark awesome, so it sounds like the Rust tests don't actually use any of ruru and that all of that is tested through Ruby itself?

If that's the case just be careful with adding additional Rust tests that call down into ruby-sys itself. If you get any issues in the future because of unresolved rb_ symbols it'll probably be because of this and, in that case, just do the whole re-export --features test business!

danielpclark commented 6 years ago

@danielpclark awesome, so it sounds like the Rust tests don't actually use any of ruru and that all of that is tested through Ruby itself?

That's not the case. This project specifically uses ruru for everything we depend on end product wise. It just works and I'm happy with that.

Just to clarify I did add the --features test to ruru like you did but did not need that on faster_path.

danielpclark commented 6 years ago

@PikachuEXE if you get a chance to try Ruby 2.5.0 again with this let me know how it goes. If it still doesn't work I'll close this off as an outside issue for Thermite. Thanks!

PikachuEXE commented 6 years ago

Now it works! (0.3.9)

Tried with 2.5.0, 2.4.3, 2.3.6

If there is anything else need to be tested let me know ;)

PikachuEXE commented 6 years ago

Fail when installing in a docker container

Env

root@557f37e012d6:/# ruby -v
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-linux]
root@557f37e012d6:/# gem -v
2.7.6
root@557f37e012d6:/# bundle -v
Bundler version 1.16.1

Error Log

root@557f37e012d6:/# gem install faster_path
Fetching: minitar-0.6.1.gem (100%)
The `minitar` executable is no longer bundled with `minitar`. If you are
expecting this executable, make sure you also install `minitar-cli`.
Successfully installed minitar-0.6.1
Fetching: tomlrb-1.2.6.gem (100%)
Successfully installed tomlrb-1.2.6
Fetching: thermite-0.13.0.gem (100%)
Successfully installed thermite-0.13.0
Fetching: faster_path-0.3.9.gem (100%)
Building native extensions. This could take a while...
ERROR:  Error installing faster_path:
    ERROR: Failed to build gem native extension.

    current directory: /usr/local/lib/ruby/gems/2.5.0/gems/faster_path-0.3.9/ext
/usr/local/bin/ruby -rrubygems /usr/local/lib/ruby/gems/2.5.0/gems/rake-12.3.0/exe/rake RUBYARCHDIR=/usr/local/lib/ruby/gems/2.5.0/extensions/x86_64-linux/2.5.0-static/faster_path-0.3.9 RUBYLIBDIR=/usr/local/lib/ruby/gems/2.5.0/extensions/x86_64-linux/2.5.0-static/faster_path-0.3.9
checking for cargo... no
rake aborted!
****
Rust's Cargo is required to build this extension. Please install
Rust and put it in the PATH, or set the CARGO environment variable appropriately.
****

Tasks: TOP => default => thermite:build
(See full trace by running task with --trace)
Downloading compiled version (0.0.1) from GitHub

rake failed, exit code 1

Gem files will remain installed in /usr/local/lib/ruby/gems/2.5.0/gems/faster_path-0.3.9 for inspection.
Results logged to /usr/local/lib/ruby/gems/2.5.0/extensions/x86_64-linux/2.5.0-static/faster_path-0.3.9/gem_make.out

/usr/local/lib/ruby/gems/2.5.0/extensions/x86_64-linux/2.5.0-static/faster_path-0.3.9/gem_make.out:

current directory: /usr/local/lib/ruby/gems/2.5.0/gems/faster_path-0.3.9/ext
/usr/local/bin/ruby -rrubygems /usr/local/lib/ruby/gems/2.5.0/gems/rake-12.3.0/exe/rake RUBYARCHDIR=/usr/local/lib/ruby/gems/2.5.0/extensions/x86_64-linux/2.5.0-static/faster_path-0.3.9 RUBYLIBDIR=/usr/local/lib/ruby/gems/2.5.0/extensions/x86_64-linux/2.5.0-static/faster_path-0.3.9
checking for cargo... no
rake aborted!
****
Rust's Cargo is required to build this extension. Please install
Rust and put it in the PATH, or set the CARGO environment variable appropriately.
****

Tasks: TOP => default => thermite:build
(See full trace by running task with --trace)
Downloading compiled version (0.0.1) from GitHub

rake failed, exit code 1
danielpclark commented 6 years ago

Which docker container did this happen for you in? All of the Ruby docker images I tried worked. Specifically fro Ruby 2.5 it was ruby:2.5.0.

~ #: docker run -i -t ruby:2.5.0 /bin/bash
root@d2be2ff4eb3b:/# gem install faster_path
Fetching: minitar-0.6.1.gem (100%)
The `minitar` executable is no longer bundled with `minitar`. If you are
expecting this executable, make sure you also install `minitar-cli`.
Successfully installed minitar-0.6.1
Fetching: tomlrb-1.2.6.gem (100%)
Successfully installed tomlrb-1.2.6
Fetching: thermite-0.13.0.gem (100%)
Successfully installed thermite-0.13.0
Fetching: faster_path-0.3.9.gem (100%)
Building native extensions. This could take a while...
Successfully installed faster_path-0.3.9
4 gems installed
root@d2be2ff4eb3b:/#

That could be an anomaly where the connection to Github Releases was unavailable. That's my best guess with the information we have here and now.