Remove support for absolute dirs from Git::Env

4 files changed, 38 insertions, 77 deletions

diff --git a/spec/lib/gitlab/git/env_spec.rb b/spec/lib/gitlab/git/hook_env_spec.rb
index 03836d49518..e6aa5ad8c90 100644
--- a/spec/lib/gitlab/git/env_spec.rb
+++ b/spec/lib/gitlab/git/hook_env_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
-describe Gitlab::Git::Env do
+describe Gitlab::Git::HookEnv do
+  let(:gl_repository) { 'project-123' }
+
   describe ".set" do
     context 'with RequestStore.store disabled' do
       before do
@@ -8,9 +10,9 @@ describe Gitlab::Git::Env do
       end
 
       it 'does not store anything' do
-        described_class.set(GIT_OBJECT_DIRECTORY: 'foo')
+        described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')
 
-        expect(described_class.all).to be_empty
+        expect(described_class.all(gl_repository)).to be_empty
       end
     end
 
@@ -21,15 +23,19 @@ describe Gitlab::Git::Env do
 
       it 'whitelist some `GIT_*` variables and stores them using RequestStore' do
         described_class.set(
-          GIT_OBJECT_DIRECTORY: 'foo',
-          GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar',
+          gl_repository,
+          GIT_OBJECT_DIRECTORY_RELATIVE: 'foo',
+          GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: 'bar',
           GIT_EXEC_PATH: 'baz',
           PATH: '~/.bin:/bin')
 
-        expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo')
-        expect(described_class[:GIT_ALTERNATE_OBJECT_DIRECTORIES]).to eq('bar')
-        expect(described_class[:GIT_EXEC_PATH]).to be_nil
-        expect(described_class[:bar]).to be_nil
+        git_env = described_class.all(gl_repository)
+
+        expect(git_env[:GIT_OBJECT_DIRECTORY_RELATIVE]).to eq('foo')
+        expect(git_env[:GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE]).to eq('bar')
+        expect(git_env[:GIT_EXEC_PATH]).to be_nil
+        expect(git_env[:PATH]).to be_nil
+        expect(git_env[:bar]).to be_nil
       end
     end
   end
@@ -39,14 +45,15 @@ describe Gitlab::Git::Env do
       before do
         allow(RequestStore).to receive(:active?).and_return(true)
         described_class.set(
-          GIT_OBJECT_DIRECTORY: 'foo',
-          GIT_ALTERNATE_OBJECT_DIRECTORIES: ['bar'])
+          gl_repository,
+          GIT_OBJECT_DIRECTORY_RELATIVE: 'foo',
+          GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: ['bar'])
       end
 
       it 'returns an env hash' do
-        expect(described_class.all).to eq({
-          'GIT_OBJECT_DIRECTORY' => 'foo',
-          'GIT_ALTERNATE_OBJECT_DIRECTORIES' => ['bar']
+        expect(described_class.all(gl_repository)).to eq({
+          'GIT_OBJECT_DIRECTORY_RELATIVE' => 'foo',
+          'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['bar']
         })
       end
     end
@@ -56,8 +63,8 @@ describe Gitlab::Git::Env do
     context 'with RequestStore.store enabled' do
       using RSpec::Parameterized::TableSyntax
 
-      let(:key) { 'GIT_OBJECT_DIRECTORY' }
-      subject { described_class.to_env_hash }
+      let(:key) { 'GIT_OBJECT_DIRECTORY_RELATIVE' }
+      subject { described_class.to_env_hash(gl_repository) }
 
       where(:input, :output) do
         nil         | nil
@@ -70,7 +77,7 @@ describe Gitlab::Git::Env do
       with_them do
         before do
           allow(RequestStore).to receive(:active?).and_return(true)
-          described_class.set(key.to_sym => input)
+          described_class.set(gl_repository, key.to_sym => input)
         end
 
         it 'puts the right value in the hash' do
@@ -84,47 +91,25 @@ describe Gitlab::Git::Env do
     end
   end
 
-  describe ".[]" do
-    context 'with RequestStore.store enabled' do
-      before do
-        allow(RequestStore).to receive(:active?).and_return(true)
-      end
-
-      before do
-        described_class.set(
-          GIT_OBJECT_DIRECTORY: 'foo',
-          GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar')
-      end
-
-      it 'returns a stored value for an existing key' do
-        expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo')
-      end
-
-      it 'returns nil for an non-existing key' do
-        expect(described_class[:foo]).to be_nil
-      end
-    end
-  end
-
   describe 'thread-safety' do
     context 'with RequestStore.store enabled' do
       before do
         allow(RequestStore).to receive(:active?).and_return(true)
-        described_class.set(GIT_OBJECT_DIRECTORY: 'foo')
+        described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')
       end
 
       it 'is thread-safe' do
         another_thread = Thread.new do
-          described_class.set(GIT_OBJECT_DIRECTORY: 'bar')
+          described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'bar')
 
           Thread.stop
-          described_class[:GIT_OBJECT_DIRECTORY]
+          described_class.all(gl_repository)[:GIT_OBJECT_DIRECTORY_RELATIVE]
         end
 
         # Ensure another_thread runs first
         sleep 0.1 until another_thread.stop?
 
-        expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo')
+        expect(described_class.all(gl_repository)[:GIT_OBJECT_DIRECTORY_RELATIVE]).to eq('foo')
 
         another_thread.run
         expect(another_thread.value).to eq('bar')
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 0e315b3f49e..5cbe2808d0b 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -120,7 +120,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     describe 'alternates keyword argument' do
       context 'with no Git env stored' do
         before do
-          allow(Gitlab::Git::Env).to receive(:all).and_return({})
+          allow(Gitlab::Git::HookEnv).to receive(:all).and_return({})
         end
 
         it "is passed an empty array" do
@@ -132,7 +132,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
 
       context 'with absolute and relative Git object dir envvars stored' do
         before do
-          allow(Gitlab::Git::Env).to receive(:all).and_return({
+          allow(Gitlab::Git::HookEnv).to receive(:all).and_return({
             'GIT_OBJECT_DIRECTORY_RELATIVE' => './objects/foo',
             'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['./objects/bar', './objects/baz'],
             'GIT_OBJECT_DIRECTORY' => 'ignored',
@@ -148,22 +148,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
           repository.rugged
         end
       end
-
-      context 'with only absolute Git object dir envvars stored' do
-        before do
-          allow(Gitlab::Git::Env).to receive(:all).and_return({
-            'GIT_OBJECT_DIRECTORY' => 'foo',
-            'GIT_ALTERNATE_OBJECT_DIRECTORIES' => %w[bar baz],
-            'GIT_OTHER' => 'another_env'
-          })
-        end
-
-        it "is passed the absolute object dir envvars as is" do
-          expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: %w[foo bar baz])
-
-          repository.rugged
-        end
-      end
     end
   end
 
diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb
index 4e0ee206219..32ec1e029c8 100644
--- a/spec/lib/gitlab/git/rev_list_spec.rb
+++ b/spec/lib/gitlab/git/rev_list_spec.rb
@@ -3,17 +3,6 @@ require 'spec_helper'
 describe Gitlab::Git::RevList do
   let(:repository) { create(:project, :repository).repository.raw }
   let(:rev_list) { described_class.new(repository, newrev: 'newrev') }
-  let(:env_hash) do
-    {
-      'GIT_OBJECT_DIRECTORY' => 'foo',
-      'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar'
-    }
-  end
-  let(:command_env) { { 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'foo:bar' } }
-
-  before do
-    allow(Gitlab::Git::Env).to receive(:all).and_return(env_hash)
-  end
 
   def args_for_popen(args_list)
     [Gitlab.config.git.bin_path, 'rev-list', *args_list]
@@ -23,7 +12,7 @@ describe Gitlab::Git::RevList do
     params = [
       args_for_popen(additional_args),
       repository.path,
-      command_env,
+      {},
       hash_including(lazy_block: with_lazy_block ? anything : nil)
     ]
 
diff --git a/spec/lib/gitlab/gitaly_client/util_spec.rb b/spec/lib/gitlab/gitaly_client/util_spec.rb
index d1e0136f8c1..550db6db6d9 100644
--- a/spec/lib/gitlab/gitaly_client/util_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/util_spec.rb
@@ -7,16 +7,19 @@ describe Gitlab::GitalyClient::Util do
     let(:gl_repository) { 'project-1' }
     let(:git_object_directory) { '.git/objects' }
     let(:git_alternate_object_directory) { ['/dir/one', '/dir/two'] }
+    let(:git_env) do
+      {
+        'GIT_OBJECT_DIRECTORY_RELATIVE' => git_object_directory,
+        'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => git_alternate_object_directory
+      }
+    end
 
     subject do
       described_class.repository(repository_storage, relative_path, gl_repository)
     end
 
     it 'creates a Gitaly::Repository with the given data' do
-      allow(Gitlab::Git::Env).to receive(:[]).with('GIT_OBJECT_DIRECTORY_RELATIVE')
-        .and_return(git_object_directory)
-      allow(Gitlab::Git::Env).to receive(:[]).with('GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE')
-        .and_return(git_alternate_object_directory)
+      allow(Gitlab::Git::HookEnv).to receive(:all).with(gl_repository).and_return(git_env)
 
       expect(subject).to be_a(Gitaly::Repository)
       expect(subject.storage_name).to eq(repository_storage)