Replace Gitlab::REVISION with Gitlab.revision and handle installations without a .git directory46600-fix-gitlab-revision-when-not-in-git-repo

Signed-off-by: Rémy Coutable <remy@rymai.me>

16 files changed, 95 insertions, 14 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 495430931aa..75fd55a8f7b 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -618,7 +618,7 @@ module Ci
         variables.append(key: 'GITLAB_FEATURES', value: project.licensed_features.join(','))
         variables.append(key: 'CI_SERVER_NAME', value: 'GitLab')
         variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION)
-        variables.append(key: 'CI_SERVER_REVISION', value: Gitlab::REVISION)
+        variables.append(key: 'CI_SERVER_REVISION', value: Gitlab.revision)
         variables.append(key: 'CI_JOB_NAME', value: name)
         variables.append(key: 'CI_JOB_STAGE', value: stage)
         variables.append(key: 'CI_COMMIT_SHA', value: sha)
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index cfa1d9d0f0c..dd1800eff01 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -132,7 +132,7 @@
               GitLab
               %span.pull-right
                 = Gitlab::VERSION
-                = "(#{Gitlab::REVISION})"
+                = "(#{Gitlab.revision})"
             %p
               GitLab Shell
               %span.pull-right
diff --git a/app/views/help/index.html.haml b/app/views/help/index.html.haml
index bf2725dc328..c7a64d77a6a 100644
--- a/app/views/help/index.html.haml
+++ b/app/views/help/index.html.haml
@@ -8,7 +8,7 @@
   Community Edition
   - if user_signed_in?
     %span= Gitlab::VERSION
-    %small= link_to Gitlab::REVISION, Gitlab::COM_URL + namespace_project_commits_path('gitlab-org', 'gitlab-ce', Gitlab::REVISION)
+    %small= link_to Gitlab.revision, Gitlab::COM_URL + namespace_project_commits_path('gitlab-org', 'gitlab-ce', Gitlab.revision)
     = version_status_badge
   %hr
 
diff --git a/changelogs/unreleased/46600-fix-gitlab-revision-when-not-in-git-repo.yml b/changelogs/unreleased/46600-fix-gitlab-revision-when-not-in-git-repo.yml
new file mode 100644
index 00000000000..1d0b11cfd2a
--- /dev/null
+++ b/changelogs/unreleased/46600-fix-gitlab-revision-when-not-in-git-repo.yml
@@ -0,0 +1,6 @@
+---
+title: Replace Gitlab::REVISION with Gitlab.revision and handle installations without
+  a .git directory
+merge_request: 19125
+author:
+type: fixed
diff --git a/config/initializers/console_message.rb b/config/initializers/console_message.rb
index 536ab337d85..2c46a25f365 100644
--- a/config/initializers/console_message.rb
+++ b/config/initializers/console_message.rb
@@ -3,7 +3,7 @@ if defined?(Rails::Console)
   # note that this will not print out when using `spring`
   justify = 15
   puts "-------------------------------------------------------------------------------------"
-  puts " Gitlab:".ljust(justify) + "#{Gitlab::VERSION} (#{Gitlab::REVISION})"
+  puts " Gitlab:".ljust(justify) + "#{Gitlab::VERSION} (#{Gitlab.revision})"
   puts " Gitlab Shell:".ljust(justify) + Gitlab::Shell.new.version
   puts " #{Gitlab::Database.adapter_name}:".ljust(justify) + Gitlab::Database.version
   puts "-------------------------------------------------------------------------------------"
diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb
index b2da3b3dc19..17d09293205 100644
--- a/config/initializers/sentry.rb
+++ b/config/initializers/sentry.rb
@@ -13,7 +13,7 @@ def configure_sentry
   if sentry_enabled
     Raven.configure do |config|
       config.dsn = Gitlab::CurrentSettings.current_application_settings.sentry_dsn
-      config.release = Gitlab::REVISION
+      config.release = Gitlab.revision
 
       # Sanitize fields based on those sanitized from Rails.
       config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s)
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 6b72caea8fd..a3dac36b8b6 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -113,7 +113,7 @@ module API
         {
           api_version: API.version,
           gitlab_version: Gitlab::VERSION,
-          gitlab_rev: Gitlab::REVISION,
+          gitlab_rev: Gitlab.revision,
           redis: redis_ping
         }
       end
diff --git a/lib/api/version.rb b/lib/api/version.rb
index 9ba576bd828..3b10bfa6a7d 100644
--- a/lib/api/version.rb
+++ b/lib/api/version.rb
@@ -6,7 +6,7 @@ module API
       detail 'This feature was introduced in GitLab 8.13.'
     end
     get '/version' do
-      { version: Gitlab::VERSION, revision: Gitlab::REVISION }
+      { version: Gitlab::VERSION, revision: Gitlab.revision }
     end
   end
 end
diff --git a/lib/gitlab.rb b/lib/gitlab.rb
index 31119471b8e..a129746e2c6 100644
--- a/lib/gitlab.rb
+++ b/lib/gitlab.rb
@@ -13,11 +13,26 @@ module Gitlab
     @_migrations_hash ||= Digest::MD5.hexdigest(ActiveRecord::Migrator.get_all_versions.to_s)
   end
 
+  def self.revision
+    @_revision ||= begin
+      if File.exist?(root.join("REVISION"))
+        File.read(root.join("REVISION")).strip.freeze
+      else
+        result = Gitlab::Popen.popen_with_detail(%W[#{config.git.bin_path} log --pretty=format:%h -n 1])
+
+        if result.status.success?
+          result.stdout.chomp.freeze
+        else
+          "Unknown".freeze
+        end
+      end
+    end
+  end
+
   COM_URL = 'https://gitlab.com'.freeze
   APP_DIRS_PATTERN = %r{^/?(app|config|ee|lib|spec|\(\w*\))}
   SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z}
   VERSION = File.read(root.join("VERSION")).strip.freeze
-  REVISION = Gitlab::Popen.popen(%W(#{config.git.bin_path} log --pretty=format:%h -n 1)).first.chomp.freeze
 
   def self.com?
     # Check `gl_subdomain?` as well to keep parity with gitlab.com
diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb
index c741dabe168..0d31934347f 100644
--- a/lib/gitlab/gon_helper.rb
+++ b/lib/gitlab/gon_helper.rb
@@ -15,7 +15,7 @@ module Gitlab
       gon.user_color_scheme      = Gitlab::ColorSchemes.for_user(current_user).css_class
       gon.sentry_dsn             = Gitlab::CurrentSettings.clientside_sentry_dsn if Gitlab::CurrentSettings.clientside_sentry_enabled
       gon.gitlab_url             = Gitlab.config.gitlab.url
-      gon.revision               = Gitlab::REVISION
+      gon.revision               = Gitlab.revision
       gon.gitlab_logo            = ActionController::Base.helpers.asset_path('gitlab_logo.png')
       gon.sprite_icons           = IconsHelper.sprite_icon_path
       gon.sprite_file_icons      = IconsHelper.sprite_file_icons_path
diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake
index 47ed522aec3..289aa5d9060 100644
--- a/lib/tasks/gitlab/info.rake
+++ b/lib/tasks/gitlab/info.rake
@@ -47,7 +47,7 @@ namespace :gitlab do
       puts ""
       puts "GitLab information".color(:yellow)
       puts "Version:\t#{Gitlab::VERSION}"
-      puts "Revision:\t#{Gitlab::REVISION}"
+      puts "Revision:\t#{Gitlab.revision}"
       puts "Directory:\t#{Rails.root}"
       puts "DB Adapter:\t#{database_adapter}"
       puts "URL:\t\t#{Gitlab.config.gitlab.url}"
diff --git a/spec/lib/gitlab_spec.rb b/spec/lib/gitlab_spec.rb
index da146e24893..d63f448883b 100644
--- a/spec/lib/gitlab_spec.rb
+++ b/spec/lib/gitlab_spec.rb
@@ -8,6 +8,66 @@ describe Gitlab do
       expect(described_class.root).to eq(Pathname.new(File.expand_path('../..', __dir__)))
     end
   end
+  describe '.revision' do
+    let(:cmd) { %W[#{described_class.config.git.bin_path} log --pretty=format:%h -n 1] }
+
+    around do |example|
+      described_class.instance_variable_set(:@_revision, nil)
+      example.run
+      described_class.instance_variable_set(:@_revision, nil)
+    end
+
+    context 'when a REVISION file exists' do
+      before do
+        expect(File).to receive(:exist?)
+          .with(described_class.root.join('REVISION'))
+          .and_return(true)
+      end
+
+      it 'returns the actual Git revision' do
+        expect(File).to receive(:read)
+          .with(described_class.root.join('REVISION'))
+          .and_return("abc123\n")
+
+        expect(described_class.revision).to eq('abc123')
+      end
+
+      it 'memoizes the revision' do
+        expect(File).to receive(:read)
+          .once
+          .with(described_class.root.join('REVISION'))
+          .and_return("abc123\n")
+
+        2.times { described_class.revision }
+      end
+    end
+
+    context 'when no REVISION file exist' do
+      context 'when the Git command succeeds' do
+        before do
+          expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with(cmd)
+          .and_return(Gitlab::Popen::Result.new(cmd, 'abc123', '', double(success?: true)))
+        end
+
+        it 'returns the actual Git revision' do
+          expect(described_class.revision).to eq('abc123')
+        end
+      end
+
+      context 'when the Git command fails' do
+        before do
+          expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with(cmd)
+          .and_return(Gitlab::Popen::Result.new(cmd, '', 'fatal: Not a git repository', double('Process::Status', success?: false)))
+        end
+
+        it 'returns "Unknown"' do
+          expect(described_class.revision).to eq('Unknown')
+        end
+      end
+    end
+  end
 
   describe '.com?' do
     it 'is true when on GitLab.com' do
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 96173889ccd..77179028ede 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1546,7 +1546,7 @@ describe Ci::Build do
         { key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true },
         { key: 'CI_SERVER_NAME', value: 'GitLab', public: true },
         { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true },
-        { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true },
+        { key: 'CI_SERVER_REVISION', value: Gitlab.revision, public: true },
         { key: 'CI_JOB_NAME', value: 'test', public: true },
         { key: 'CI_JOB_STAGE', value: 'test', public: true },
         { key: 'CI_COMMIT_SHA', value: build.sha, public: true },
diff --git a/spec/requests/api/version_spec.rb b/spec/requests/api/version_spec.rb
index 7bbf34422b8..38b618191fb 100644
--- a/spec/requests/api/version_spec.rb
+++ b/spec/requests/api/version_spec.rb
@@ -18,7 +18,7 @@ describe API::Version do
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['version']).to eq(Gitlab::VERSION)
-        expect(json_response['revision']).to eq(Gitlab::REVISION)
+        expect(json_response['revision']).to eq(Gitlab.revision)
       end
     end
   end
diff --git a/spec/views/admin/dashboard/index.html.haml_spec.rb b/spec/views/admin/dashboard/index.html.haml_spec.rb
index 099baacf019..59c777ea338 100644
--- a/spec/views/admin/dashboard/index.html.haml_spec.rb
+++ b/spec/views/admin/dashboard/index.html.haml_spec.rb
@@ -22,6 +22,6 @@ describe 'admin/dashboard/index.html.haml' do
   it "includes revision of GitLab" do
     render
 
-    expect(rendered).to have_content "#{Gitlab::VERSION} (#{Gitlab::REVISION})"
+    expect(rendered).to have_content "#{Gitlab::VERSION} (#{Gitlab.revision})"
   end
 end
diff --git a/spec/views/help/index.html.haml_spec.rb b/spec/views/help/index.html.haml_spec.rb
index 0a78606171d..836d452304c 100644
--- a/spec/views/help/index.html.haml_spec.rb
+++ b/spec/views/help/index.html.haml_spec.rb
@@ -39,7 +39,7 @@ describe 'help/index' do
 
   def stub_version(version, revision)
     stub_const('Gitlab::VERSION', version)
-    stub_const('Gitlab::REVISION', revision)
+    allow(Gitlab).to receive(:revision).and_return(revision)
   end
 
   def stub_helpers