Closed dazza-codes closed 9 years ago
@ndushay might appreciate some git rebase -i
work that went into this PR (although it's not obvious now that it's done).
One important thing to discuss is what happens if the ENV['RAILS_VERSION']
is set. Having removed the code that uses this in the triannon Gemfile, will it be used anyway by the engine cart generate process? So, I have yet to evaluate what happens when we run, say:
RAILS_VERSION=4.2.1 rake ci
BTW, the gemnasium badge on master indicates the gemsets are outdated, but this seems to be only because of 'tilt' at 1.1 (latest is 2.x), see https://gemnasium.com/sul-dlss/triannon
Pushing changes that result from discussion with @cbeer to clarify how and why engine cart modifies the triannon Gemfile. The file change is something like this pseudocode:
if engine_cart_gemfile_exists
load_it
else
define_development_dependencies
end
The 'else' clause is designed for defining dependencies to be tweaked and evaluated in development (this might be used to specify or update a dependency in triannon.gemspec). By separating these out into a :development
group, they cannot have side-effects on the :test
environment. The goal is to ensure that the :test
environment mimics how the triannon gem will behave in third-party rails applications.
When the 'else' clause is available to the :test
environment, it can have side effects. For example, when travis runs tests, it first runs a bundle install
on triannon to get the engine cart gem and other triannon gem and test dependencies. This creates a new Gemfile.lock
file with dependencies specific to triannon, without any 'consideration' for the engine cart dependencies. The next step is to create the engine cart app, which should only use the explicit dependencies in triannon.gemspec. However, when the Gemfile.lock
file exists, the engine cart generation process will use it to resolve the app dependencies and this can result in unnecessary conflicts. In the :test
environmnent, the engine cart app should be created without regard to the triannon Gemfile.lock
file.
It might be useful if you added a comment in your Gemfile
to document the approach and deviations from the pattern provided by the engine_cart install generator , e.g.:
file = File.expand_path("Gemfile", ENV['ENGINE_CART_DESTINATION'] || ENV['RAILS_ROOT'] || File.expand_path("../spec/internal", __FILE__))
if File.exists?(file)
puts "Loading #{file} ..." if $DEBUG # `ruby -d` or `bundle -v`
instance_eval File.read(file)
else
gem 'rails', ENV['RAILS_VERSION'] if ENV['RAILS_VERSION']
if ENV['RAILS_VERSION'] and ENV['RAILS_VERSION'] =~ /^4.2/
gem 'responders', "~> 2.0"
gem 'sass-rails', ">= 5.0"
else
gem 'sass-rails', "< 5.0"
end
end
@cbeer - good suggestion and, with further testing, the most recent revision to this PR gets a lot closer to the desired goals, including added commentary. (Note the commit messages are no longer generated with commit -m 'xyz comment'
and should provide more useful information.)
After team discussion today, this PR will be abandoned. The changes proposed here may be useful reference at some later date (so the PR should be available in the github PR view when the pr:open
filter is removed).
This PR is designed to cleanup and clarify how to update the gemsets for triannon and the engine cart app that will test triannon.
These changes have been evaluated by repeatedly running
rake ci
on my laptop and the same task has been run on travis with successful results.Comments and feedback are especially appreciated from @ndushay and @cbeer. Please bear in mind that a guiding principle of this PR is to avoid using Gemfile.lock, as much as possible, by containing gem pins solely in triannon.gemspec (i.e. removing them from the triannon Gemspec file).