Merge pull request #3880 from hiroponz/fix-check-git-version

Add Gitlab::VersionInfo class to fix and simplify version check.

lib/gitlab/version_info.rb

+module Gitlab
+  class VersionInfo
+    include Comparable
+
+    attr_reader :major, :minor, :patch
+
+    def self.parse(str)
+      if m = str.match(/(\d+)\.(\d+)\.(\d+)/)
+        VersionInfo.new(m[1].to_i, m[2].to_i, m[3].to_i)
+      else
+        VersionInfo.new
+      end
+    end
+
+    def initialize(major = 0, minor = 0, patch = 0)
+      @major = major
+      @minor = minor
+      @patch = patch
+    end
+
+    def <=>(other)
+      return unless other.is_a? VersionInfo
+      return unless valid? && other.valid?
+
+      if other.major < @major
+        1
+      elsif @major < other.major
+        -1
+      elsif other.minor < @minor
+        1
+      elsif @minor < other.minor
+        -1
+      elsif other.patch < @patch
+        1
+      elsif @patch < other.patch
+        -1
+      else
+        0
+      end
+    end
+
+    def to_s
+      if valid?
+        "%d.%d.%d" % [@major, @minor, @patch]
+      else
+        "Unknown"
+      end
+    end
+
+    def valid?
+      @major >= 0 && @minor >= 0 && @patch >= 0 && @major + @minor + @patch > 0
+    end
+  end
+end

lib/tasks/gitlab/check.rake

@@ -655,39 +655,25 @@ namespace :gitlab do
   end
 
   def check_gitlab_shell
-    required_version = '1.4.0'
+    required_version = Gitlab::VersionInfo.new(1, 4, 0)
+    current_version = Gitlab::VersionInfo.parse(gitlab_shell_version)
 
-    print "GitLab Shell version? ... "
-    if gitlab_shell_version.strip == required_version
-      puts "OK (#{required_version})".green
+    print "GitLab Shell version >= #{required_version} ? ... "
+    if required_version <= current_version
+      puts "OK (#{current_version})".green
     else
-      puts "FAIL. Please update gitlab-shell to v#{required_version}".red
+      puts "FAIL. Please update gitlab-shell to #{required_version} from #{current_version}".red
     end
   end
 
   def check_git_version
-    required_version_major = 1
-    required_version_minor = 7
-    required_version_patch = 10
-
-    required_version = "%d.%d.%d" %[required_version_major, required_version_minor, required_version_patch]
+    required_version = Gitlab::VersionInfo.new(1, 7, 10)
+    current_version = Gitlab::VersionInfo.parse(run("git --version"))
 
     print "Git version >= #{required_version} ? ... "
 
-    if m = run_and_match("git --version", /git version ((\d+)\.(\d+)\.(\d+))/)
-      current_version = m[1]
-      major = m[2].to_i
-      minor = m[3].to_i
-      patch = m[4].to_i
-      unless major <= required_version_major && minor <= required_version_minor && patch < required_version_patch
-        satisfying_git_version = true
-      end
-    else
-      current_version = "Unknown"
-    end
-
-    if satisfying_git_version
-        puts "yes".green
+    if required_version <= current_version
+        puts "yes (#{current_version})".green
     else
       puts "no".red
       try_fixing_it(

spec/lib/gitlab/version_info_spec.rb

+require 'spec_helper'
+
+describe 'Gitlab::VersionInfo', no_db: true do
+  before do
+    @unknown = Gitlab::VersionInfo.new
+    @v0_0_1 = Gitlab::VersionInfo.new(0, 0, 1)
+    @v0_1_0 = Gitlab::VersionInfo.new(0, 1, 0)
+    @v1_0_0 = Gitlab::VersionInfo.new(1, 0, 0)
+    @v1_0_1 = Gitlab::VersionInfo.new(1, 0, 1)
+    @v1_1_0 = Gitlab::VersionInfo.new(1, 1, 0)
+    @v2_0_0 = Gitlab::VersionInfo.new(2, 0, 0)
+  end
+
+  context '>' do
+    it { @v2_0_0.should > @v1_1_0 }
+    it { @v1_1_0.should > @v1_0_1 }
+    it { @v1_0_1.should > @v1_0_0 }
+    it { @v1_0_0.should > @v0_1_0 }
+    it { @v0_1_0.should > @v0_0_1 }
+  end
+
+  context '>=' do
+    it { @v2_0_0.should >= Gitlab::VersionInfo.new(2, 0, 0) }
+    it { @v2_0_0.should >= @v1_1_0 }
+  end
+
+  context '<' do
+    it { @v0_0_1.should < @v0_1_0 }
+    it { @v0_1_0.should < @v1_0_0 }
+    it { @v1_0_0.should < @v1_0_1 }
+    it { @v1_0_1.should < @v1_1_0 }
+    it { @v1_1_0.should < @v2_0_0 }
+  end
+
+  context '<=' do
+    it { @v0_0_1.should <= Gitlab::VersionInfo.new(0, 0, 1) }
+    it { @v0_0_1.should <= @v0_1_0 }
+  end
+
+  context '==' do
+    it { @v0_0_1.should == Gitlab::VersionInfo.new(0, 0, 1) }
+    it { @v0_1_0.should == Gitlab::VersionInfo.new(0, 1, 0) }
+    it { @v1_0_0.should == Gitlab::VersionInfo.new(1, 0, 0) }
+  end
+
+  context '!=' do
+    it { @v0_0_1.should_not == @v0_1_0 }
+  end
+
+  context 'unknown' do
+    it { @unknown.should_not be @v0_0_1 }
+    it { @unknown.should_not be Gitlab::VersionInfo.new }
+    it { expect{@unknown > @v0_0_1}.to raise_error(ArgumentError) }
+    it { expect{@unknown < @v0_0_1}.to raise_error(ArgumentError) }
+  end
+
+  context 'parse' do
+    it { Gitlab::VersionInfo.parse("1.0.0").should == @v1_0_0 }
+    it { Gitlab::VersionInfo.parse("1.0.0.1").should == @v1_0_0 }
+    it { Gitlab::VersionInfo.parse("git 1.0.0b1").should == @v1_0_0 }
+    it { Gitlab::VersionInfo.parse("git 1.0b1").should_not be_valid }
+  end
+
+  context 'to_s' do
+    it { @v1_0_0.to_s.should == "1.0.0" }
+    it { @unknown.to_s.should == "Unknown" }
+  end
+end
+