Merge branch...

Merge branch '34502-gitlab-git-hook-should-set-the-gl_repository-environment-variable' into 'master'

Set the GL_REPOSITORY env variable on Gitlab::Git::Hook

Closes #34502

See merge request !12572

app/services/git_hooks_service.rb

@@ -3,8 +3,8 @@ class GitHooksService
 
   attr_accessor :oldrev, :newrev, :ref
 
-  def execute(user, repo_path, oldrev, newrev, ref)
-    @repo_path  = repo_path
+  def execute(user, project, oldrev, newrev, ref)
+    @project    = project
     @user       = Gitlab::GlId.gl_id(user)
     @oldrev     = oldrev
     @newrev     = newrev
@@ -26,7 +26,7 @@ class GitHooksService
   private
 
   def run_hook(name)
-    hook = Gitlab::Git::Hook.new(name, @repo_path)
+    hook = Gitlab::Git::Hook.new(name, @project)
     hook.trigger(@user, oldrev, newrev, ref)
   end
 end

app/services/git_operation_service.rb

@@ -120,7 +120,7 @@ class GitOperationService
   def with_hooks(ref, newrev, oldrev)
     GitHooksService.new.execute(
       user,
-      repository.path_to_repo,
+      repository.project,
       oldrev,
       newrev,
       ref) do |service|

lib/gitlab/git/hook.rb

@@ -4,9 +4,10 @@ module Gitlab
       GL_PROTOCOL = 'web'.freeze
       attr_reader :name, :repo_path, :path
 
-      def initialize(name, repo_path)
+      def initialize(name, project)
         @name = name
-        @repo_path = repo_path
+        @project = project
+        @repo_path = project.repository.path
         @path = File.join(repo_path.strip, 'hooks', name)
       end
 
@@ -38,7 +39,8 @@ module Gitlab
         vars = {
           'GL_ID' => gl_id,
           'PWD' => repo_path,
-          'GL_PROTOCOL' => GL_PROTOCOL
+          'GL_PROTOCOL' => GL_PROTOCOL,
+          'GL_REPOSITORY' => Gitlab::GlRepository.gl_repository(@project, false)
         }
 
         options = {

spec/features/projects/import_export/import_file_spec.rb

@@ -98,6 +98,6 @@ feature 'Import/Export - project import integration test', feature: true, js: tr
   end
 
   def project_hook_exists?(project)
-    Gitlab::Git::Hook.new('post-receive', project.repository.path).exists?
+    Gitlab::Git::Hook.new('post-receive', project).exists?
   end
 end

spec/lib/gitlab/git/hook_spec.rb

@@ -4,18 +4,20 @@ require 'fileutils'
 describe Gitlab::Git::Hook, lib: true do
   describe "#trigger" do
     let(:project) { create(:project, :repository) }
+    let(:repo_path) { project.repository.path }
     let(:user) { create(:user) }
+    let(:gl_id) { Gitlab::GlId.gl_id(user) }
 
     def create_hook(name)
-      FileUtils.mkdir_p(File.join(project.repository.path, 'hooks'))
-      File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f|
+      FileUtils.mkdir_p(File.join(repo_path, 'hooks'))
+      File.open(File.join(repo_path, 'hooks', name), 'w', 0755) do |f|
         f.write('exit 0')
       end
     end
 
     def create_failing_hook(name)
-      FileUtils.mkdir_p(File.join(project.repository.path, 'hooks'))
-      File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f|
+      FileUtils.mkdir_p(File.join(repo_path, 'hooks'))
+      File.open(File.join(repo_path, 'hooks', name), 'w', 0755) do |f|
         f.write(<<-HOOK)
           echo 'regular message from the hook'
           echo 'error message from the hook' 1>&2
@@ -27,13 +29,29 @@ describe Gitlab::Git::Hook, lib: true do
     ['pre-receive', 'post-receive', 'update'].each do |hook_name|
       context "when triggering a #{hook_name} hook" do
         context "when the hook is successful" do
+          let(:hook_path) { File.join(repo_path, 'hooks', hook_name) }
+          let(:gl_repository) { Gitlab::GlRepository.gl_repository(project, false) }
+          let(:env) do
+            {
+              'GL_ID' => gl_id,
+              'PWD' => repo_path,
+              'GL_PROTOCOL' => 'web',
+              'GL_REPOSITORY' => gl_repository
+            }
+          end
+
           it "returns success with no errors" do
             create_hook(hook_name)
-            hook = Gitlab::Git::Hook.new(hook_name, project.repository.path)
+            hook = Gitlab::Git::Hook.new(hook_name, project)
             blank = Gitlab::Git::BLANK_SHA
             ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch'
 
-            status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref)
+            if hook_name != 'update'
+              expect(Open3).to receive(:popen3)
+                .with(env, hook_path, chdir: repo_path).and_call_original
+            end
+
+            status, errors = hook.trigger(gl_id, blank, blank, ref)
             expect(status).to be true
             expect(errors).to be_blank
           end
@@ -42,11 +60,11 @@ describe Gitlab::Git::Hook, lib: true do
         context "when the hook is unsuccessful" do
           it "returns failure with errors" do
             create_failing_hook(hook_name)
-            hook = Gitlab::Git::Hook.new(hook_name, project.repository.path)
+            hook = Gitlab::Git::Hook.new(hook_name, project)
             blank = Gitlab::Git::BLANK_SHA
             ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch'
 
-            status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref)
+            status, errors = hook.trigger(gl_id, blank, blank, ref)
             expect(status).to be false
             expect(errors).to eq("error message from the hook\n")
           end
@@ -56,11 +74,11 @@ describe Gitlab::Git::Hook, lib: true do
 
     context "when the hook doesn't exist" do
       it "returns success with no errors" do
-        hook = Gitlab::Git::Hook.new('unknown_hook', project.repository.path)
+        hook = Gitlab::Git::Hook.new('unknown_hook', project)
         blank = Gitlab::Git::BLANK_SHA
         ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch'
 
-        status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref)
+        status, errors = hook.trigger(gl_id, blank, blank, ref)
         expect(status).to be true
         expect(errors).to be_nil
       end

spec/lib/gitlab/import_export/repo_restorer_spec.rb

@@ -34,7 +34,7 @@ describe Gitlab::ImportExport::RepoRestorer, services: true do
     it 'has the webhooks' do
       restorer.restore
 
-      expect(Gitlab::Git::Hook.new('post-receive', project.repository.path_to_repo)).to exist
+      expect(Gitlab::Git::Hook.new('post-receive', project)).to exist
     end
   end
 end

spec/models/repository_spec.rb

@@ -780,7 +780,7 @@ describe Repository, models: true do
     context 'when pre hooks were successful' do
       it 'runs without errors' do
         expect_any_instance_of(GitHooksService).to receive(:execute)
-          .with(user, project.repository.path_to_repo, old_rev, blank_sha, 'refs/heads/feature')
+          .with(user, project, old_rev, blank_sha, 'refs/heads/feature')
 
         expect { repository.rm_branch(user, 'feature') }.not_to raise_error
       end
@@ -823,12 +823,7 @@ describe Repository, models: true do
         service = GitHooksService.new
         expect(GitHooksService).to receive(:new).and_return(service)
         expect(service).to receive(:execute)
-          .with(
-            user,
-            repository.path_to_repo,
-            old_rev,
-            new_rev,
-            'refs/heads/feature')
+          .with(user, project, old_rev, new_rev, 'refs/heads/feature')
           .and_yield(service).and_return(true)
       end
 
@@ -1474,9 +1469,9 @@ describe Repository, models: true do
 
       it 'passes commit SHA to pre-receive and update hooks,\
         and tag SHA to post-receive hook' do
-        pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', repository.path_to_repo)
-        update_hook = Gitlab::Git::Hook.new('update', repository.path_to_repo)
-        post_receive_hook = Gitlab::Git::Hook.new('post-receive', repository.path_to_repo)
+        pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', project)
+        update_hook = Gitlab::Git::Hook.new('update', project)
+        post_receive_hook = Gitlab::Git::Hook.new('post-receive', project)
 
         allow(Gitlab::Git::Hook).to receive(:new)
           .and_return(pre_receive_hook, update_hook, post_receive_hook)

spec/services/git_hooks_service_spec.rb

@@ -12,7 +12,6 @@ describe GitHooksService, services: true do
     @oldrev = sample_commit.parent_id
     @newrev = sample_commit.id
     @ref = 'refs/heads/feature'
-    @repo_path = project.repository.path_to_repo
   end
 
   describe '#execute' do
@@ -21,7 +20,7 @@ describe GitHooksService, services: true do
         hook = double(trigger: [true, nil])
         expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
 
-        service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }
+        service.execute(user, project, @blankrev, @newrev, @ref) { }
       end
     end
 
@@ -31,7 +30,7 @@ describe GitHooksService, services: true do
         expect(service).not_to receive(:run_hook).with('post-receive')
 
         expect do
-          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+          service.execute(user, project, @blankrev, @newrev, @ref)
         end.to raise_error(GitHooksService::PreReceiveError)
       end
     end
@@ -43,7 +42,7 @@ describe GitHooksService, services: true do
         expect(service).not_to receive(:run_hook).with('post-receive')
 
         expect do
-          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+          service.execute(user, project, @blankrev, @newrev, @ref)
         end.to raise_error(GitHooksService::PreReceiveError)
       end
     end