Deprecate OpenSSL::Digest and OpenSSL::Cipher constants

[bdewater]

Follow-up to #362 and #304 - actually deprecate these constants.

[ioquatix]

I think we should do one minor point release before deprecating these constants in order to allow people to migrate without a million warnings. @rhenium what do you think? How far away are we from cutting a 2.2 release?

[rhenium]

I'm afraid deprecating those constants would have a too big impact at this point, especially since the constant deprecation warning is shown without $VERBOSE set to true.

A quick grep locally shows Ruby's standard libraries, RubyGems, Rails, etc. still use them. They should be updated in their future releases, but that would discourage people to use the latest version of openssl gem in existing projects.

[bdewater]

I'm working on a Rubocop rule to provide an easy way to autocorrect these. If the long term intent is to follow through with the deprecation I'm sure it shouldn't be a problem to get it merged upstream. We can hold off with merging this PR until then.

[bdewater]

This can be merged now. The cop has been released in Rubocop 0.84 and as can be seen by the pull requests mentioning this one, people have started making changes in their codebases.

[rhenium]

My concern is that a user could want to upgrade openssl gem but not to upgrade everything else at the same time.

The warning can be a blocker for the upgrade since many projects have a habit of clearing up all warnings. It's not a desirable outcome if people start writing strict version constraints in the Gemfile just to suppress it.

I'm all for the (soft) deprecation of OpenSSL::Digest::* constants, which don't offer any additional benefits compared to the "reference by name" ways and are unsustainable from the maintenance perspective. The changes in Ruby/OpenSSL docs, RubyGems, Rails, etc. are definitely a good thing.

However, I feel applying deprecate_constant right now is too aggressive.

[ioquatix]

@rhenium it's chicken and egg problem. If we don't issue warnings, code will never change, if code never changes, by your logic warnings are too intrusive. So can you propose an alternative? Maybe it's wait for 6 months?

[bdewater]

The warning can be a blocker for the upgrade since many projects have a habit of clearing up all warnings.

This is easy with rubocop/rubocop#7950 by running bundle exec rubocop --only Lint/DeprecatedOpenSSLConstant -a and it autocorrects code for you. I can call this out in the changelog entry to make sure everyone upgrading knows.

It's not a desirable outcome if people start writing strict version constraints in the Gemfile just to suppress it. [...] However, I feel applying deprecate_constant right now is too aggressive.

I assumed this would be merged for a 3.0 release, deprecating something in a major upgrade sounds like the right time to do this. Especially if it has been soft-deprecated in the documentation a release before. We should add it to this milestone or another one if that's deemed more suitable.

I agree with @ioquatix this is a chicken and egg problem. People may temporarily downgrade but the compatibility with a new OpenSSL library version and new features (Ed25519 support) will have people upgrade eventually and deal with this.

[rhenium]

This is easy with rubocop-hq/rubocop#7950 by running bundle exec rubocop --only Lint/DeprecatedOpenSSLConstant -a and it autocorrects code for you. I can call this out in the changelog entry to make sure everyone upgrading knows.

It's complicated when the warning is coming out of dependency libraries, not just from the own application code.

The current release of RubyGems uses those constants in Gem::Security namespace. My quick test showed that once openssl gem is upgraded (to include the deprecation warning) globally, every gem install/bundle install call afterward will emit "warning: constant OpenSSL::Digest::SHA256 is deprecated". This can be resolved by upgrading RubyGems itself with gem update --system, of course, but I have the feeling people typically stick with the version bundled with the Ruby release in recent years.

Another example, they are used by Rails (and probably similar cases in other libraries/frameworks). The changes will not be backported since it involves deprecations in their interface. So everyone who wants to use newer openssl gem, and to clear the warnings, would be required to upgrade to bleeding-edge Rails 6.1.

It might indeed be a chicken and egg problem, but honestly, I don't see a much immediate need for the transition. The use of those legacy constants per se isn't an actual issue until OpenSSL decides to drop the implementation for the algorithm. The transition is a good thing but we're not ready.

[eregon]

I agree with @rhenium, the shipped RubyGems should work without warnings, and if installing a newer openssl gem means many warnings it seems very annoying.

Is there a need to remove those constants?
If not I'd keep them to save everyone the trouble.

Personally, I find OpenSSL::Digest::MD5.digest("") much nicer than OpenSSL::Digest.digest("MD5", ""), similar as for encodings Encoding::UTF_8 vs Encoding.find('utf-8').
But I guess libssl doesn't provide a way to list the digest names?