-
Notifications
You must be signed in to change notification settings - Fork 416
Builds two gems from one repo. #205
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
Conversation
This PR resumes the discussion begin in PR #197. |
To load the whole gem I think Edit: I would not call what rails does a "convention" since they are the only project to do that ;) |
I'm pretty sure that everyone in this community understands why I'm so cautious about C/Java extension, but this protobuf merge from Google underscores the point. |
562ae6d
to
9caecc5
Compare
The latest commits fix broken tests and update the README. Thoughts? |
d8fcfd3
to
fef743b
Compare
the issue you linked was an interesting read indeed xD |
We're building a ruby app, deployed on Heroku, and concurrent-ruby is one of the many gems we use. My understanding of these gem building and deployment issues is shallow, but if I understand the bottom line correctly, under this proposal we'll be able to gain the C-level performance on Heroku, with the tiny cost of having to refer to an additional gem. I'm completely satisfied with that result. I also like the idea mentioned earlier that a message could be emitted alerting people to the option of using that extension gem for increased performance. |
@hsm3 You are absolutely correct. To deploy on Heroku and have the C extension you would simply install Once we merge this PR I'll create pre-release gems and test specifically on Heroku. We'll make sure Heroku works as expected before a final release. |
386115d
to
54c4d2a
Compare
@jdantonio Commenting on:
This could be problematic if |
The current plan:
|
@pitr-ch The original plan was to ask users who wanted to install the extensions to explicitly install both gems. The problem was that if a user were to install just I'll experiment with all the builds and if there are problems I'll remove the runtime dependency from the extension gem. |
@jdantonio aha that makes sense. It should work with the run-time dependency as long as it's also a gem dependency which I think it should. |
@pitr-ch |
A PR has been created to update the automated build process to support this PR and the two companion gems. |
@jdantonio I'll have time on weekend to do some testing. |
@pitr-ch I've created all the gem packages for the first pre-release and have been testing them on various computers. I should be able to upload them tonight. |
I've created a new pre-release and uploaded both gems to Rubygems (here and here). I've started writing tests that test the built gem packages so we can verify that everything works correctly on all platforms. I'm still working on those. I've tested the pre-release gems on several platforms and I think we may have things working the way we discussed. Any help testing the built packages will be greatly appreciated. |
s.description = <<-EOF | ||
Modern concurrency tools including agents, futures, promises, thread pools, actors, supervisors, and more. | ||
Inspired by Erlang, Clojure, Go, JavaScript, actors, and classic concurrency patterns. | ||
EOF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Purpose of this gem should also be explained in description. It's the main text used on https://rubygems.org/gems/concurrent-ruby-ext
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I began by copying the original gemspec file then started making updates. I never thought to change the description. I'll update this before the next release.
58447e1
to
3877635
Compare
Builds two gems from one repo.
hi @jdantonio, what was the reason behind dropping pre-compiled gems for unix-like systems? |
@pitr-ch My builds were broken. I decided it was better to release 0.8 without them and figure it out later rather than hold off the release. My professional experience with C and C++ is several years old and was strictly with Visual C++ and the full MS stack. I have virtually no experience building on Linux. One of our users reported that the pre-compiled extensions weren't working on Heroku. The way the error messages worked on 0.7.0 I couldn't determine the root of the problem. It could have been that Heroku was downloading the pure-Ruby gem or it could have been a problem with the extension gem itself. Under 0.8 I changed the way the error messages worked and created a project specifically for testing on Heroku (https://github.com/jdantonio/heroku-tester) and determined it was the latter: Heroku was correctly installing the pre-compiled extension but Ruby was raising an exception when it attempted to load the .so module. It was raising a The most frustrating thing about this is that the pre-compiled extension gem worked fine on the Ubuntu VM I used to build the gems! Since I have little experience building C on Linux I wasn't able to figure out what the problem was. I assume it has something to do with the version of Ruby or the version of Ubuntu I used for building but I honestly don't know. I've left all the build commands for Linux in the build scripts but commented them out. Once I can figure out what's going on I would be happy to start releasing the pre-compiled Linux binaries, but I need to make sure they work consistently first. |
@jdantonio Thanks for the detailed explanation! |
It may have nothing to do with this but on linux/unix the library will link against system libraries if they are needed as dependencies, if you compile it on a system with a specific version of said dependency and then try to run it on another one with different version it may not work. If you just run ldd on a binary / shared library the system will tell you what are the dependencies, something like this:
|
This update will now build two gems from within one repo.
Building
Commands to build the gems:
rake build:core
under MRI or Rbx will build the pure-Rubyconcurrent-ruby
gemrake build:core
under JRuby will be the Java-specificconcurrent-ruby
gemrake build:ext
under MRI will build theconcurrent-ruby-ext
gem with uncompiled C extensionsrake build:native
under MRI will build theconcurrent-ruby-ext
gem with pre-compiled C extensions for the current platformrake build
under MRI runs bothrake build:core
andrake build:ext
rake build
is an alias forrake build:core
under JRuby or RbxInstalling
The
concurrent-ruby-ext
gem listsconcurrent-ruby
as a runtime dependency. Installingconcurrent-ruby-ext
will causeconcurrent-ruby
to be installed.Installing
concurrent-ruby
alone will install the JRuby-specific gem under JRuby. It will install the pure-Ruby gem under any other platform.Using
All the user ever needs to do is
require 'concurrent'
(or a load a subset of the gem, as inrequire 'concurrent/atomic'
orrequire 'concurrent/atomics'
) and the C extensions will load if an only if running under MRI and theconcurrent-ruby-ext
gem has been installed.Testing
The scripts in the
build-tests
directory will test the various gem packages built with the aforementioned rake tasks. Therunner.rb
script consolidates all the individual tests against the relevant builds. The individual*.gem
files need to be in thepkg
directory and therunner.rb
script needs run from a directory other than the root directory of this repo. The approach I normally use is:ruby-concurrency/rake-compiler-dev-box
repositorytest-concurrent-ruby.sh
script from within theruby-concurrency/rake-compiler-dev-box
repositoryThis will not test the Windows builds. The
ruby-concurrency/rake-compiler-dev-box/tests/windows-test.cmd
file will do that, but it requires a Windows machine and manual setup of various Ruby versions.