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

allow to install specific gems on specific rubys #125

Merged
merged 1 commit into from Apr 26, 2016

Conversation

bastelfreak
Copy link
Member

one of our needed gems has listen as a dependency. they upgraded listen
from 3.0.6 to 3.1.1. It needed at least ruby19, now it's ruby22. We try
to pin it to the last working version on certain platforms. this is
based on:
https://github.com/theforeman/puppet-puppet/blob/8c0ccfc8268d90f8ae19d92a4d395733a845db84/Gemfile
https://github.com/theforeman/foreman-installer-modulesync/blob/b4ce08e08933b6dcf88978a50304e056cd1878aa/config_defaults.yml

for reference:
https://travis-ci.org/voxpupuli/puppet-collectd/jobs/125300240#L206

@bastelfreak
Copy link
Member Author

@jyaworski could you take a look? I'n not completely sure about the syntax of the jruby version

options:
platforms:
- 'ruby_21'
- 'jruby_19'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastelfreak bastelfreak force-pushed the pin-listen branch 5 times, most recently from ceaf5e3 to cbac5d6 Compare April 24, 2016 00:40
@rnelson0
Copy link
Sponsor Member

That is horrible for a minor release. Has anyone reported this to listen as breaking server yet?

@bastelfreak
Copy link
Member Author

I opened guard/listen#374

@bastelfreak
Copy link
Member Author

for reference: semver/semver#148

bastelfreak added a commit that referenced this pull request Apr 24, 2016
this is a long story, please have a look at
#125
@bastelfreak
Copy link
Member Author

so, after digging very deeply in very dirty places, the awesome @jyaworski and I came to two working solutions:

We can get the needed behavior with a little if statement:

install_if -> { Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.0') } do
  gem 'listen', '~> 3.0.0', :require => false
end

@jyaworski discovered that, I tested it . However this is hard to automate if we need to do this for multiple gems in the future, the only solution that came up my mind is to hardcode these three lines in our Gemfile provided by modulesync_config. This sucks because not every module needs the listen gem.


the way more awesome method:

gem 'listen', '3.0.6', :require => false, :platforms => [:ruby_21, :jruby]

adding a platforms attribute to the gem line works fine, I tested this. The modulesync_config update in this PR should add the above code, however this is currently untested. It would allow us to specify and ruby/gem version combination that we want.


Both of these solutions require a recent rubygems/bundler version. Getting them will be implemented in #127. I tested that already. @jyaworski @rnelson0 can I get some opinions here? If you prefer the templated solution (the second one), like I do, I would like to rebase/squash this PR, test if it generates the correct Gemfile and merge it. Thank you all!

@jyaworski
Copy link
Member

Squash please.

bastelfreak added a commit to bastelfreak/modulesync_config that referenced this pull request Apr 25, 2016
this is related to a very long story. TL;DR: they moved from 3.0.6 to
3.1.0 which bumped the required ruby from 1.9.3 to 2.2. This will break
a few systems. For reference:
voxpupuli#125
guard/listen#374
@rnelson0
Copy link
Sponsor Member

Merging this with just the option to pin to :platform, rather than actually pinning anything. We will hold off on that until a user actually requires it.

@bastelfreak bastelfreak changed the title pin listen on jruby and ruby21 allow to install specific gems on specific rubys Apr 26, 2016
@rnelson0 rnelson0 merged commit bb6d338 into voxpupuli:master Apr 26, 2016
elmendalerenda pushed a commit to elmendalerenda/puppet-kafka that referenced this pull request Mar 30, 2018
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

3 participants