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

Lock sqlite3 version to 1.3 #3088

Merged

Conversation

mdesantis
Copy link
Contributor

Fixes #3087. It includes the following changes:

Lock sqlite3 version to 1.3

ActiveRecord 5.2.2 has sqlite3 dependency locked to ~> 1.3.6, we need to reflect that in order to avoid conflicts due to sqlite3 1.4 version releasement.

Fix sqlite3 version conflict on sandbox generation

rails new generates a Gemfile with gem 'sqlite3' dependency declaration, but since sqlite3 1.4 has been released it conflicts with activerecord sqlite3 adapter dependency declaration, which is sqlite3 ~> 1.3.6.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have added a CHANGELOG entry for this change (if needed)

Related to solidusio#3087. ActiveRecord 5.2.2 has sqlite3 dependency locked to ~>
1.3.6, we need to reflect that in order to avoid conflicts due to
sqlite3 1.4 version releasement.
@peterberkenbosch
Copy link
Contributor

sweet! I was just working on this. Trying to figure out how to "hack" the generated Gemfile. This will work!

@kennyadsl
Copy link
Member

This has been fixed in Rails (I think just in master for now).
What do you think about this solution made on factory_bot_rails using gsub_file instead of sed?

gsub_file "Gemfile", /^gem 'sqlite3'$/, 'gem "sqlite3", "~> 1.3.6"'

I think it's a bit more "Rails friendly".

@mdesantis
Copy link
Contributor Author

mdesantis commented Feb 7, 2019

@kennyadsl I guess they have a template for the new Rails app just checked and they didn't have already a template 😞 , but we don't have it, and I wouldn't add it just for this edit. Anyway, since in lib/sandbox.sh there is also

cat <<RUBY >> Gemfile

gem 'solidus', path: '..'
gem 'solidus_auth_devise', '>= 2.1.0'
gem 'rails-i18n'
gem 'solidus_i18n'

group :test, :development do
 platforms :mri do
   gem 'pry-byebug'
 end
end
RUBY

I could convert both of them to the "Rails template way", WDYT?

@kennyadsl
Copy link
Member

If possible, that would not be bad in my opinion. But maybe we can do that into another PR?

@mdesantis
Copy link
Contributor Author

@kennyadsl 👍 I'll do that in another PR

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

I am good with this, but what do you think about adding a comment in sandbox.sh explaining why we need to do this? Maybe we could just link to #3087.

@mdesantis
Copy link
Contributor Author

@jacobherrington here we are! can you please check that what I wrote makes sense? I feel it could be expressed better

Fixes solidusio#3087. `rails new` generates a Gemfile with `gem 'sqlite3'`
dependency declaration, but since sqlite3 1.4 has been released it
conflicts with activerecord sqlite3 adapter dependency declaration,
which is `sqlite3 ~> 1.3.6`.
Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

LGTM!

@jacobherrington jacobherrington merged commit 4c9f71f into solidusio:master Feb 8, 2019
@mdesantis mdesantis deleted the lock-sqlite3-version-to-1-3 branch February 8, 2019 13:16
@peterberkenbosch
Copy link
Contributor

This has been fixed in Rails (I think just in master for now).
What do you think about this solution made on factory_bot_rails using gsub_file instead of sed?

gsub_file "Gemfile", /^gem 'sqlite3'$/, 'gem "sqlite3", "~> 1.3.6"'

I think it's a bit more "Rails friendly".

This is not working in sandbox.sh since it's a shell script, not a ruby file.

@kennyadsl
Copy link
Member

@peterberkenbosch yeah, we talked about that. The only way to use gsub_file is moving that part into a Gemfile template and use that template with the -m option on the rails new command. Definitely not worth it. 🙂

@cedum cedum mentioned this pull request Sep 3, 2019
kennyadsl added a commit to nebulab/solidus that referenced this pull request Sep 3, 2019
Rails 5.2.2 was not compatible with sqlite 1.4 and needed us
to lock sqlite to 1.3.x to being able to complete the bundle
without errors

See solidusio#3088 and https://github.com/rails/rails/blob/94b5cd3a20edadd6f6b8cf0bdf1a4d4919df86cb/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

This is not true anymore starting from Rails 5.2.3 and we can safely
remove this constraint:

https://github.com/rails/rails/blob/b9ca94caea2ca6a6cc09abaffaad67b447134079/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
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

4 participants