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.

Sign up for GitHub

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

Jump to bottom

Add former default gems as a dependency for Ruby 3.1 compatibility #1439

Closed
wants to merge 2 commits into from
Closed

Add former default gems as a dependency for Ruby 3.1 compatibility #1439

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f80a1e
Add net-smtp as a dependency for Ruby 3.1 compatibility
byroot May 28, 2021
651a59a
Also add net-imap and net-pop to the gemspec
byroot May 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions mail.gemspec
View file
Open in desktop
Expand Up @@ -14,6 +14,9 @@ Gem::Specification.new do |s|
s.rdoc_options << '--exclude' << 'lib/mail/values/unicode_tables.dat'

s.add_dependency('mini_mime', '>= 0.1.1')
s.add_dependency('net-smtp')
s.add_dependency('net-imap')
s.add_dependency('net-pop')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these gems no-op on older Rubies (1.8, 1.9, etc)?

Copy link
Author

Choose a reason for hiding this comment

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

These gems yes, however they themselves have digest as dependency which is causing some doable loading problem:

/Users/byroot/.gem/ruby/2.7.2/gems/digest-3.0.0/lib/digest.rb:6: warning: already initialized constant Digest::REQUIRE_MUTEX
/opt/rubies/2.7.2/lib/ruby/2.7.0/digest.rb:6: warning: previous definition of REQUIRE_MUTEX was here
....

I already submitted the fix in bundler rubygems/rubygems#4989, hopefully it should be fixed soon. I totally understand if you want to wait for bundler to fix this first.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is already fixed @jeremy @casperisfine. Can we release this?

Copy link

Choose a reason for hiding this comment

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

Would it make sense to wrap these in an if statement that is only true for Ruby 3.1+?

Copy link
Contributor

@eregon eregon Jan 4, 2022 •
edited

Choose a reason for hiding this comment

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

The problem here is those gems can't be installed on older versions, @casperisfine's reply only shows part of the issue.

For example on 2.0.0:

$ gem i net-smtp            
Fetching: timeout-0.2.0.gem (100%)
Successfully installed timeout-0.2.0
Fetching: io-wait-0.2.1.gem (100%)
ERROR:  Error installing net-smtp:
	io-wait requires Ruby version >= 2.6.0.

$ gem i net-imap
ERROR:  Error installing net-imap:
	io-wait requires Ruby version >= 2.6.0.

$ gem i net-pop 
ERROR:  Error installing net-pop:
	io-wait requires Ruby version >= 2.6.0.

Also #1439 (comment)

Copy link

Choose a reason for hiding this comment

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

@nertzy I am pretty sure that an if statement in a .gemspec ends up getting evaluated at gem release time, and trying to condition on ruby version would end up being on what ruby version is running in the environment doing the gem release, and then fixed in the release.

My understanding is you can't actually do dynamic conditions based on the running environment in a gemspec like that.

Copy link
Owner

Choose a reason for hiding this comment

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

Ideally those gems ('net-smtp', 'net-imap' and 'net-pop') would all NOOP when installed on an older unsupported Ruby. That would just solve it for everyone.

Copy link
Author

Choose a reason for hiding this comment

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

So I'm still trying to find solutions upstream, but ultimately would you be open to drop pre 2.5 support to get this moved forward now?

Copy link
Contributor

@eregon eregon Feb 2, 2022 •
edited

Choose a reason for hiding this comment

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

For context, CRuby devs don't seem generally happy to need to explicitly support (even as noop) such old and EOL Ruby versions based on recent discussions, which brings the age-old question: do gems depending on net-* and specifically mail still need to support Ruby <= 2.5 today?

Copy link
Contributor

@simi simi Feb 2, 2022 •
edited

Choose a reason for hiding this comment

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

We can still go this way as well.

#1439 (comment)

Bump minor (or major) version and we can still maintain releases compat with <= 2.5 if needed in "legacy" branch.

Btw. I'm happy to help with anything moving this forward.


s.add_development_dependency('bundler', '>= 1.0.3')
s.add_development_dependency('rake', '> 0.8.7')
Expand Down