RailsApps / rails_apps_composer

A gem with recipes to create Rails application templates for Rails starter apps.
http://railsapps.github.io/rails_apps_composer/
1.42k stars 306 forks source link

PostgreSQL support broken since January 10, 2018 (release of pg 1.0.0) #368

Closed yemartin closed 6 years ago

yemartin commented 6 years ago

The release of version 1.0.0 of the pg gem on January 10, 2018 has broken PostgreSQL support. Steps to reproduce: create a new Custom application with PostgreSQL support. It shows the following error:

      option  Okay to drop all existing databases named epark-affiliates? 'No' will abort immediately! (y/n) y
         run    bundle exec rake db:drop from "."
rake aborted!
Gem::LoadError: Specified 'postgresql' for database adapter, but the gem is not loaded. Add `gem 'pg'` to your Gemfile (and ensure its version is at the minimum required by ActiveRecord).

This is a knows issue that will be addressed in Rails 5.1.5, but even when fixed in Rails, it will leave rails_apps_composer with broken PostgreSQL support for any earlier Rails version...

As specified on the Rails pull request that fixes the issue, Rails versions prior to 5.1.5 (and to 5.0.?? must use:

gem "pg", "~> 0.18"

So maybe rails_apps_composer should look at the Rails version, and create the pg Gemfile line accordingly?

DanielKehoe commented 6 years ago

Yes, thanks for bringing it to my attention. I'll address it as soon.

DanielKehoe commented 6 years ago

@yemartin can you improve on my conditional logic, please?

if prefer :database, 'postgresql'
  if Rails::VERSION::MAJOR < 5
    add_gem 'pg', '~> 0.18'
  else
    if Rails::VERSION::MAJOR == 5 && Rails::VERSION::MINOR <= 1 && Rails::VERSION::MINOR <= 4
      add_gem 'pg', '~> 0.18'
    else
      add_gem 'pg'
    end
  end
end
DanielKehoe commented 6 years ago

Fixed in rails_apps_composer version 3.1.31.

DanielKehoe commented 6 years ago

@yemartin I've released the new rails_apps_composer version 3.1.31 and updated Rails Composer. Could you please test and reopen this issue if it's not resolved.

yemartin commented 6 years ago

@DanielKehoe Thank you for the great response time!

I see you already released the fix, but in case you are still interested in improving the conditional logic, how about using Gem::Version to do the semantic version comparisons? It makes the code cleaner, easier to grok, and less error-prone. If we only care about the fix in 5.1.5, the code becomes simply:

# We may want to extract this somewhere to the top to make it reusable:
RAILS_VERSION = Gem::Version.new(Rails::VERSION::STRING)

if prefer :database, 'postgresql'
  if RAILS_VERSION < Gem::Version.new("5.1.5")
    add_gem 'pg', '~> 0.18'
  else
    add_gem 'pg'
  end
end

Now if we want to take into account that the fix was also backported to both Rails 5.0.7 and 5.1.5, it is a bit more complex, but still not too bad:

# We may want to extract this somewhere to the top to make it reusable:
RAILS_VERSION = Gem::Version.new(Rails::VERSION::STRING)

if prefer :database, 'postgresql'
  if RAILS_VERSION < Gem::Version.new("5.0.7") || \
    (RAILS_VERSION >= Gem::Version.new("5.1") && RAILS_VERSION < Gem::Version.new("5.1.5"))
    add_gem 'pg', '~> 0.18'
  else
    add_gem 'pg'
  end
end

I tested it locally and it seems to work. But now, I see the rest of the code does not use Gem::Version... Is there is a good reason against it?