Move puppet dev dep to Gemfile, add puppet-strings dep #19
Conversation
| puppet_spec = puppet_version ? "~> #{puppet_version}" : '< 5.0.0' | ||
| gem 'puppet', puppet_spec | ||
|
|
||
| if puppet_version.nil? || puppet_version >= '3.7' |
There was a problem hiding this comment.
Does this string comparison work?
There was a problem hiding this comment.
Yeah, should do for any typical string that's likely to be entered here. I can't think of an example that wouldn't.
There was a problem hiding this comment.
In the case of puppet it's unlikely, but I'd expect it to fail with 3.10.
There was a problem hiding this comment.
Would it be possible to rather load custom Gemfile.local.rb which would be created in jenkins job? It seems a bit safer, puppet env variable looks too generic. If this is just for jenkins purposes, why do we need two env variables?
There was a problem hiding this comment.
I was trying to keep the logic inside the Gemfile for handling multiple Puppet versions, so it's easier for a developer to install a certain Puppet version and get the right extras (e.g. puppet-strings) and to avoid editing Gemfiles from Jenkins jobs. This should make it easier for a developer to reproduce a Jenkins build on a particular version.
The puppet environment variable came from the current name of the Jenkins job matrix axis, while PUPPET_VERSION came from our Puppet module tests where this pattern's used. I could remove puppet here and rename the axis in Jenkins after it's merged.
There was a problem hiding this comment.
ok, I'm fine with env variables then, but please remove the puppet one, then I'm happy to merge
theforeman-bot
added
Needs testing
Needs re-review
and removed
Waiting on contributor
labels
|
Updated to remove the |
|
I suppose tests are failing because the Jenkins job would have to be fixed, it seems to work locally... @domcleal do you me want to keep this PR as a reproducer for job updates or should I merge? I'll release 0.1.1 once it's in. |
The 'PUPPET_VERSION' environment variable will be used to install specific versions of Puppet, and if the version specified supports puppet-strings, then the dep's added to test the strings parser. This allows the removal of Gemfile alterations from the Jenkins job and makes it easier to test different versions in development.
|
oh right, RUBY_VERSION >= '1.9' 💡 |
I had to make a further change to the Gemfile for the Puppet 3.7/8 and Ruby 1.8.7 combination, so puppet-strings is only installed on Ruby 1.9 or higher as it's incompatible with 1.8.
Yeah, I saw another odd error to do with Bundler indexes on the previous run. It seems to be an intermittent error - the latest run doesn't exhibit it. |
|
Most of the intermittent failures are from a shared cache in Bundler 1.12 I think, so I filed rubygems/bundler#4519. The segfaults appear to only be on Ruby 1.9.2, perhaps it's a bug in that version as the new Bundler release is highly parallel. I've pinned Bundler to 1.11.2 in the jobs for now to avoid these issues. |
|
tbh, I don't think we should test anything on Ruby 1.9.2, because no distro uses it anyway. |
|
Yeah, was that mostly for Squeeze? |
|
Maybe, but even there 1.8 was the ruby used, I think... |
|
I think @domcleal is right: Squeeze has a package named |
|
Thanks @domcleal, merging! |
The 'PUPPET_VERSION' or 'puppet' environment variables will be used to
install specific versions of Puppet, and if the version specified
supports puppet-strings, then the dep's added to test the strings
parser.
This allows the removal of Gemfile alterations from the Jenkins job and
makes it easier to test different versions in development.