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

Add rubygems version enforcement #32

Merged
merged 5 commits into from
Oct 23, 2015
Merged

Conversation

pcarranza
Copy link
Contributor

To prevent having weird errors like TypeError: no implicit conversion of StringIO into String for using old versions of rubygems, we enforce the use of anything over 2.4.

There are 2 lines of defense:

  • we do not allow rake to install at all if the rubygems version is older.
  • we do not allow running gemstash in the same condition.

In the latter, we are also going to show how to fix the problem in the error message.

cc @indirect @smellsblue

@pcarranza
Copy link
Contributor Author

My bad, fixing

@@ -36,6 +37,12 @@ def store_daemonized
Gemstash::Env.daemonized = daemonize?
end

def check_rubygems_version
raise "Rubygems version is too old, please update rubygems by running: " \
"gem install rubygems-update && update_rubygems && gem update system" unless
Copy link
Member

Choose a reason for hiding this comment

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

you can update with just gem update --system :)

@pcarranza pcarranza force-pushed the check-rubygems-version branch from c89a629 to 4443d84 Compare October 19, 2015 21:35
agis added a commit to rubygems/bundler that referenced this pull request Oct 20, 2015
Since gemstash will only support RubyGems >= 2.4 (rubygems/gemstash#32),
we only set this header in RubyGems 2.0+.
agis added a commit to rubygems/bundler that referenced this pull request Oct 20, 2015
Since gemstash will only support RubyGems >= 2.4 (rubygems/gemstash#32),
we only set this header in RubyGems 2.0+.
agis added a commit to rubygems/bundler that referenced this pull request Oct 20, 2015
Since gemstash will only support RubyGems >= 2.4 (rubygems/gemstash#32),
we only set this header in RubyGems 2.0+.
agis added a commit to rubygems/bundler that referenced this pull request Oct 20, 2015
Since gemstash will only support RubyGems >= 2.4 (rubygems/gemstash#32),
we only set this header in RubyGems 2.0+.
@pcarranza
Copy link
Contributor Author

Are we good to go here?

@pcarranza pcarranza force-pushed the check-rubygems-version branch from 4443d84 to fb066b5 Compare October 21, 2015 22:47
def check_rubygems_version
STDERR.puts("Rubygems version is too old, please update rubygems by running: " \
"gem update --system") unless
Gem::Requirement.new("~> 2.4").satisfied_by?(Gem::Version.new(Gem::VERSION))
Copy link
Contributor

Choose a reason for hiding this comment

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

How about gem requirement of ">= 2.4" instead? Otherwise this will give an erroneous too old message if they go to 3.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, do you think it makes sense to colorize the message as red or yellow to draw more attention to it, or is that overkill?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep to both, I like colors, red is so warm.

On Thu, Oct 22, 2015 at 1:25 AM, Mike Virata-Stone <notifications@github.com

wrote:

In lib/gemstash/cli/start.rb
#32 (comment):

@@ -36,6 +37,12 @@ def store_daemonized
Gemstash::Env.daemonized = daemonize?
end

  •  def check_rubygems_version
    
  •    STDERR.puts("Rubygems version is too old, please update rubygems by running: " \
    
  •                "gem update --system") unless
    
  •                Gem::Requirement.new("~> 2.4").satisfied_by?(Gem::Version.new(Gem::VERSION))
    

Also, do you think it makes sense to colorize the message as red or yellow
to draw more attention to it, or is that overkill?


Reply to this email directly or view it on GitHub
https://github.com/bundler/gemstash/pull/32/files#r42699875.

@pcarranza pcarranza force-pushed the check-rubygems-version branch from 133102b to 9015877 Compare October 22, 2015 21:50
@smellsblue
Copy link
Contributor

👍

@pcarranza pcarranza merged commit 9015877 into master Oct 23, 2015
@pcarranza pcarranza deleted the check-rubygems-version branch October 23, 2015 08:25
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.

3 participants