Add latest changes from gitlab-org/gitlab@15-1-stable-eev15.1.0-rc42

2 files changed, 61 insertions, 32 deletions

diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 5e1f92ae835..9fb34f74c82 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -258,9 +258,9 @@ module Gitlab
       end
 
       # List all commits which are new in the repository. If commits have been pushed into the repo
-      def list_new_commits(revisions, allow_quarantine: false)
+      def list_new_commits(revisions)
         git_env = Gitlab::Git::HookEnv.all(@gitaly_repo.gl_repository)
-        if allow_quarantine && git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].present?
+        if git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].present?
           # If we have a quarantine environment, then we can optimize the check
           # by doing a ListAllCommitsRequest. Instead of walking through
           # references, we just walk through all quarantined objects, which is
@@ -278,32 +278,29 @@ module Gitlab
           response = GitalyClient.call(@repository.storage, :commit_service, :list_all_commits, request, timeout: GitalyClient.medium_timeout)
 
           quarantined_commits = consume_commits_response(response)
-
-          if Feature.enabled?(:filter_quarantined_commits)
-            quarantined_commit_ids = quarantined_commits.map(&:id)
-
-            # While in general the quarantine directory would only contain objects
-            # which are actually new, this is not guaranteed by Git. In fact,
-            # git-push(1) may sometimes push objects which already exist in the
-            # target repository. We do not want to return those from this method
-            # though given that they're not actually new.
-            #
-            # To fix this edge-case we thus have to filter commits down to those
-            # which don't yet exist. To do so, we must check for object existence
-            # in the main repository, but the object directory of our repository
-            # points into the object quarantine. This can be fixed by unsetting
-            # it, which will cause us to use the normal repository as indicated by
-            # its relative path again.
-            main_repo = @gitaly_repo.dup
-            main_repo.git_object_directory = ""
-
-            # Check object existence of all quarantined commits' IDs.
-            quarantined_commit_existence = object_existence_map(quarantined_commit_ids, gitaly_repo: main_repo)
-
-            # And then we reject all quarantined commits which exist in the main
-            # repository already.
-            quarantined_commits.reject! { |c| quarantined_commit_existence[c.id] }
-          end
+          quarantined_commit_ids = quarantined_commits.map(&:id)
+
+          # While in general the quarantine directory would only contain objects
+          # which are actually new, this is not guaranteed by Git. In fact,
+          # git-push(1) may sometimes push objects which already exist in the
+          # target repository. We do not want to return those from this method
+          # though given that they're not actually new.
+          #
+          # To fix this edge-case we thus have to filter commits down to those
+          # which don't yet exist. To do so, we must check for object existence
+          # in the main repository, but the object directory of our repository
+          # points into the object quarantine. This can be fixed by unsetting
+          # it, which will cause us to use the normal repository as indicated by
+          # its relative path again.
+          main_repo = @gitaly_repo.dup
+          main_repo.git_object_directory = ""
+
+          # Check object existence of all quarantined commits' IDs.
+          quarantined_commit_existence = object_existence_map(quarantined_commit_ids, gitaly_repo: main_repo)
+
+          # And then we reject all quarantined commits which exist in the main
+          # repository already.
+          quarantined_commits.reject! { |c| quarantined_commit_existence[c.id] }
 
           quarantined_commits
         else
diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb
index 4637bf2e3ff..d575c0f470d 100644
--- a/lib/gitlab/gitaly_client/operation_service.rb
+++ b/lib/gitlab/gitaly_client/operation_service.rb
@@ -101,6 +101,16 @@ module Gitlab
         if pre_receive_error = response.pre_receive_error.presence
           raise Gitlab::Git::PreReceiveError, pre_receive_error
         end
+      rescue GRPC::BadStatus => e
+        detailed_error = decode_detailed_error(e)
+
+        case detailed_error&.error
+        when :custom_hook
+          raise Gitlab::Git::PreReceiveError.new(custom_hook_error_message(detailed_error.custom_hook),
+                                                 fallback_message: e.details)
+        else
+          raise
+        end
       end
 
       def user_merge_to_ref(user, source_sha:, branch:, target_ref:, message:, first_parent_ref:, allow_conflicts: false)
@@ -163,6 +173,9 @@ module Gitlab
           access_check_error = detailed_error.access_check
           # These messages were returned from internal/allowed API calls
           raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
+        when :custom_hook
+          raise Gitlab::Git::PreReceiveError.new(custom_hook_error_message(detailed_error.custom_hook),
+                                                 fallback_message: e.details)
         when :reference_update
           # We simply ignore any reference update errors which are typically an
           # indicator of multiple RPC calls trying to update the same reference
@@ -299,10 +312,6 @@ module Gitlab
           timeout: GitalyClient.long_timeout
         )
 
-        if response.git_error.presence
-          raise Gitlab::Git::Repository::GitError, response.git_error
-        end
-
         response.squash_sha
       rescue GRPC::BadStatus => e
         detailed_error = decode_detailed_error(e)
@@ -464,6 +473,21 @@ module Gitlab
         )
 
         handle_cherry_pick_or_revert_response(response)
+      rescue GRPC::BadStatus => e
+        detailed_error = decode_detailed_error(e)
+
+        case detailed_error&.error
+        when :access_check
+          access_check_error = detailed_error.access_check
+          # These messages were returned from internal/allowed API calls
+          raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
+        when :cherry_pick_conflict
+          raise Gitlab::Git::Repository::CreateTreeError, 'CONFLICT'
+        when :target_branch_diverged
+          raise Gitlab::Git::CommitError, 'branch diverged'
+        else
+          raise e
+        end
       end
 
       def handle_cherry_pick_or_revert_response(response)
@@ -526,6 +550,14 @@ module Gitlab
         # Error Class might not be known to ruby yet
         nil
       end
+
+      def custom_hook_error_message(custom_hook_error)
+        # Custom hooks may return messages via either stdout or stderr which have a specific prefix. If
+        # that prefix is present we'll want to print the hook's output, otherwise we'll want to print the
+        # Gitaly error as a fallback.
+        custom_hook_output = custom_hook_error.stderr.presence || custom_hook_error.stdout
+        EncodingHelper.encode!(custom_hook_output)
+      end
     end
   end
 end