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

Builds two gems from one repo. #205

Merged
merged 25 commits into from Jan 25, 2015
Merged

Builds two gems from one repo. #205

merged 25 commits into from Jan 25, 2015

Conversation

jdantonio
Copy link
Member

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-Ruby concurrent-ruby gem
  • rake build:core under JRuby will be the Java-specific concurrent-ruby gem
  • rake build:ext under MRI will build the concurrent-ruby-ext gem with uncompiled C extensions
  • rake build:native under MRI will build the concurrent-ruby-ext gem with pre-compiled C extensions for the current platform
  • rake build under MRI runs both rake build:core and rake build:ext
  • rake build is an alias for rake build:core under JRuby or Rbx
  • The rake-compiler-dev-box scripts have not yet been updated

Installing

The concurrent-ruby-ext gem lists concurrent-ruby as a runtime dependency. Installing concurrent-ruby-ext will cause concurrent-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 in require 'concurrent/atomic' or require 'concurrent/atomics') and the C extensions will load if an only if running under MRI and the concurrent-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. The runner.rb script consolidates all the individual tests against the relevant builds. The individual *.gem files need to be in the pkg directory and the runner.rb script needs run from a directory other than the root directory of this repo. The approach I normally use is:

  1. Clone the ruby-concurrency/rake-compiler-dev-box repository
  2. Clone the ruby-concurrency/concurrent-ruby repository under the ruby-concurrency/rake-compiler-dev-box repository
  3. Use the build scripts to build the gem packages needing tested
  4. Run the test-concurrent-ruby.sh script from within the ruby-concurrency/rake-compiler-dev-box repository

This 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.

@jdantonio
Copy link
Member Author

This PR resumes the discussion begin in PR #197.

@schmurfy
Copy link

To load the whole gem I think require 'concurrent' is fine, when we want a specific piece we can do require 'concurrent/atomic', activesupport is a different beast where you would rarely if ever need everything outside of rails. For concurrent-ruby I consider it the other way: the standard use case is to require everything and sometimes you might want to use only specific pieces, what do you think ?

Edit: I would not call what rails does a "convention" since they are the only project to do that ;)

@jdantonio
Copy link
Member Author

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.

@jdantonio jdantonio changed the title Builds two gems from one repo. Specs and docs need updated. Builds two gems from one repo. Dec 11, 2014
@jdantonio
Copy link
Member Author

The latest commits fix broken tests and update the README.

Thoughts?

@schmurfy
Copy link

the issue you linked was an interesting read indeed xD
I think the split gems is a good solution.

@hsm3
Copy link

hsm3 commented Dec 12, 2014

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.

@jdantonio
Copy link
Member Author

@hsm3 You are absolutely correct. To deploy on Heroku and have the C extension you would simply install concurrent-ruby-ext instead of concurrent-ruby (or list it in your Gemfile if you ), but the require code would be unchanged.

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.

@jdantonio jdantonio force-pushed the refactor/two-gems-one-repo branch 2 times, most recently from 386115d to 54c4d2a Compare December 13, 2014 14:15
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) when pulling 54c4d2a on refactor/two-gems-one-repo into 204e770 on master.

@pitr-ch
Copy link
Member

pitr-ch commented Dec 13, 2014

@jdantonio Commenting on:

simply install concurrent-ruby-ext instead of concurrent-ruby

This could be problematic if concurrent-ruby is indirect dependency, e.g. app -> a gem -> concurrent-ruby then it will keep installing concurrent-ruby. concurrent-ruby-ext can be only added by app. I thought this was the intention to have concurrent-ruby-ext act as a addition not replacement, but maybe I got that wrong.

@jdantonio
Copy link
Member Author

The current plan:

  • Update our build automation to build the new gems
  • Update refactor/two-gems-one-repo branch from master
  • Create pre-release builds (0.8.0.pre1 core, 0.1.0.pre1 ext) for both gems from refactor/two-gems-one-repo branch
  • Publish both pre-release gems (0.8.0.pre1 core, 0.1.0.pre1 ext) to Rubygems
  • Test gems locally on all platforms
    • MRI OS X (both gems)
    • JRuby OS X
    • Rbx OS X
    • MRI Windows 32 (both gems)
    • MRI Windows 64 (both gems)
    • MRI Ubuntu 32 (both gems + compile on install)
    • MRI Ubuntu 64 (both gems + compile on install)
    • Solaris (both gems)
  • Test gems (0.8.0.pre1 core, 0.1.0.pre1 ext) on Heroku
    • MRI (both gems + compile on install)
    • JRuby
    • Rbx
  • Merge Adds counting semaphores #206 Concurrent:: Semaphore
  • Create a second pre-release where the core and extension gems have the same version number (0.8.0.pre2)
  • Publish 0.8.0.pre2 pre-release gems to Rubygems
  • Test 0.8.0.pre2 gems locally on all platforms
    • MRI OS X (both gems)
    • JRuby OS X
    • Rbx OS X
    • MRI Windows 32 (both gems)
    • MRI Windows 64 (both gems)
    • MRI Ubuntu 32 (both gems + compile on install)
    • MRI Ubuntu 64 (both gems + compile on install)
    • Solaris (both gems)
  • Test 0.8.0.pre2 gems on Heroku
    • MRI (both gems + compile on install)
    • JRuby
    • Rbx
  • Release v0.7.2 with all updates except this branch
  • Merge refactor/two-gems-one-repo into master
  • Create and publish a new set of pre-release gems if there have been additional changes on master

@jdantonio
Copy link
Member Author

@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 concurrent-ruby-ext the app would not work. Somewhere during the discussion it was suggested that installing concurrent-ruby-ext could force the installation of concurrent-ruby, thus avoiding that edge case.

I'll experiment with all the builds and if there are problems I'll remove the runtime dependency from the extension gem.

@pitr-ch
Copy link
Member

pitr-ch commented Dec 13, 2014

@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.

@jdantonio
Copy link
Member Author

@pitr-ch concurrent-ruby is listed as a runtime dependency in the gemspec for concurrent-ruby-ext

@jdantonio
Copy link
Member Author

A PR has been created to update the automated build process to support this PR and the two companion gems.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) when pulling 4237046 on refactor/two-gems-one-repo into 204e770 on master.

@pitr-ch
Copy link
Member

pitr-ch commented Dec 17, 2014

@jdantonio I'll have time on weekend to do some testing.

@jdantonio
Copy link
Member Author

@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.

@jdantonio
Copy link
Member Author

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
Copy link
Member

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

Copy link
Member Author

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.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 91.68% when pulling 3877635 on refactor/two-gems-one-repo into 4b1ac2f on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 91.68% when pulling 1795faa on refactor/two-gems-one-repo into 4b1ac2f on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 91.68% when pulling 12ba3a8 on refactor/two-gems-one-repo into 4b1ac2f on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 91.76% when pulling 6a17ca0 on refactor/two-gems-one-repo into 4b1ac2f on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 91.7% when pulling 372a68f on refactor/two-gems-one-repo into 4b1ac2f on master.

jdantonio added a commit that referenced this pull request Jan 25, 2015
@jdantonio jdantonio merged commit 474bdf6 into master Jan 25, 2015
@jdantonio jdantonio deleted the refactor/two-gems-one-repo branch January 26, 2015 00:32
@pitr-ch
Copy link
Member

pitr-ch commented Jan 27, 2015

hi @jdantonio, what was the reason behind dropping pre-compiled gems for unix-like systems?

@jdantonio
Copy link
Member Author

@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 LoadError exception and the error message stated that the .so file did not exists. I was able to run some shell commands against Heroku and determine that the file was in the correct place and had the right permissions. My best guess is that the C code was crashing on load and Ruby was giving a bad error message.

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.

@pitr-ch
Copy link
Member

pitr-ch commented Jan 28, 2015

@jdantonio Thanks for the detailed explanation!

@schmurfy
Copy link

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.
The others obvious problems are different architecture and 32/64 bits but I imagine you already ruled them out.

If you just run ldd on a binary / shared library the system will tell you what are the dependencies, something like this:

$ ldd /usr/local/lib/libnet.so
    linux-vdso.so.1 =>  (0x00007fffde5ff000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f0f164ff000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f0f16ae1000)

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

5 participants