launchdarkly / ruby-server-sdk

LaunchDarkly Server-side SDK for Ruby
https://docs.launchdarkly.com/sdk/server-side/ruby
Other
34 stars 52 forks source link

Building error when added as a dependency #155

Closed camillof closed 4 years ago

camillof commented 4 years ago

Describe the bug When adding this gem as a dependency to another gem, causes the gem to raise an error when building it.

To reproduce

  1. Add launchdarkly-server-sdk as a dependency to your gem
    spec.add_dependency "launchdarkly-server-sdk", "~> 5.7"
  2. Try to build your gem

Expected behavior I expect my gem to be built without errors.

Logs

current directory: /Users/camillo/Desktop/WyeProjects/TRR/dfs-scripts/dist/DFS/gems/launchdarkly-server-sdk-5.6.2/ext
/Users/camillo/.rvm/rubies/ruby-2.6.3/bin/ruby mkrf_conf.rb

current directory: /Users/camillo/Desktop/WyeProjects/TRR/dfs-scripts/dist/DFS/gems/launchdarkly-server-sdk-5.6.2/ext
rake RUBYARCHDIR\=/Users/camillo/Desktop/WyeProjects/TRR/dfs-scripts/dist/DFS/extensions/x86_64-darwin-18/2.6.0/launchdarkly-server-sdk-5.6.2 RUBYLIBDIR\=/Users/camillo/Desktop/WyeProjects/TRR/dfs-scripts/dist/DFS/extensions/x86_64-darwin-18/2.6.0/launchdarkly-server-sdk-5.6.2
/Users/camillo/.rvm/rubies/ruby-2.6.3/lib/ruby/site_ruby/2.6.0/bundler/rubygems_integration.rb:462:in `block in replace_bin_path': can't find executable rake for gem rake. rake is not currently included in the bundle, perhaps you meant to add it to your Gemfile? (Gem::Exception)
    from /Users/camillo/.rvm/rubies/ruby-2.6.3/lib/ruby/site_ruby/2.6.0/bundler/rubygems_integration.rb:482:in `block in replace_bin_path'
    from /Users/camillo/.rvm/gems/ruby-2.6.3/bin/rake:23:in `<main>'
    from /Users/camillo/.rvm/gems/ruby-2.6.3/bin/ruby_executable_hooks:24:in `eval'
    from /Users/camillo/.rvm/gems/ruby-2.6.3/bin/ruby_executable_hooks:24:in `<main>'

rake failed, exit code 1

SDK version 5.7.2

Language version, developer tools ruby-2.6.3 [ x86_64 ]

OS/platform MacOS Catalina 10.15

Additional context This problem seems to be solved if you add rake as a dependency into your gem too, but shouldn't this be a ruby-server-sdk dependency instead?

eli-darkly commented 4 years ago

That's odd; I'm not sure why it would expect rake to be present at all. Our SDK did previously have a rake dependency but it was removed since we no longer use Rake in the build. We'll look into this.

camillof commented 4 years ago

I think it’s because it’s used to build the gem. I saw that PR removing the rake dependency and tried with an older version, the result was the same, probably because it was a development_dependency only

On Thu, Apr 23, 2020 at 20:21 Eli Bishop notifications@github.com wrote:

That's odd; I'm not sure why it would expect rake to be present at all. Our SDK did previously have a rake dependency but it was removed since we no longer use Rake in the build. We'll look into this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/launchdarkly/ruby-server-sdk/issues/155#issuecomment-618720007, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMP53CNYB3RHAN5I4CF5UBTRODEPXANCNFSM4MPNGSXA .

eli-darkly commented 4 years ago

Sorry if I'm misunderstanding something basic, but if Rake is required to build your own gem, then why would you need it to be provided as a transitive dependency by the Ruby SDK? Our gem is already built.

camillof commented 4 years ago

No, rake is not required to build my own gem. I have been building my gem without it for a while.

But, if I add the Ruby SDK as a dependency, I couldn't find another way to get it working than adding also rake as a dependency to my gem too, which is not the expected behavior.

camillof commented 4 years ago

Seems like you have an implicit rake dependency here when using

spec.extensions    = 'ext/mkrf_conf.rb'

As it uses rake when building it https://github.com/launchdarkly/ruby-server-sdk/blob/master/launchdarkly-server-sdk.gemspec#L22

eli-darkly commented 4 years ago

Ahh, thanks, that's definitely it.

eli-darkly commented 4 years ago

Well... hmm. I thought I understood the issue, but I am having trouble reproducing it.

In an environment where rake is not installed, I created a minimal gemspec that included spec.add_dependency "launchdarkly-server-sdk", "5.7.2". I then did a gem build. It worked fine.

Since the purpose of the logic in ext/mkrf_conf.rb (which I don't think we really need) was to check for the presence of openssl... does your Ruby installation include openssl, either compiled in or as a gem? Mine does, and it would take a bit longer to set up one that doesn't. I'm not sure what other differences there might be between my test case and yours.

If you could provide an example of your gemspec with any sensitive info removed, and the build command you're using, that might help.

camillof commented 4 years ago

Hi, I created a dummy gem to demostrate this error, you can clone it from here: https://github.com/camillof/example_gem

Thank you for your time!

eli-darkly commented 4 years ago

Thanks - I was able to reproduce a failure with that code. I'm not sure what the crucial difference was between that and my other test code, but this should work to test the fix.

I think the fix we will go with is just to completely remove the logic that was in ext/mkrf_conf.rb. Its purpose was to prevent installing the gem in an environment that doesn't have openssl, but that would now be a very rare condition since openssl is bundled in all modern Ruby versions/

camillof commented 4 years ago

Yes, it's weird, because if I try to bundle and install the gem from the terminal it works fine, but not through the create_gem script, maybe some dependency is not loading?

I look forward to your fix results 🙌

eli-darkly commented 4 years ago

@camillof I have a change that as far as I can tell resolves the issue, but I'd like to be double sure, so if you have a chance: could you please try your build again with the LaunchDarkly SDK version changed to "5.7.3.pre.beta.2"? That's a prerelease version that I've pushed containing the change.

camillof commented 4 years ago

🚀 it worked! Thank you very much!

PS: I'm now having an issue with a dependency, ld-eventsource, which also depends on socketry which is a bit outdated, using hitimes 1.3.1 which had a major bump to 2.0 fixing a loading paths issue But that's for another ticket in another repo, so thank you for helping me with this one.

eli-darkly commented 4 years ago

Hmm, well, if the ld-eventsource dependency issue is straightforward then we could try to get it into the same upcoming patch release of the SDK, but I'm not sure that it is. We do not have a good alternative to using socketry yet as far as I know. I'm unclear on what the actual problem is that you're running into.

camillof commented 4 years ago
/usr/local/DFS/gems/hitimes-1.3.1/lib/hitimes.rb:56:in `<top (required)>': Unable to find binary extension, was hitimes installed correctly? The following paths were tried. (LoadError)
/Users/camillo/hitimes/2.6/hitimes : incompatible library version - /usr/local/DFS/gems/hitimes-1.3.1/lib/hitimes/2.6/hitimes.bundle
/Users/camillo/hitimes/hitimes : cannot load such file -- hitimes/hitimes
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /usr/local/DFS/gems/socketry-0.5.1/lib/socketry.rb:10:in `<top (required)>'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /usr/local/DFS/gems/ld-eventsource-1.0.3/lib/ld-eventsource/impl/streaming_http.rb:5:in `<top (required)>'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /usr/local/DFS/gems/ld-eventsource-1.0.3/lib/ld-eventsource/client.rb:3:in `<top (required)>'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /usr/local/DFS/gems/ld-eventsource-1.0.3/lib/ld-eventsource.rb:1:in `<top (required)>'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /usr/local/DFS/gems/launchdarkly-server-sdk-5.7.3.pre.beta.2/lib/ldclient-rb/stream.rb:3:in `<top (required)>'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /usr/local/DFS/gems/launchdarkly-server-sdk-5.7.3.pre.beta.2/lib/ldclient-rb.rb:20:in `<top (required)>'

This is the error I'm getting when requiring the ldclient-rb (I can't even require hitimes directly in my code if gem version is < 2.0). It's solved if I replace all the code inside hitimes installed library with the latest one. But yeah, it is not that straightforward, as socketry is locked on hitimes ~> 1.2.

This issue may provide further information: https://github.com/socketry/socketry/issues/29

eli-darkly commented 4 years ago

That's unfortunate. We may need to fork it. IIRC the only reason we use Socketry is that we need to be able to enforce read timeouts on a streaming connection, but that is a crucial requirement.

eli-darkly commented 4 years ago

The original issue (not including the socketry/hitimes issue) is fixed in the 5.7.3 release, and I'm deleting the prerelease test version. Thanks for catching this.

eli-darkly commented 4 years ago

@camillof I've filed the hitimes issue here and I'll also open an issue in this repo pointing to that, so if anyone else runs into it they'll see that it's known.

Out of curiosity, are you using TruffleRuby at all (which was mentioned in the hitimes release notes regarding that loading problem), or what is your runtime environment? I know that in your original issue description you just said Ruby 2.6.3, but I presume it is not just a vanilla 2.6.3 distribution because we test with that all the time.

camillof commented 4 years ago

Thanks a lot @eli-darkly 🙌, I'll keep an eye on that issue.

We're not using TruffleRuby at all, our environment is a Mac OS pkg that is delivered to our users, and inside that pkg we embed our gem (which has dependencies on other gems), that's why our gem is built and installed every time we create a new pkg. In that build was when we got the original rake error. Now we are getting this hitimes error at runtime level, after the user install the pkg and try to run our gem. Does that answer your question?

In the meantime, we are going to use the LaunchDarkly REST API.

eli-darkly commented 4 years ago

We're not using TruffleRuby at all, our environment is a Mac OS pkg that is delivered to our users, and inside that pkg we embed our gem (which has dependencies on other gems), that's why our gem is built and installed every time we create a new pkg. In that build was when we got the original rake error. Now we are getting this hitimes error at runtime level, after the user install the pkg and try to run our gem. Does that answer your question?

Well, it's mysterious, but it does tell me that the issue isn't limited to any particular runtimes as what I saw in the hitimes repo seemed to suggest. I wish I knew how to reproduce it, but if we can either update or get rid of the hitimes dependency I presume that will fix it.

our environment is a Mac OS pkg that is delivered to our users, and inside that pkg we embed our gem (which has dependencies on other gems), that's why our gem is built and installed every time we create a new pkg

I'm glad you mentioned this since I wasn't sure about the intended deployment. If this is code that's going to run on an individual user's machine, we would highly recommend not using any of the server-side SDKs, for reasons that are discussed here. Since there isn't a Ruby client-side SDK at present, directly calling a REST endpoint— one of the ones used by client-side SDKs, that evaluates the flags for you rather than returning the flag configurations— would be the way to go. That is not part of the REST AP that is documented here— I'll find you the relevant docs.

camillof commented 4 years ago

I'm glad you mentioned this since I wasn't sure about the intended deployment. If this is code that's going to run on an individual user's machine, we would highly recommend not using any of the server-side SDKs, for reasons that are discussed here. Since there isn't a Ruby client-side SDK at present, directly calling a REST endpoint— one of the ones used by client-side SDKs, that evaluates the flags for you rather than returning the flag configurations— would be the way to go. That is not part of the REST AP that is documented here— I'll find you the relevant docs.

Wow, we really missed that point. It would be really helpful if you find that "client rest endpoint" documentation. I'm trying to figure it out from the javascript SDK repo but it is not that easy.